From ec501991a444d876c379d37bba600817dc70f7f7 Mon Sep 17 00:00:00 2001 From: Taylor Blau Date: Wed, 10 Aug 2016 13:51:53 -0600 Subject: [PATCH 1/7] config,etc: remove uses of GetenvBool --- auth/credentials.go | 2 +- commands/command_smudge.go | 2 +- commands/commands.go | 6 +++--- config/config.go | 13 ++++--------- httputil/certs.go | 2 +- 5 files changed, 10 insertions(+), 15 deletions(-) diff --git a/auth/credentials.go b/auth/credentials.go index 41b4b581..f8970020 100644 --- a/auth/credentials.go +++ b/auth/credentials.go @@ -212,7 +212,7 @@ func execCredsCommand(cfg *config.Configuration, input Creds, subCommand string) } if _, ok := err.(*exec.ExitError); ok { - if !cfg.GetenvBool("GIT_TERMINAL_PROMPT", true) { + if !cfg.Os.Bool("GIT_TERMINAL_PROMPT", true) { return nil, fmt.Errorf("Change the GIT_TERMINAL_PROMPT env var to be prompted to enter your credentials for %s://%s.", input["protocol"], input["host"]) } diff --git a/commands/command_smudge.go b/commands/command_smudge.go index 79042aec..33c3d663 100644 --- a/commands/command_smudge.go +++ b/commands/command_smudge.go @@ -64,7 +64,7 @@ func smudgeCommand(cmd *cobra.Command, args []string) { download := lfs.FilenamePassesIncludeExcludeFilter(filename, cfg.FetchIncludePaths(), cfg.FetchExcludePaths()) - if smudgeSkip || cfg.GetenvBool("GIT_LFS_SKIP_SMUDGE", false) { + if smudgeSkip || cfg.Os.Bool("GIT_LFS_SKIP_SMUDGE", false) { download = false } diff --git a/commands/commands.go b/commands/commands.go index 07c1973c..e78364e3 100644 --- a/commands/commands.go +++ b/commands/commands.go @@ -275,14 +275,14 @@ func usage(cmd *cobra.Command) error { } // isCommandEnabled returns whether the environment variable GITLFSENABLED -// is "truthy" according to config.GetenvBool (see -// github.com/github/git-lfs/config#Configuration.GetenvBool), returning false +// is "truthy" according to config.Os.Bool (see +// github.com/github/git-lfs/config#Configuration.Env.Os), returning false // by default if the enviornment variable is not specified. // // This function call should only guard commands that do not yet have stable // APIs or solid server implementations. func isCommandEnabled(cfg *config.Configuration, cmd string) bool { - return cfg.GetenvBool(fmt.Sprintf("GITLFS%sENABLED", strings.ToUpper(cmd)), false) + return cfg.Os.Bool(fmt.Sprintf("GITLFS%sENABLED", strings.ToUpper(cmd)), false) } func init() { diff --git a/config/config.go b/config/config.go index ecc87b7c..0932bbf1 100644 --- a/config/config.go +++ b/config/config.go @@ -82,9 +82,9 @@ func New() *Configuration { CurrentRemote: defaultRemote, envVars: make(map[string]string), } - c.IsTracingHttp = c.GetenvBool("GIT_CURL_VERBOSE", false) - c.IsDebuggingHttp = c.GetenvBool("LFS_DEBUG_HTTP", false) - c.IsLoggingStats = c.GetenvBool("GIT_LOG_STATS", false) + c.IsTracingHttp = c.Os.Bool("GIT_CURL_VERBOSE", false) + c.IsDebuggingHttp = c.Os.Bool("LFS_DEBUG_HTTP", false) + c.IsLoggingStats = c.Os.Bool("GIT_LOG_STATS", false) return c } @@ -208,11 +208,6 @@ func (c *Configuration) Getenv(key string) string { return v } -// GetenvBool is shorthand for `c.Os.Bool(key, def)`. -func (c *Configuration) GetenvBool(key string, def bool) bool { - return c.Os.Bool(key, def) -} - // GitRemoteUrl returns the git clone/push url for a given remote (blank if not found) // the forpush argument is to cater for separate remote.name.pushurl settings func (c *Configuration) GitRemoteUrl(remote string, forpush bool) string { @@ -465,7 +460,7 @@ func (c *Configuration) FetchPruneConfig() FetchPruneConfig { } func (c *Configuration) SkipDownloadErrors() bool { - return c.GetenvBool("GIT_LFS_SKIP_DOWNLOAD_ERRORS", false) || c.GitConfigBool("lfs.skipdownloaderrors", false) + return c.Os.Bool("GIT_LFS_SKIP_DOWNLOAD_ERRORS", false) || c.GitConfigBool("lfs.skipdownloaderrors", false) } func (c *Configuration) loadGitConfig() bool { diff --git a/httputil/certs.go b/httputil/certs.go index bf8feca8..dc1dd675 100644 --- a/httputil/certs.go +++ b/httputil/certs.go @@ -19,7 +19,7 @@ func isCertVerificationDisabledForHost(cfg *config.Configuration, host string) b } globalSslVerify, _ := cfg.GitConfig("http.sslverify") - if globalSslVerify == "false" || cfg.GetenvBool("GIT_SSL_NO_VERIFY", false) { + if globalSslVerify == "false" || cfg.Os.Bool("GIT_SSL_NO_VERIFY", false) { return true } From 254c11d179c323a7c2d643c88d2811ff5ad6b5b3 Mon Sep 17 00:00:00 2001 From: Taylor Blau Date: Wed, 10 Aug 2016 14:23:03 -0600 Subject: [PATCH 2/7] config.etc: remove Getenv utility method --- auth/ssh.go | 5 +++-- commands/command_checkout.go | 4 +++- config/config.go | 6 ------ config/config_netrc.go | 2 +- httputil/certs.go | 4 ++-- httputil/proxy.go | 12 ++++++------ lfs/transfer_queue.go | 4 +++- lfs/util.go | 2 +- 8 files changed, 19 insertions(+), 20 deletions(-) diff --git a/auth/ssh.go b/auth/ssh.go index 30b27220..be3b0a4b 100644 --- a/auth/ssh.go +++ b/auth/ssh.go @@ -69,8 +69,9 @@ func sshGetExeAndArgs(cfg *config.Configuration, endpoint config.Endpoint) (exe isPlink := false isTortoise := false - ssh := cfg.Getenv("GIT_SSH") - cmdArgs := strings.Fields(cfg.Getenv("GIT_SSH_COMMAND")) + ssh, _ := cfg.Os.Get("GIT_SSH") + sshCmd, _ := cfg.Os.Get("GIT_SSH_COMMAND") + cmdArgs := strings.Fields(sshCmd) if len(cmdArgs) > 0 { ssh = cmdArgs[0] cmdArgs = cmdArgs[1:] diff --git a/commands/command_checkout.go b/commands/command_checkout.go index b6113820..048c5763 100644 --- a/commands/command_checkout.go +++ b/commands/command_checkout.go @@ -120,7 +120,9 @@ func checkoutWithIncludeExclude(include []string, exclude []string) { for _, pointer := range pointers { totalBytes += pointer.Size } - progress := progress.NewProgressMeter(len(pointers), totalBytes, false, cfg.Getenv("GIT_LFS_PROGRESS")) + + logPath, _ := cfg.Os.Get("GIT_LFS_PROGRESS") + progress := progress.NewProgressMeter(len(pointers), totalBytes, false, logPath) progress.Start() totalBytes = 0 for _, pointer := range pointers { diff --git a/config/config.go b/config/config.go index 0932bbf1..05ed0a36 100644 --- a/config/config.go +++ b/config/config.go @@ -202,12 +202,6 @@ func (c *Configuration) parseTag(tag reflect.StructTag) (key string, env *Enviro return } -// Getenv is shorthand for `c.Os.Get(key)`. -func (c *Configuration) Getenv(key string) string { - v, _ := c.Os.Get(key) - return v -} - // GitRemoteUrl returns the git clone/push url for a given remote (blank if not found) // the forpush argument is to cater for separate remote.name.pushurl settings func (c *Configuration) GitRemoteUrl(remote string, forpush bool) string { diff --git a/config/config_netrc.go b/config/config_netrc.go index 4c3ff19b..27156459 100644 --- a/config/config_netrc.go +++ b/config/config_netrc.go @@ -18,7 +18,7 @@ func (n *noNetrc) FindMachine(host string) *netrc.Machine { } func (c *Configuration) parseNetrc() (netrcfinder, error) { - home := c.Getenv("HOME") + home, _ := c.Os.Get("HOME") if len(home) == 0 { return &noNetrc{}, nil } diff --git a/httputil/certs.go b/httputil/certs.go index dc1dd675..ad3f5df6 100644 --- a/httputil/certs.go +++ b/httputil/certs.go @@ -48,7 +48,7 @@ func appendRootCAsForHostFromGitconfig(cfg *config.Configuration, pool *x509.Cer // Accumulate certs from all these locations: // GIT_SSL_CAINFO first - if cafile := cfg.Getenv("GIT_SSL_CAINFO"); len(cafile) > 0 { + if cafile, _ := cfg.Os.Get("GIT_SSL_CAINFO"); len(cafile) > 0 { return appendCertsFromFile(pool, cafile) } // http./.sslcainfo or http..sslcainfo @@ -66,7 +66,7 @@ func appendRootCAsForHostFromGitconfig(cfg *config.Configuration, pool *x509.Cer return appendCertsFromFile(pool, cafile) } // GIT_SSL_CAPATH - if cadir := cfg.Getenv("GIT_SSL_CAPATH"); len(cadir) > 0 { + if cadir, _ := cfg.Os.Get("GIT_SSL_CAPATH"); len(cadir) > 0 { return appendCertsFromFilesInDir(pool, cadir) } // http.sslcapath diff --git a/httputil/proxy.go b/httputil/proxy.go index 54315700..e717195d 100644 --- a/httputil/proxy.go +++ b/httputil/proxy.go @@ -20,24 +20,24 @@ func ProxyFromGitConfigOrEnvironment(c *config.Configuration) func(req *http.Req } if len(https_proxy) == 0 { - https_proxy = c.Getenv("HTTPS_PROXY") + https_proxy, _ = c.Os.Get("HTTPS_PROXY") } if len(https_proxy) == 0 { - https_proxy = c.Getenv("https_proxy") + https_proxy, _ = c.Os.Get("https_proxy") } if len(http_proxy) == 0 { - http_proxy = c.Getenv("HTTP_PROXY") + http_proxy, _ = c.Os.Get("HTTP_PROXY") } if len(http_proxy) == 0 { - http_proxy = c.Getenv("http_proxy") + http_proxy, _ = c.Os.Get("http_proxy") } - no_proxy := c.Getenv("NO_PROXY") + no_proxy, _ := c.Os.Get("NO_PROXY") if len(no_proxy) == 0 { - no_proxy = c.Getenv("no_proxy") + no_proxy, _ = c.Os.Get("no_proxy") } return func(req *http.Request) (*url.URL, error) { diff --git a/lfs/transfer_queue.go b/lfs/transfer_queue.go index 2aca6979..0cd98937 100644 --- a/lfs/transfer_queue.go +++ b/lfs/transfer_queue.go @@ -58,10 +58,12 @@ type TransferQueue struct { // newTransferQueue builds a TransferQueue, direction and underlying mechanism determined by adapter func newTransferQueue(files int, size int64, dryRun bool, dir transfer.Direction) *TransferQueue { + logPath, _ := config.Config.Os.Get("GIT_LFS_PROGRESS") + q := &TransferQueue{ direction: dir, dryRun: dryRun, - meter: progress.NewProgressMeter(files, size, dryRun, config.Config.Getenv("GIT_LFS_PROGRESS")), + meter: progress.NewProgressMeter(files, size, dryRun, logPath), apic: make(chan Transferable, batchSize), retriesc: make(chan Transferable, batchSize), errorc: make(chan error), diff --git a/lfs/util.go b/lfs/util.go index 64300007..123f5e15 100644 --- a/lfs/util.go +++ b/lfs/util.go @@ -27,7 +27,7 @@ const ( var currentPlatform = PlatformUndetermined func CopyCallbackFile(event, filename string, index, totalFiles int) (progress.CopyCallback, *os.File, error) { - logPath := config.Config.Getenv("GIT_LFS_PROGRESS") + logPath, _ := config.Config.Os.Get("GIT_LFS_PROGRESS") if len(logPath) == 0 || len(filename) == 0 || len(event) == 0 { return nil, nil, nil } From c1170b75f28d47d8b11178759003c39bf526f343 Mon Sep 17 00:00:00 2001 From: Taylor Blau Date: Fri, 12 Aug 2016 13:21:25 -0600 Subject: [PATCH 3/7] transfer/custom: encode "event" as lowercase --- transfer/custom.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/transfer/custom.go b/transfer/custom.go index 210dc2c6..1e4949c8 100644 --- a/transfer/custom.go +++ b/transfer/custom.go @@ -63,7 +63,7 @@ type customAdapterWorkerContext struct { } type customAdapterInitRequest struct { - Event string `json:"Event"` + Event string `json:"event"` Operation string `json:"operation"` Concurrent bool `json:"concurrent"` ConcurrentTransfers int `json:"concurrenttransfers"` From 6c60d2b2db418f876b2c4fd934ee09db5131364f Mon Sep 17 00:00:00 2001 From: Taylor Blau Date: Mon, 15 Aug 2016 13:17:11 -0600 Subject: [PATCH 4/7] config: demote *Environment to interface In order to support lazily loading the values stored in a user's `.gitconfig`, we must wait until calling `*config.Configuration.loadGitConfig()` until it is _absolutely necessary_. To accomplish this, it was proposed that we introduce a wrapped variant of the `*Environment` type, only for interacting with the `GitFetcher` that was capable of supporting such beahvior. As such, a new implementation of the `Environment` type must be defined. Since previously there only existed the concrete type `*Environment`, this commit demotes that down to `*enviornment`, and introduces the interface `Environment`, which it implements. --- config/config.go | 7 ++--- config/environment.go | 64 +++++++++++++++++++++----------------- config/environment_test.go | 17 +++------- 3 files changed, 44 insertions(+), 44 deletions(-) diff --git a/config/config.go b/config/config.go index c2b7357c..370c175c 100644 --- a/config/config.go +++ b/config/config.go @@ -50,14 +50,13 @@ type Configuration struct { // Os provides a `*Environment` used to access to the system's // environment through os.Getenv. It is the point of entry for all // system environment configuration. - Os *Environment + Os Environment // Git provides a `*Environment` used to access to the various levels of // `.gitconfig`'s. It is the point of entry for all Git environment // configuration. - Git *Environment + Git Environment - // gitConfig map[string]string CurrentRemote string @@ -187,7 +186,7 @@ func (c *Configuration) Unmarshal(v interface{}) error { // both is not. // // If neither field was found, then a nil environment will be returned. -func (c *Configuration) parseTag(tag reflect.StructTag) (key string, env *Environment, err error) { +func (c *Configuration) parseTag(tag reflect.StructTag) (key string, env Environment, err error) { git, os := tag.Get("git"), tag.Get("os") if len(git) != 0 && len(os) != 0 { diff --git a/config/environment.go b/config/environment.go index 878cf500..604da378 100644 --- a/config/environment.go +++ b/config/environment.go @@ -11,33 +11,50 @@ import ( // `Environment`s are the primary way to communicate with various configuration // sources, such as the OS environment variables, the `.gitconfig`, and even // `map[string]string`s. -type Environment struct { - // Fetcher is the `Environment`'s source of data. +type Environment interface { + // Get is shorthand for calling `e.Fetcher.Get(key)`. + Get(key string) (val string, ok bool) + + // Bool returns the boolean state associated with a given key, or the + // value "def", if no value was associated. + // + // The "boolean state associated with a given key" is defined as the + // case-insensitive string comparison with the following: + // + // 1) true if... + // "true", "1", "on", "yes", or "t" + // 2) false if... + // "false", "0", "off", "no", "f", or otherwise. + Bool(key string, def bool) (val bool) + + // Int returns the int value associated with a given key, or the value + // "def", if no value was associated. + // + // To convert from a the string value attached to a given key, + // `strconv.Atoi(val)` is called. If `Atoi` returned a non-nil error, + // then the value "def" will be returned instead. + // + // Otherwise, if the value was converted `string -> int` successfully, + // then it will be returned wholesale. + Int(key string, def int) (val int) +} + +type environment struct { + // Fetcher is the `environment`'s source of data. Fetcher Fetcher } -// EnvironmentOf creates a new `*Environment` initialized with the givne +// EnvironmentOf creates a new `Environment` initialized with the givne // `Fetcher`, "f". -func EnvironmentOf(f Fetcher) *Environment { - return &Environment{f} +func EnvironmentOf(f Fetcher) Environment { + return &environment{f} } -// Get is shorthand for calling `e.Fetcher.Get(key)`. -func (e *Environment) Get(key string) (val string, ok bool) { +func (e *environment) Get(key string) (val string, ok bool) { return e.Fetcher.Get(key) } -// Bool returns the boolean state associated with a given key, or the value -// "def", if no value was associated. -// -// The "boolean state associated with a given key" is defined as the -// case-insensitive string comparison with the following: -// -// 1) true if... -// "true", "1", "on", "yes", or "t" -// 2) false if... -// "false", "0", "off", "no", "f", or otherwise. -func (e *Environment) Bool(key string, def bool) (val bool) { +func (e *environment) Bool(key string, def bool) (val bool) { s, _ := e.Fetcher.Get(key) if len(s) == 0 { return def @@ -53,16 +70,7 @@ func (e *Environment) Bool(key string, def bool) (val bool) { } } -// Int returns the int value associated with a given key, or the value "def", -// if no value was associated. -// -// To convert from a the string value attached to a given key, -// `strconv.Atoi(val)` is called. If `Atoi` returned a non-nil error, then the -// value "def" will be returned instead. -// -// Otherwise, if the value was converted `string -> int` successfully, then it -// will be returned wholesale. -func (e *Environment) Int(key string, def int) (val int) { +func (e *environment) Int(key string, def int) (val int) { s, _ := e.Fetcher.Get(key) if len(s) == 0 { return def diff --git a/config/environment_test.go b/config/environment_test.go index 3c3222ea..ca01f83d 100644 --- a/config/environment_test.go +++ b/config/environment_test.go @@ -7,13 +7,6 @@ import ( "github.com/stretchr/testify/assert" ) -func TestEnvironmentOfReturnsCorrectlyInitializedEnvironment(t *testing.T) { - fetcher := MapFetcher(map[string]string{}) - env := EnvironmentOf(fetcher) - - assert.Equal(t, fetcher, env.Fetcher) -} - func TestEnvironmentGetDelegatesToFetcher(t *testing.T) { fetcher := MapFetcher(map[string]string{ "foo": "bar", @@ -68,18 +61,18 @@ type EnvironmentConversionTestCase struct { Val string Expected interface{} - GotFn func(env *Environment, key string) interface{} + GotFn func(env Environment, key string) interface{} } var ( - GetBoolDefault = func(def bool) func(e *Environment, key string) interface{} { - return func(e *Environment, key string) interface{} { + GetBoolDefault = func(def bool) func(e Environment, key string) interface{} { + return func(e Environment, key string) interface{} { return e.Bool(key, def) } } - GetIntDefault = func(def int) func(e *Environment, key string) interface{} { - return func(e *Environment, key string) interface{} { + GetIntDefault = func(def int) func(e Environment, key string) interface{} { + return func(e Environment, key string) interface{} { return e.Int(key, def) } } From 04c27548a2f40f9af42dc399ea4f31dbca0fd416 Mon Sep 17 00:00:00 2001 From: Taylor Blau Date: Mon, 15 Aug 2016 14:48:43 -0600 Subject: [PATCH 5/7] config: introduce `*gitEnvironment` implementation To wrap the old behavior found in `*config.Configuration.loadGitConfig`, a `*gitEnvironment` implementation was introduced to wrap the behavior of another Environment wholesale while at the same time prepending a call to `loadGitConfig()`. It should be noted that in order to preserve legacy behavior with using certain member variables in `*config.Configuration`, there is a circular dependency between the two types. --- config/config.go | 53 ++++++++++++----------------- config/git_environment.go | 71 +++++++++++++++++++++++++++++++++++++++ 2 files changed, 93 insertions(+), 31 deletions(-) create mode 100644 config/git_environment.go diff --git a/config/config.go b/config/config.go index 370c175c..d4a93e6d 100644 --- a/config/config.go +++ b/config/config.go @@ -78,11 +78,11 @@ type Configuration struct { func New() *Configuration { c := &Configuration{ - Os: EnvironmentOf(NewOsFetcher()), - + Os: EnvironmentOf(NewOsFetcher()), CurrentRemote: defaultRemote, envVars: make(map[string]string), } + c.Git = &gitEnvironment{config: c} c.IsTracingHttp = c.GetenvBool("GIT_CURL_VERBOSE", false) c.IsDebuggingHttp = c.GetenvBool("LFS_DEBUG_HTTP", false) c.IsLoggingStats = c.GetenvBool("GIT_LOG_STATS", false) @@ -132,8 +132,6 @@ func NewFrom(v Values) *Configuration { // Otherwise, the field will be set to the value of calling the // appropriately-typed method on the specified environment. func (c *Configuration) Unmarshal(v interface{}) error { - c.loadGitConfig() - into := reflect.ValueOf(v) if into.Kind() != reflect.Ptr { return fmt.Errorf("lfs/config: unable to parse non-pointer type of %T", v) @@ -347,6 +345,8 @@ func (c *Configuration) EndpointAccess(e Endpoint) string { } func (c *Configuration) SetEndpointAccess(e Endpoint, authType string) { + c.loadGitConfig() + tracerx.Printf("setting repository access to %s", authType) key := fmt.Sprintf("lfs.%s.access", e.Url) @@ -369,13 +369,11 @@ func (c *Configuration) SetEndpointAccess(e Endpoint, authType string) { } func (c *Configuration) FetchIncludePaths() []string { - c.loadGitConfig() patterns, _ := c.Git.Get("lfs.fetchinclude") return tools.CleanPaths(patterns, ",") } func (c *Configuration) FetchExcludePaths() []string { - c.loadGitConfig() patterns, _ := c.Git.Get("lfs.fetchexclude") return tools.CleanPaths(patterns, ",") } @@ -405,6 +403,7 @@ func (c *Configuration) RemoteEndpoint(remote, operation string) Endpoint { func (c *Configuration) Remotes() []string { c.loadGitConfig() + return c.remotes } @@ -419,6 +418,7 @@ func (c *Configuration) GitProtocol() string { func (c *Configuration) Extensions() map[string]Extension { c.loadGitConfig() + return c.extensions } @@ -429,25 +429,22 @@ func (c *Configuration) SortedExtensions() ([]Extension, error) { // GitConfigInt parses a git config value and returns it as an integer. func (c *Configuration) GitConfigInt(key string, def int) int { - c.loadGitConfig() return c.Git.Int(strings.ToLower(key), def) } // GitConfigBool parses a git config value and returns true if defined as // true, 1, on, yes, or def if not defined func (c *Configuration) GitConfigBool(key string, def bool) bool { - c.loadGitConfig() return c.Git.Bool(strings.ToLower(key), def) } func (c *Configuration) GitConfig(key string) (string, bool) { - c.loadGitConfig() - value, ok := c.gitConfig[strings.ToLower(key)] - return value, ok + return c.Git.Get(strings.ToLower(key)) } func (c *Configuration) AllGitConfig() map[string]string { c.loadGitConfig() + return c.gitConfig } @@ -513,27 +510,21 @@ func (c *Configuration) SkipDownloadErrors() bool { return c.GetenvBool("GIT_LFS_SKIP_DOWNLOAD_ERRORS", false) || c.GitConfigBool("lfs.skipdownloaderrors", false) } +// loadGitConfig is a temporary measure to support legacy behavior dependent on +// accessing properties set by ReadGitConfig, namely: +// - `c.extensions` +// - `c.uniqRemotes` +// - `c.gitConfig` +// +// Since the *gitEnvironment is responsible for setting these values on the +// (*config.Configuration) instance, we must call that method, if it exists. +// +// loadGitConfig returns a bool returning whether or not `loadGitConfig` was +// called AND the method did not return early. func (c *Configuration) loadGitConfig() bool { - c.loading.Lock() - defer c.loading.Unlock() - - if c.Git != nil { - return false + if g, ok := c.Git.(*gitEnvironment); ok { + return g.loadGitConfig() } - gf, extensions, uniqRemotes := ReadGitConfig(getGitConfigs()...) - - c.Git = EnvironmentOf(gf) - c.gitConfig = gf.vals // XXX TERRIBLE - c.extensions = extensions - - c.remotes = make([]string, 0, len(uniqRemotes)) - for remote, isOrigin := range uniqRemotes { - if isOrigin { - continue - } - c.remotes = append(c.remotes, remote) - } - - return true + return false } diff --git a/config/git_environment.go b/config/git_environment.go new file mode 100644 index 00000000..b8560227 --- /dev/null +++ b/config/git_environment.go @@ -0,0 +1,71 @@ +package config + +// gitEnvironment is an implementation of the Environment which wraps the legacy +// behavior or `*config.Configuration.loadGitConfig()`. +// +// It is functionally equivelant to call `cfg.loadGitConfig()` before calling +// methods on the Environment type. +type gitEnvironment struct { + // git is the Environment which gitEnvironment wraps. + git Environment + // config is the *Configuration instance which is mutated by + // `loadGitConfig`. + config *Configuration +} + +// Get is shorthand for calling the loadGitConfig, and then returning +// `g.git.Get(key)`. +func (g *gitEnvironment) Get(key string) (val string, ok bool) { + g.loadGitConfig() + + return g.git.Get(key) +} + +// Get is shorthand for calling the loadGitConfig, and then returning +// `g.git.Bool(key, def)`. +func (g *gitEnvironment) Bool(key string, def bool) (val bool) { + g.loadGitConfig() + + return g.git.Bool(key, def) +} + +// Get is shorthand for calling the loadGitConfig, and then returning +// `g.git.Int(key, def)`. +func (g *gitEnvironment) Int(key string, def int) (val int) { + g.loadGitConfig() + + return g.git.Int(key, def) +} + +// loadGitConfig reads and parses the .gitconfig by calling ReadGitConfig. It +// also sets values on the configuration instance `g.config`. +// +// If loadGitConfig has already been called, this method will bail out early, +// and return false. Otherwise it will preform the entire parse and return true. +// +// loadGitConfig is safe to call across multiple goroutines. +func (g *gitEnvironment) loadGitConfig() bool { + g.config.loading.Lock() + defer g.config.loading.Unlock() + + if g.git != nil { + return false + } + + gf, extensions, uniqRemotes := ReadGitConfig(getGitConfigs()...) + + g.git = EnvironmentOf(gf) + + g.config.gitConfig = gf.vals // XXX TERRIBLE + g.config.extensions = extensions + + g.config.remotes = make([]string, 0, len(uniqRemotes)) + for remote, isOrigin := range uniqRemotes { + if isOrigin { + continue + } + g.config.remotes = append(g.config.remotes, remote) + } + + return true +} From d3bc59abd54c2481a707ecc6b8f290ef7d7913bc Mon Sep 17 00:00:00 2001 From: risk danger olson Date: Mon, 15 Aug 2016 15:43:38 -0600 Subject: [PATCH 6/7] replace `.GitConfig*` with `.Git.*` --- api/lock_api.go | 4 ++-- commands/command_env.go | 2 +- config/config.go | 41 +++++++++++++---------------------------- httputil/certs.go | 12 ++++++------ httputil/http.go | 6 +++--- httputil/proxy.go | 2 +- lfs/hook.go | 2 +- transfer/custom.go | 6 +++--- 8 files changed, 30 insertions(+), 45 deletions(-) diff --git a/api/lock_api.go b/api/lock_api.go index 472556f2..7c83ae9b 100644 --- a/api/lock_api.go +++ b/api/lock_api.go @@ -149,8 +149,8 @@ type Committer struct { // "user.name" and "user.email" configuration values are used from the // config.Config singleton. func CurrentCommitter() Committer { - name, _ := config.Config.GitConfig("user.name") - email, _ := config.Config.GitConfig("user.email") + name, _ := config.Config.Git.Get("user.name") + email, _ := config.Config.Git.Get("user.email") return Committer{name, email} } diff --git a/commands/command_env.go b/commands/command_env.go index 804beaef..1a0a2caf 100644 --- a/commands/command_env.go +++ b/commands/command_env.go @@ -40,7 +40,7 @@ func envCommand(cmd *cobra.Command, args []string) { } for _, key := range []string{"filter.lfs.smudge", "filter.lfs.clean"} { - value, _ := cfg.GitConfig(key) + value, _ := cfg.Git.Get(key) Print("git config %s = %q", key, value) } } diff --git a/config/config.go b/config/config.go index d4a93e6d..7a3600f9 100644 --- a/config/config.go +++ b/config/config.go @@ -216,12 +216,12 @@ func (c *Configuration) GetenvBool(key string, def bool) bool { // the forpush argument is to cater for separate remote.name.pushurl settings func (c *Configuration) GitRemoteUrl(remote string, forpush bool) string { if forpush { - if u, ok := c.GitConfig("remote." + remote + ".pushurl"); ok { + if u, ok := c.Git.Get("remote." + remote + ".pushurl"); ok { return u } } - if u, ok := c.GitConfig("remote." + remote + ".url"); ok { + if u, ok := c.Git.Get("remote." + remote + ".url"); ok { return u } @@ -240,12 +240,12 @@ func (c *Configuration) Endpoint(operation string) Endpoint { } if operation == "upload" { - if url, ok := c.GitConfig("lfs.pushurl"); ok { + if url, ok := c.Git.Get("lfs.pushurl"); ok { return NewEndpointWithConfig(url, c) } } - if url, ok := c.GitConfig("lfs.url"); ok { + if url, ok := c.Git.Get("lfs.url"); ok { return NewEndpointWithConfig(url, c) } @@ -265,7 +265,7 @@ func (c *Configuration) ConcurrentTransfers() int { uploads := 3 - if v, ok := c.GitConfig("lfs.concurrenttransfers"); ok { + if v, ok := c.Git.Get("lfs.concurrenttransfers"); ok { n, err := strconv.Atoi(v) if err == nil && n > 0 { uploads = n @@ -278,17 +278,17 @@ func (c *Configuration) ConcurrentTransfers() int { // BasicTransfersOnly returns whether to only allow "basic" HTTP transfers. // Default is false, including if the lfs.basictransfersonly is invalid func (c *Configuration) BasicTransfersOnly() bool { - return c.GitConfigBool("lfs.basictransfersonly", false) + return c.Git.Bool("lfs.basictransfersonly", false) } // TusTransfersAllowed returns whether to only use "tus.io" HTTP transfers. // Default is false, including if the lfs.tustransfers is invalid func (c *Configuration) TusTransfersAllowed() bool { - return c.GitConfigBool("lfs.tustransfers", false) + return c.Git.Bool("lfs.tustransfers", false) } func (c *Configuration) BatchTransfer() bool { - return c.GitConfigBool("lfs.batch", true) + return c.Git.Bool("lfs.batch", true) } func (c *Configuration) NtlmAccess(operation string) bool { @@ -334,7 +334,7 @@ func (c *Configuration) SetNetrc(n netrcfinder) { func (c *Configuration) EndpointAccess(e Endpoint) string { key := fmt.Sprintf("lfs.%s.access", e.Url) - if v, ok := c.GitConfig(key); ok && len(v) > 0 { + if v, ok := c.Git.Get(key); ok && len(v) > 0 { lower := strings.ToLower(v) if lower == "private" { return "basic" @@ -385,11 +385,11 @@ func (c *Configuration) RemoteEndpoint(remote, operation string) Endpoint { // Support separate push URL if specified and pushing if operation == "upload" { - if url, ok := c.GitConfig("remote." + remote + ".lfspushurl"); ok { + if url, ok := c.Git.Get("remote." + remote + ".lfspushurl"); ok { return NewEndpointWithConfig(url, c) } } - if url, ok := c.GitConfig("remote." + remote + ".lfsurl"); ok { + if url, ok := c.Git.Get("remote." + remote + ".lfsurl"); ok { return NewEndpointWithConfig(url, c) } @@ -410,7 +410,7 @@ func (c *Configuration) Remotes() []string { // GitProtocol returns the protocol for the LFS API when converting from a // git:// remote url. func (c *Configuration) GitProtocol() string { - if value, ok := c.GitConfig("lfs.gitprotocol"); ok { + if value, ok := c.Git.Get("lfs.gitprotocol"); ok { return value } return "https" @@ -427,21 +427,6 @@ func (c *Configuration) SortedExtensions() ([]Extension, error) { return SortExtensions(c.Extensions()) } -// GitConfigInt parses a git config value and returns it as an integer. -func (c *Configuration) GitConfigInt(key string, def int) int { - return c.Git.Int(strings.ToLower(key), def) -} - -// GitConfigBool parses a git config value and returns true if defined as -// true, 1, on, yes, or def if not defined -func (c *Configuration) GitConfigBool(key string, def bool) bool { - return c.Git.Bool(strings.ToLower(key), def) -} - -func (c *Configuration) GitConfig(key string) (string, bool) { - return c.Git.Get(strings.ToLower(key)) -} - func (c *Configuration) AllGitConfig() map[string]string { c.loadGitConfig() @@ -507,7 +492,7 @@ func (c *Configuration) FetchPruneConfig() FetchPruneConfig { } func (c *Configuration) SkipDownloadErrors() bool { - return c.GetenvBool("GIT_LFS_SKIP_DOWNLOAD_ERRORS", false) || c.GitConfigBool("lfs.skipdownloaderrors", false) + return c.GetenvBool("GIT_LFS_SKIP_DOWNLOAD_ERRORS", false) || c.Git.Bool("lfs.skipdownloaderrors", false) } // loadGitConfig is a temporary measure to support legacy behavior dependent on diff --git a/httputil/certs.go b/httputil/certs.go index bf8feca8..0605355d 100644 --- a/httputil/certs.go +++ b/httputil/certs.go @@ -13,12 +13,12 @@ import ( // isCertVerificationDisabledForHost returns whether SSL certificate verification // has been disabled for the given host, or globally func isCertVerificationDisabledForHost(cfg *config.Configuration, host string) bool { - hostSslVerify, _ := cfg.GitConfig(fmt.Sprintf("http.https://%v/.sslverify", host)) + hostSslVerify, _ := cfg.Git.Get(fmt.Sprintf("http.https://%v/.sslverify", host)) if hostSslVerify == "false" { return true } - globalSslVerify, _ := cfg.GitConfig("http.sslverify") + globalSslVerify, _ := cfg.Git.Get("http.sslverify") if globalSslVerify == "false" || cfg.GetenvBool("GIT_SSL_NO_VERIFY", false) { return true } @@ -54,15 +54,15 @@ func appendRootCAsForHostFromGitconfig(cfg *config.Configuration, pool *x509.Cer // http./.sslcainfo or http..sslcainfo // we know we have simply "host" or "host:port" hostKeyWithSlash := fmt.Sprintf("http.https://%v/.sslcainfo", host) - if cafile, ok := cfg.GitConfig(hostKeyWithSlash); ok { + if cafile, ok := cfg.Git.Get(hostKeyWithSlash); ok { return appendCertsFromFile(pool, cafile) } hostKeyWithoutSlash := fmt.Sprintf("http.https://%v.sslcainfo", host) - if cafile, ok := cfg.GitConfig(hostKeyWithoutSlash); ok { + if cafile, ok := cfg.Git.Get(hostKeyWithoutSlash); ok { return appendCertsFromFile(pool, cafile) } // http.sslcainfo - if cafile, ok := cfg.GitConfig("http.sslcainfo"); ok { + if cafile, ok := cfg.Git.Get("http.sslcainfo"); ok { return appendCertsFromFile(pool, cafile) } // GIT_SSL_CAPATH @@ -70,7 +70,7 @@ func appendRootCAsForHostFromGitconfig(cfg *config.Configuration, pool *x509.Cer return appendCertsFromFilesInDir(pool, cadir) } // http.sslcapath - if cadir, ok := cfg.GitConfig("http.sslcapath"); ok { + if cadir, ok := cfg.Git.Get("http.sslcapath"); ok { return appendCertsFromFilesInDir(pool, cadir) } diff --git a/httputil/http.go b/httputil/http.go index b5a37699..85af68e6 100644 --- a/httputil/http.go +++ b/httputil/http.go @@ -117,9 +117,9 @@ func NewHttpClient(c *config.Configuration, host string) *HttpClient { return client } - dialtime := c.GitConfigInt("lfs.dialtimeout", 30) - keepalivetime := c.GitConfigInt("lfs.keepalive", 1800) // 30 minutes - tlstime := c.GitConfigInt("lfs.tlstimeout", 30) + dialtime := c.Git.Int("lfs.dialtimeout", 30) + keepalivetime := c.Git.Int("lfs.keepalive", 1800) // 30 minutes + tlstime := c.Git.Int("lfs.tlstimeout", 30) tr := &http.Transport{ Proxy: ProxyFromGitConfigOrEnvironment(c), diff --git a/httputil/proxy.go b/httputil/proxy.go index 54315700..b5ba40f5 100644 --- a/httputil/proxy.go +++ b/httputil/proxy.go @@ -14,7 +14,7 @@ import ( // Logic is copied, with small changes, from "net/http".ProxyFromEnvironment in the go std lib. func ProxyFromGitConfigOrEnvironment(c *config.Configuration) func(req *http.Request) (*url.URL, error) { var https_proxy string - http_proxy, _ := c.GitConfig("http.proxy") + http_proxy, _ := c.Git.Get("http.proxy") if strings.HasPrefix(http_proxy, "https://") { https_proxy = http_proxy } diff --git a/lfs/hook.go b/lfs/hook.go index 1c42ad4e..bd6d8793 100644 --- a/lfs/hook.go +++ b/lfs/hook.go @@ -39,7 +39,7 @@ func (h *Hook) Path() string { // greater than "2.9.0"), it will return that instead. func (h *Hook) Dir() string { customHooksSupported := git.Config.IsGitVersionAtLeast("2.9.0") - if hp, ok := config.Config.GitConfig("core.hooksPath"); ok && customHooksSupported { + if hp, ok := config.Config.Git.Get("core.hooksPath"); ok && customHooksSupported { return hp } diff --git a/transfer/custom.go b/transfer/custom.go index 210dc2c6..c6abf6e5 100644 --- a/transfer/custom.go +++ b/transfer/custom.go @@ -358,9 +358,9 @@ func configureCustomAdapters(cfg *config.Configuration, m *Manifest) { name := match[1] path := v // retrieve other values - args, _ := cfg.GitConfig(fmt.Sprintf("lfs.customtransfer.%s.args", name)) - concurrent := cfg.GitConfigBool(fmt.Sprintf("lfs.customtransfer.%s.concurrent", name), true) - direction, _ := cfg.GitConfig(fmt.Sprintf("lfs.customtransfer.%s.direction", name)) + args, _ := cfg.Git.Get(fmt.Sprintf("lfs.customtransfer.%s.args", name)) + concurrent := cfg.Git.Bool(fmt.Sprintf("lfs.customtransfer.%s.concurrent", name), true) + direction, _ := cfg.Git.Get(fmt.Sprintf("lfs.customtransfer.%s.direction", name)) if len(direction) == 0 { direction = "both" } else { From 14055fcc648f226df67dbda59c3fe26e39d4d390 Mon Sep 17 00:00:00 2001 From: Taylor Blau Date: Mon, 15 Aug 2016 23:23:55 -0600 Subject: [PATCH 7/7] config: make GitFetcher lookups case-insensitive --- config/git_fetcher.go | 10 +++++++++- 1 file changed, 9 insertions(+), 1 deletion(-) diff --git a/config/git_fetcher.go b/config/git_fetcher.go index cdb54cfd..e19e0f0a 100644 --- a/config/git_fetcher.go +++ b/config/git_fetcher.go @@ -110,11 +110,19 @@ func ReadGitConfig(configs ...*GitConfig) (gf *GitFetcher, extensions map[string return } +// Get implements the Fetcher interface, and returns the value associated with +// a given key and true, signaling that the value was present. Otherwise, an +// empty string and false will be returned, signaling that the value was +// absent. +// +// Map lookup by key is case-insensitive, as per the .gitconfig specification. +// +// Get is safe to call across multiple goroutines. func (g *GitFetcher) Get(key string) (val string, ok bool) { g.vmu.RLock() defer g.vmu.RUnlock() - val, ok = g.vals[key] + val, ok = g.vals[strings.ToLower(key)] return }