cmd/gitannex: Add configparse.go and refactor

This is a behavior-preserving refactor. I'm mostly just moving the code
that defines and parses configs (e.g. "rcloneremotename") into a new
source file. This lets us focus more on implementing the text protocol
in gitannex.go.
This commit is contained in:
Dan McArdle 2025-03-22 13:48:29 -04:00
parent bc4234a9cb
commit 6380322c2e
3 changed files with 153 additions and 142 deletions

131
cmd/gitannex/configparse.go Normal file
View File

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

View File

@ -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")

View File

@ -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",