cli: use cached index with lib search (#1624)

[ardnew]

Please check if the PR fulfills these requirements

• The PR has no duplicates (please search among the Pull Requests
  before creating one)
• The PR follows
  our contributing guidelines
• Tests for the changes have been added (for bug fixes / features)
• Docs have been added / updated (for bug fixes / features)
• UPGRADING.md has been updated with a migration guide (for breaking changes)

• What kind of change does this PR introduce?
  Feature

• What is the current behavior?
  A new copy of library_index.json is downloaded every time lib search is called (unconditionally).

• What is the new behavior?
  A new copy of library_index.json is downloaded every time lib search is called and either one of the following conditions exists:
  1. The index has never been downloaded
  2. The time since the last index update is greater than one hour

• Does this PR introduce a breaking change, and is
  titled accordingly?
  No

• Other information:
  • Fixes: Avoid excessive libraries index downloads on lib search #1624
  • Changes based on: Fix update indexes running when not necessary #1063
  • Need assistance adding test cases and documentation for this change

---

See how to contribute

[cmaglie]

Thanks for the PR.
I see that you have taken the same patch for the core search and that's great!

I've made some comments to simplify this patch, IMHO after we validate these changes we can "backport" some of the suggestions to the core search command too.

Finally, the integration tests are failing, some of the tests expect the Upgrading library_index... message from the CLI, but this is no more happening after the patch. Could you fix the tests too? https://github.com/arduino/arduino-cli/runs/7155610048?check_suite_focus=true#step:10:6806

[cmaglie]

I would not bother too much with validating the timeout since it's a hardcoded constant.

Suggested change

	// The behavior of now.After(T) is confusing if T < 0 and MTIME in the future,
	// and is probably not what the user intended. Disallow negative T and inform
	// the user that positive thresholds are expected.
	if modTimeThreshold < 0 {
	feedback.Error(tr("Timeout must be non-negative: %dns (%s)", modTimeThreshold, duration))
	os.Exit(errorcodes.ErrBadArgument)
	}

[cmaglie]

I'd like to change the indexUpdateInterval constant to a time.Duration like:

const indexUpdateInterval = 60 * time.Minute

so the indexNeedsUpdating function may take directly a time.Duration

Suggested change

	// Valid duration units are "ns", "us" (or "µs"), "ms", "s", "m", "h".
	// Use a duration of 0 to always update the index.
	func indexNeedsUpdating(duration string) bool {
	func indexNeedsUpdating(timeout time.Duration) bool {

[cmaglie]

Also this one can be much more simplified, something like

return time.Since(info.ModTime()) > timeout

should work.

[cmaglie]

I think that just removing these two lines

arduino-cli/test/test_lib.py

Lines 456 to 457 in f556aff

	assert "Downloading index: library_index.json.gz downloaded" in lines
	assert "Downloading index signature: library_index.json.sig downloaded" in lines

should be enough to fix the integration tests.