From 446d6b28b85641fc0e5d44edb6210e804367cd09 Mon Sep 17 00:00:00 2001 From: Dan McArdle Date: Fri, 10 May 2024 21:54:42 -0400 Subject: [PATCH] cmd/gitannex: Support synonyms of config values --- cmd/gitannex/gitannex.go | 68 ++++++++++++------ cmd/gitannex/gitannex_test.go | 126 ++++++++++++++++++++++++++++++++++ 2 files changed, 174 insertions(+), 20 deletions(-) diff --git a/cmd/gitannex/gitannex.go b/cmd/gitannex/gitannex.go index 0eb09da12..b8ef8b78f 100644 --- a/cmd/gitannex/gitannex.go +++ b/cmd/gitannex/gitannex.go @@ -110,12 +110,32 @@ func (m *messageParser) finalParameter() string { // 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 { - name string + 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 @@ -274,7 +294,7 @@ func (s *server) getRequiredConfigs() []configDefinition { return []configDefinition{ { - "rcloneremotename", + []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.)", @@ -282,7 +302,7 @@ func (s *server) getRequiredConfigs() []configDefinition { nil, }, { - "rcloneprefix", + []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.", @@ -290,7 +310,7 @@ func (s *server) getRequiredConfigs() []configDefinition { &defaultRclonePrefix, }, { - "rclonelayout", + []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), @@ -309,27 +329,35 @@ 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() { - s.sendMsg(fmt.Sprintf("GETCONFIG %s", config.name)) + var valueReceived bool + // Try each of the config's names in sequence, starting with the + // canonical name. + for _, configName := range config.names { + s.sendMsg(fmt.Sprintf("GETCONFIG %s", configName)) - message, err := s.getMsg() - if err != nil { - return err - } + message, err := s.getMsg() + if err != nil { + return err + } - valueKeyword, err := message.nextSpaceDelimitedParameter() - if err != nil || valueKeyword != "VALUE" { - return fmt.Errorf("failed to parse config value: %s %s", valueKeyword, message.line) - } + valueKeyword, err := message.nextSpaceDelimitedParameter() + if err != nil || valueKeyword != "VALUE" { + return fmt.Errorf("failed to parse config value: %s %s", valueKeyword, message.line) + } - value := message.finalParameter() - if value == "" && config.defaultValue == nil { - return fmt.Errorf("config value of %q must not be empty", config.name) + value := message.finalParameter() + if value != "" { + *config.destination = value + valueReceived = true + break + } } - if value == "" { + 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 - continue } - *config.destination = value } s.configsDone = true @@ -349,7 +377,7 @@ func (s *server) handlePrepare() error { // in sync with `handlePrepare()`. func (s *server) handleListConfigs() { for _, config := range s.getRequiredConfigs() { - s.sendMsg(fmt.Sprintf("CONFIG %s %s", config.name, config.description)) + 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 2b023b8c0..d53a7f921 100644 --- a/cmd/gitannex/gitannex_test.go +++ b/cmd/gitannex/gitannex_test.go @@ -7,6 +7,7 @@ import ( "io" "os" "path/filepath" + "regexp" "strings" "sync" "testing" @@ -192,6 +193,63 @@ 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, + } + + assert.Equal(t, "foo", + configFoo.getCanonicalName()) + + assert.Equal(t, + configFoo.description, + configFoo.fullDescription()) +} + +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, + } + + assert.Equal(t, "foo", + configFoo.getCanonicalName()) + + assert.Equal(t, + "(synonyms: bar) The foo config is utterly useless.", + configFoo.fullDescription()) +} + +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, + } + + assert.Equal(t, "foo", + configFoo.getCanonicalName()) + + assert.Equal(t, + `(synonyms: bar, baz) The foo config is utterly useless.`, + configFoo.fullDescription()) +} + type testState struct { t *testing.T server *server @@ -224,6 +282,12 @@ func (h *testState) requireReadLineExact(line string) { require.Equal(h.t, line+"\n", receivedLine) } +func (h *testState) requireReadLine() string { + receivedLine, err := h.mockStdoutReader.ReadString('\n') + require.NoError(h.t, err) + return receivedLine +} + func (h *testState) requireWriteLine(line string) { _, err := h.mockStdinW.Write([]byte(line + "\n")) require.NoError(h.t, err) @@ -269,6 +333,34 @@ var localBackendTestCases = []testCase{ require.NoError(t, h.mockStdinW.Close()) }, }, + { + label: "HandlesListConfigs", + testProtocolFunc: func(t *testing.T, h *testState) { + h.preconfigureServer() + + h.requireReadLineExact("VERSION 1") + h.requireWriteLine("INITREMOTE") + h.requireReadLineExact("INITREMOTE-SUCCESS") + + h.requireWriteLine("LISTCONFIGS") + + require.Regexp(t, + regexp.MustCompile(`^CONFIG rcloneremotename \(synonyms: target\) (.|\n)*$`), + h.requireReadLine(), + ) + require.Regexp(t, + regexp.MustCompile(`^CONFIG rcloneprefix \(synonyms: prefix\) (.|\n)*$`), + h.requireReadLine(), + ) + require.Regexp(t, + regexp.MustCompile(`^CONFIG rclonelayout \(synonyms: rclone_layout\) (.|\n)*$`), + h.requireReadLine(), + ) + h.requireReadLineExact("CONFIGEND") + + require.NoError(t, h.mockStdinW.Close()) + }, + }, { label: "HandlesPrepare", testProtocolFunc: func(t *testing.T, h *testState) { @@ -297,6 +389,38 @@ var localBackendTestCases = []testCase{ require.NoError(t, h.mockStdinW.Close()) }, }, + { + label: "HandlesPrepareWithSynonyms", + testProtocolFunc: func(t *testing.T, h *testState) { + h.requireReadLineExact("VERSION 1") + h.requireWriteLine("EXTENSIONS INFO") // Advertise that we support the INFO extension + h.requireReadLineExact("EXTENSIONS") + + if !h.server.extensionInfo { + t.Errorf("expected INFO extension to be enabled") + return + } + + h.requireWriteLine("PREPARE") + h.requireReadLineExact("GETCONFIG rcloneremotename") + // TODO check what git-annex does when asked for a config value it does not have. + h.requireWriteLine("VALUE") + h.requireReadLineExact("GETCONFIG target") + h.requireWriteLine("VALUE " + h.remoteName) + + h.requireReadLineExact("GETCONFIG rcloneprefix") + h.requireWriteLine("VALUE " + h.localFsDir) + h.requireReadLineExact("GETCONFIG rclonelayout") + h.requireWriteLine("VALUE foo") + h.requireReadLineExact("PREPARE-SUCCESS") + + require.Equal(t, h.server.configRcloneRemoteName, h.remoteName) + require.Equal(t, h.server.configPrefix, h.localFsDir) + require.True(t, h.server.configsDone) + + require.NoError(t, h.mockStdinW.Close()) + }, + }, { label: "HandlesPrepareAndDoesNotTrimWhitespaceFromValue", testProtocolFunc: func(t *testing.T, h *testState) { @@ -322,6 +446,8 @@ var localBackendTestCases = []testCase{ h.requireReadLineExact("GETCONFIG rclonelayout") h.requireWriteLine("VALUE") + h.requireReadLineExact("GETCONFIG rclone_layout") + h.requireWriteLine("VALUE") h.requireReadLineExact("PREPARE-SUCCESS")