Skip to content

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

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Closed
wants to merge 3 commits into from
Closed

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

Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
15264b2
cli: use cached index with lib search (#1624)
ardnew Jul 1, 2022
bcbda90
remove comment on default cache timeout (maintenance smell)
ardnew Jul 1, 2022
f556aff
run gofmt on changes
ardnew Jul 1, 2022
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
57 changes: 50 additions & 7 deletions cli/lib/search.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -21,14 +21,17 @@ import (
"os"
"sort"
"strings"
"time"

"github.com/arduino/arduino-cli/cli/errorcodes"
"github.com/arduino/arduino-cli/cli/feedback"
"github.com/arduino/arduino-cli/cli/instance"
"github.com/arduino/arduino-cli/cli/output"
"github.com/arduino/arduino-cli/commands"
"github.com/arduino/arduino-cli/commands/lib"
"github.com/arduino/arduino-cli/configuration"
rpc "github.com/arduino/arduino-cli/rpc/cc/arduino/cli/commands/v1"
"github.com/arduino/go-paths-helper"
"github.com/sirupsen/logrus"
"github.com/spf13/cobra"
semver "go.bug.st/relaxed-semver"
Expand All @@ -51,6 +54,9 @@ func initSearchCommand() *cobra.Command {
return searchCommand
}

// indexUpdateInterval specifies the time threshold over which indexes are updated
const indexUpdateInterval = "60m"

func runSearchCommand(cmd *cobra.Command, args []string) {
inst, status := instance.Create()
logrus.Info("Executing `arduino-cli lib search`")
Expand All @@ -60,13 +66,15 @@ func runSearchCommand(cmd *cobra.Command, args []string) {
os.Exit(errorcodes.ErrGeneric)
}

if err := commands.UpdateLibrariesIndex(
context.Background(),
&rpc.UpdateLibrariesIndexRequest{Instance: inst},
output.ProgressBar(),
); err != nil {
feedback.Errorf(tr("Error updating library index: %v"), err)
os.Exit(errorcodes.ErrGeneric)
if indexNeedsUpdating(indexUpdateInterval) {
if err := commands.UpdateLibrariesIndex(
context.Background(),
&rpc.UpdateLibrariesIndexRequest{Instance: inst},
output.ProgressBar(),
); err != nil {
feedback.Errorf(tr("Error updating library index: %v"), err)
os.Exit(errorcodes.ErrGeneric)
}
}

for _, err := range instance.Init(inst) {
Expand Down Expand Up @@ -193,3 +201,38 @@ func versionsFromSearchedLibrary(library *rpc.SearchedLibrary) []*semver.Version
sort.Sort(semver.List(res))
return res
}

// indexNeedsUpdating returns whether library_index.json need updating.
// A positive duration string must be provided to calculate the time threshold
// used to update the index.
// 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 {
Comment on lines +208 to +210
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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 {

// Library index path is constant (relative to the data directory).
// It does not depend on board manager URLs or any other configuration.
dataDir := configuration.Settings.GetString("directories.Data")
indexPath := paths.New(dataDir).Join("library_index.json")
// Verify the index file exists and we can read its fstat attrs.
if indexPath.NotExist() {
return true
}
info, err := indexPath.Stat()
if err != nil {
return true
}
// Sanity check the given threshold duration string.
now := time.Now()
modTimeThreshold, err := time.ParseDuration(duration)
if err != nil {
feedback.Error(tr("Invalid timeout: %s", err))
os.Exit(errorcodes.ErrBadArgument)
}
// 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)
}
Comment on lines +230 to +236
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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)
}

return modTimeThreshold == 0 || now.After(info.ModTime().Add(modTimeThreshold))
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Also this one can be much more simplified, something like

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

should work.

}