diff --git a/cmd/gitannex/configparse.go b/cmd/gitannex/configparse.go new file mode 100644 index 000000000..af0f627a7 --- /dev/null +++ b/cmd/gitannex/configparse.go @@ -0,0 +1,131 @@ +package gitannex + +import ( + "fmt" + "slices" + "strings" + + "github.com/rclone/rclone/fs" + "github.com/rclone/rclone/fs/config" + "github.com/rclone/rclone/fs/fspath" +) + +type configID int + +const ( + configRemoteName configID = iota + configPrefix + configLayout +) + +// configDefinition describes a configuration value required by this command. We +// use "GETCONFIG" messages to query git-annex for these values at runtime. +type configDefinition struct { + id configID + names []string + description string + defaultValue string +} + +const ( + defaultRclonePrefix = "git-annex-rclone" + defaultRcloneLayout = "nodir" +) + +var requiredConfigs = []configDefinition{ + { + id: configRemoteName, + names: []string{"rcloneremotename", "target"}, + description: "Name of the rclone remote to use. " + + "Must match a remote known to rclone. " + + "(Note that rclone remotes are a distinct concept from git-annex remotes.)", + }, + { + id: configPrefix, + names: []string{"rcloneprefix", "prefix"}, + description: "Directory where rclone will write git-annex content. " + + fmt.Sprintf("If not specified, defaults to %q. ", defaultRclonePrefix) + + "This directory will be created on init if it does not exist.", + defaultValue: defaultRclonePrefix, + }, + { + id: configLayout, + names: []string{"rclonelayout", "rclone_layout"}, + description: "Defines where, within the rcloneprefix directory, rclone will write git-annex content. " + + fmt.Sprintf("Must be one of %v. ", allLayoutModes()) + + fmt.Sprintf("If empty, defaults to %q.", defaultRcloneLayout), + defaultValue: defaultRcloneLayout, + }, +} + +func (c *configDefinition) getCanonicalName() string { + if len(c.names) < 1 { + panic(fmt.Errorf("configDefinition must have at least one name: %v", c)) + } + return c.names[0] +} + +// fullDescription returns a single-line, human-readable description for this +// config. The returned string begins with a list of synonyms and ends with +// `c.description`. +func (c *configDefinition) fullDescription() string { + if len(c.names) <= 1 { + return c.description + } + // Exclude the canonical name from the list of synonyms. + synonyms := c.names[1:len(c.names)] + commaSeparatedSynonyms := strings.Join(synonyms, ", ") + return fmt.Sprintf("(synonyms: %s) %s", commaSeparatedSynonyms, c.description) +} + +// validateRemoteName validates the "rcloneremotename" config that we receive +// from git-annex. It returns nil iff `value` is valid. Otherwise, it returns a +// descriptive error suitable for sending back to git-annex via stdout. +// +// The value is only valid when: +// 1. It is the exact name of an existing remote. +// 2. It is an fspath string that names an existing remote or a backend. The +// string may include options, but it must not include a path. (That's what +// the "rcloneprefix" config is for.) +// +// While backends are not remote names, per se, they are permitted for +// compatibility with [fstest]. We could guard this behavior behind +// [testing.Testing] to prevent users from specifying backend strings, but +// there's no obvious harm in permitting it. +func validateRemoteName(value string) error { + remoteNames := config.GetRemoteNames() + // Check whether `value` is an exact match for an existing remote. + // + // If we checked whether [cache.Get] returns [fs.ErrorNotFoundInConfigFile], + // we would incorrectly identify file names as valid remote names. We also + // avoid [config.FileSections] because it will miss remotes that are defined + // by environment variables. + if slices.Contains(remoteNames, value) { + return nil + } + parsed, err := fspath.Parse(value) + if err != nil { + return fmt.Errorf("remote could not be parsed: %s", value) + } + if parsed.Path != "" { + return fmt.Errorf("remote does not exist or incorrectly contains a path: %s", value) + } + // Now that we've established `value` is an fspath string that does not + // include a path component, we only need to check whether it names an + // existing remote or backend. + if slices.Contains(remoteNames, parsed.Name) { + return nil + } + maybeBackend := strings.HasPrefix(value, ":") + if !maybeBackend { + return fmt.Errorf("remote does not exist: %s", value) + } + // Strip the leading colon before searching for the backend. For instance, + // search for "local" instead of ":local". Note that `parsed.Name` already + // omits any config options baked into the string. + trimmedBackendName := strings.TrimPrefix(parsed.Name, ":") + if _, err = fs.Find(trimmedBackendName); err != nil { + return fmt.Errorf("backend does not exist: %s", trimmedBackendName) + } + return nil +} diff --git a/cmd/gitannex/gitannex.go b/cmd/gitannex/gitannex.go index b27a1dfb8..ae70554b8 100644 --- a/cmd/gitannex/gitannex.go +++ b/cmd/gitannex/gitannex.go @@ -28,14 +28,11 @@ import ( "io" "os" "path/filepath" - "slices" "strings" "github.com/rclone/rclone/cmd" "github.com/rclone/rclone/fs" "github.com/rclone/rclone/fs/cache" - "github.com/rclone/rclone/fs/config" - "github.com/rclone/rclone/fs/fspath" "github.com/rclone/rclone/fs/operations" "github.com/spf13/cobra" ) @@ -110,35 +107,6 @@ func (m *messageParser) finalParameter() string { return param } -// configDefinition describes a configuration value required by this command. We -// use "GETCONFIG" messages to query git-annex for these values at runtime. -type configDefinition struct { - names []string - description string - destination *string - defaultValue *string -} - -func (c *configDefinition) getCanonicalName() string { - if len(c.names) < 1 { - panic(fmt.Errorf("configDefinition must have at least one name: %v", c)) - } - return c.names[0] -} - -// fullDescription returns a single-line, human-readable description for this -// config. The returned string begins with a list of synonyms and ends with -// `c.description`. -func (c *configDefinition) fullDescription() string { - if len(c.names) <= 1 { - return c.description - } - // Exclude the canonical name from the list of synonyms. - synonyms := c.names[1:len(c.names)] - commaSeparatedSynonyms := strings.Join(synonyms, ", ") - return fmt.Sprintf("(synonyms: %s) %s", commaSeparatedSynonyms, c.description) -} - // server contains this command's current state. type server struct { reader *bufio.Reader @@ -283,88 +251,16 @@ func (s *server) handleInitRemote() error { return nil } -// validateRemoteName validates the "rcloneremotename" config that we receive -// from git-annex. It returns nil iff `value` is valid. Otherwise, it returns a -// descriptive error suitable for sending back to git-annex via stdout. -// -// The value is only valid when: -// 1. It is the exact name of an existing remote. -// 2. It is an fspath string that names an existing remote or a backend. The -// string may include options, but it must not include a path. (That's what -// the "rcloneprefix" config is for.) -// -// While backends are not remote names, per se, they are permitted for -// compatibility with [fstest]. We could guard this behavior behind -// [testing.Testing] to prevent users from specifying backend strings, but -// there's no obvious harm in permitting it. -func validateRemoteName(value string) error { - remoteNames := config.GetRemoteNames() - // Check whether `value` is an exact match for an existing remote. - // - // If we checked whether [cache.Get] returns [fs.ErrorNotFoundInConfigFile], - // we would incorrectly identify file names as valid remote names. We also - // avoid [config.FileSections] because it will miss remotes that are defined - // by environment variables. - if slices.Contains(remoteNames, value) { - return nil - } - parsed, err := fspath.Parse(value) - if err != nil { - return fmt.Errorf("remote could not be parsed: %s", value) - } - if parsed.Path != "" { - return fmt.Errorf("remote does not exist or incorrectly contains a path: %s", value) - } - // Now that we've established `value` is an fspath string that does not - // include a path component, we only need to check whether it names an - // existing remote or backend. - if slices.Contains(remoteNames, parsed.Name) { - return nil - } - maybeBackend := strings.HasPrefix(value, ":") - if !maybeBackend { - return fmt.Errorf("remote does not exist: %s", value) - } - // Strip the leading colon before searching for the backend. For instance, - // search for "local" instead of ":local". Note that `parsed.Name` already - // omits any config options baked into the string. - trimmedBackendName := strings.TrimPrefix(parsed.Name, ":") - if _, err = fs.Find(trimmedBackendName); err != nil { - return fmt.Errorf("backend does not exist: %s", trimmedBackendName) - } - return nil -} - -// Get a list of configs with pointers to fields of `s`. -func (s *server) getRequiredConfigs() []configDefinition { - defaultRclonePrefix := "git-annex-rclone" - defaultRcloneLayout := "nodir" - - return []configDefinition{ - { - []string{"rcloneremotename", "target"}, - "Name of the rclone remote to use. " + - "Must match a remote known to rclone. " + - "(Note that rclone remotes are a distinct concept from git-annex remotes.)", - &s.configRcloneRemoteName, - nil, - }, - { - []string{"rcloneprefix", "prefix"}, - "Directory where rclone will write git-annex content. " + - fmt.Sprintf("If not specified, defaults to %q. ", defaultRclonePrefix) + - "This directory will be created on init if it does not exist.", - &s.configPrefix, - &defaultRclonePrefix, - }, - { - []string{"rclonelayout", "rclone_layout"}, - "Defines where, within the rcloneprefix directory, rclone will write git-annex content. " + - fmt.Sprintf("Must be one of %v. ", allLayoutModes()) + - fmt.Sprintf("If empty, defaults to %q.", defaultRcloneLayout), - &s.configRcloneLayout, - &defaultRcloneLayout, - }, +func (s *server) mustSetConfigValue(id configID, value string) { + switch id { + case configRemoteName: + s.configRcloneRemoteName = value + case configPrefix: + s.configPrefix = value + case configLayout: + s.configRcloneLayout = value + default: + panic(fmt.Errorf("unhandled configId: %v", id)) } } @@ -376,8 +272,8 @@ func (s *server) queryConfigs() error { // Send a "GETCONFIG" message for each required config and parse git-annex's // "VALUE" response. - for _, config := range s.getRequiredConfigs() { - var valueReceived bool +queryNextConfig: + for _, config := range requiredConfigs { // Try each of the config's names in sequence, starting with the // canonical name. for _, configName := range config.names { @@ -393,19 +289,15 @@ func (s *server) queryConfigs() error { return fmt.Errorf("failed to parse config value: %s %s", valueKeyword, message.line) } - value := message.finalParameter() - if value != "" { - *config.destination = value - valueReceived = true - break + if value := message.finalParameter(); value != "" { + s.mustSetConfigValue(config.id, value) + continue queryNextConfig } } - if !valueReceived { - if config.defaultValue == nil { - return fmt.Errorf("did not receive a non-empty config value for %q", config.getCanonicalName()) - } - *config.destination = *config.defaultValue + if config.defaultValue == "" { + return fmt.Errorf("did not receive a non-empty config value for %q", config.getCanonicalName()) } + s.mustSetConfigValue(config.id, config.defaultValue) } s.configsDone = true @@ -424,7 +316,7 @@ func (s *server) handlePrepare() error { // Git-annex is asking us to return the list of settings that we use. Keep this // in sync with `handlePrepare()`. func (s *server) handleListConfigs() { - for _, config := range s.getRequiredConfigs() { + for _, config := range requiredConfigs { s.sendMsg(fmt.Sprintf("CONFIG %s %s", config.getCanonicalName(), config.fullDescription())) } s.sendMsg("CONFIGEND") diff --git a/cmd/gitannex/gitannex_test.go b/cmd/gitannex/gitannex_test.go index 786cd7a4c..267c2de9d 100644 --- a/cmd/gitannex/gitannex_test.go +++ b/cmd/gitannex/gitannex_test.go @@ -190,14 +190,10 @@ func TestMessageParser(t *testing.T) { } func TestConfigDefinitionOneName(t *testing.T) { - var parsed string - var defaultValue = "abc" - configFoo := configDefinition{ names: []string{"foo"}, description: "The foo config is utterly useless.", - destination: &parsed, - defaultValue: &defaultValue, + defaultValue: "abc", } assert.Equal(t, "foo", @@ -209,14 +205,10 @@ func TestConfigDefinitionOneName(t *testing.T) { } func TestConfigDefinitionTwoNames(t *testing.T) { - var parsed string - var defaultValue = "abc" - configFoo := configDefinition{ names: []string{"foo", "bar"}, description: "The foo config is utterly useless.", - destination: &parsed, - defaultValue: &defaultValue, + defaultValue: "abc", } assert.Equal(t, "foo", @@ -228,14 +220,10 @@ func TestConfigDefinitionTwoNames(t *testing.T) { } func TestConfigDefinitionThreeNames(t *testing.T) { - var parsed string - var defaultValue = "abc" - configFoo := configDefinition{ names: []string{"foo", "bar", "baz"}, description: "The foo config is utterly useless.", - destination: &parsed, - defaultValue: &defaultValue, + defaultValue: "abc", } assert.Equal(t, "foo",