Merge pull request #5561 from bk2204/locking-performance

Improve locking performance

commands/command_lock.go

@@ -23,6 +23,11 @@ func lockCommand(cmd *cobra.Command, args []string) {
 		cfg.SetRemote(lockRemote)
 	}
 
+	lockData, err := computeLockData()
+	if err != nil {
+		ExitWithError(err)
+	}
+
 	refUpdate := git.NewRefUpdate(cfg.Git, cfg.PushRemote(), cfg.CurrentRef(), nil)
 	lockClient := newLockClient()
 	lockClient.RemoteRef = refUpdate.RemoteRef()
@@ -31,7 +36,7 @@ func lockCommand(cmd *cobra.Command, args []string) {
 	success := true
 	locks := make([]locking.Lock, 0, len(args))
 	for _, path := range args {
-		path, err := lockPath(path)
+		path, err := lockPath(lockData, path)
 		if err != nil {
 			Error(err.Error())
 			success = false
@@ -67,6 +72,28 @@ func lockCommand(cmd *cobra.Command, args []string) {
 	}
 }
 
+type lockData struct {
+	rootDir    string
+	workingDir string
+}
+
+// computeLockData computes data about the given repository and working
+// directory to use in lockPath.
+func computeLockData() (*lockData, error) {
+	wd, err := tools.Getwd()
+	if err != nil {
+		return nil, err
+	}
+	wd, err = tools.CanonicalizeSystemPath(wd)
+	if err != nil {
+		return nil, err
+	}
+	return &lockData{
+		rootDir:    cfg.LocalWorkingDir(),
+		workingDir: wd,
+	}, nil
+}
+
 // lockPaths relativizes the given filepath such that it is relative to the root
 // path of the repository it is contained within, taking into account the
 // working directory of the caller.
@@ -74,42 +101,28 @@ func lockCommand(cmd *cobra.Command, args []string) {
 // lockPaths also respects different filesystem directory separators, so that a
 // Windows path of "\foo\bar" will be normalized to "foo/bar".
 //
-// If the root directory, working directory, or file cannot be
-// determined, an error will be returned. If the file in question is
-// actually a directory, an error will be returned. Otherwise, the cleaned path
-// will be returned.
+// If the file path cannot be determined, an error will be returned. If the file
+// in question is actually a directory, an error will be returned. Otherwise,
+// the cleaned path will be returned.
 //
 // For example:
 //   - Working directory: /code/foo/bar/
 //   - Repository root: /code/foo/
 //   - File to lock: ./baz
 //   - Resolved path bar/baz
-func lockPath(file string) (string, error) {
-	repo, err := git.RootDir()
-	if err != nil {
-		return "", err
-	}
-
-	wd, err := os.Getwd()
-	if err != nil {
-		return "", err
-	}
-	wd, err = tools.CanonicalizeSystemPath(wd)
-	if err != nil {
-		return "", errors.Wrapf(err,
-			tr.Tr.Get("could not follow symlinks for %s", wd))
-	}
-
+func lockPath(data *lockData, file string) (string, error) {
 	var abs string
+	var err error
+
 	if filepath.IsAbs(file) {
 		abs, err = tools.CanonicalizeSystemPath(file)
 		if err != nil {
 			return "", errors.New(tr.Tr.Get("unable to canonicalize path %q: %v", file, err))
 		}
 	} else {
-		abs = filepath.Join(wd, file)
+		abs = filepath.Join(data.workingDir, file)
 	}
-	path, err := filepath.Rel(repo, abs)
+	path, err := filepath.Rel(data.rootDir, abs)
 	if err != nil {
 		return "", err
 	}

commands/command_locks.go

@@ -19,7 +19,11 @@ var (
 )
 
 func locksCommand(cmd *cobra.Command, args []string) {
-	filters, err := locksCmdFlags.Filters()
+	lockData, err := computeLockData()
+	if err != nil {
+		ExitWithError(err)
+	}
+	filters, err := locksCmdFlags.Filters(lockData)
 	if err != nil {
 		Exit(tr.Tr.Get("Error building filters: %v", err))
 	}
@@ -154,11 +158,11 @@ type locksFlags struct {
 }
 
 // Filters produces a filter based on locksFlags instance.
-func (l *locksFlags) Filters() (map[string]string, error) {
+func (l *locksFlags) Filters(data *lockData) (map[string]string, error) {
 	filters := make(map[string]string)
 
 	if l.Path != "" {
-		path, err := lockPath(l.Path)
+		path, err := lockPath(data, l.Path)
 		if err != nil {
 			return nil, err
 		}

commands/command_unlock.go

@@ -58,6 +58,11 @@ func unlockCommand(cmd *cobra.Command, args []string) {
 		cfg.SetRemote(lockRemote)
 	}
 
+	lockData, err := computeLockData()
+	if err != nil {
+		ExitWithError(err)
+	}
+
 	refUpdate := git.NewRefUpdate(cfg.Git, cfg.PushRemote(), cfg.CurrentRef(), nil)
 	lockClient := newLockClient()
 	lockClient.RemoteRef = refUpdate.RemoteRef()
@@ -67,7 +72,7 @@ func unlockCommand(cmd *cobra.Command, args []string) {
 	success := true
 	if hasPath {
 		for _, pathspec := range args {
-			path, err := lockPath(pathspec)
+			path, err := lockPath(lockData, pathspec)
 			if err != nil {
 				if !unlockCmdFlags.Force {
 					locks = handleUnlockError(locks, "", path, errors.New(tr.Tr.Get("Unable to determine path: %v", err.Error())))

git/git.go

@@ -531,13 +531,20 @@ func ValidateRemote(remote string) error {
 	if err != nil {
 		return err
 	}
+	return ValidateRemoteFromList(remotes, remote)
+}
+
+// ValidateRemote checks that a named remote is valid for use given a list from
+// RemoteList.  This is completely identical to ValidateRemote, except that it
+// allows caching the remote list.
+func ValidateRemoteFromList(remotes []string, remote string) error {
 	for _, r := range remotes {
 		if r == remote {
 			return nil
 		}
 	}
 
-	if err = ValidateRemoteURL(remote); err == nil {
+	if err := ValidateRemoteURL(remote); err == nil {
 		return nil
 	}
 

lfsapi/endpoint_finder.go

@@ -37,10 +37,12 @@ type endpointGitFinder struct {
 	gitConfig   *git.Configuration
 	gitEnv      config.Environment
 	gitProtocol string
+	gitDir      string
 
 	aliasMu     sync.Mutex
 	aliases     map[string]string
 	pushAliases map[string]string
+	remoteList  []string
 
 	accessMu  sync.Mutex
 	urlAccess map[string]creds.AccessMode
@@ -52,15 +54,27 @@ func NewEndpointFinder(ctx lfshttp.Context) EndpointFinder {
 		ctx = lfshttp.NewContext(nil, nil, nil)
 	}
 
+	var gitDir string
+	cfg := ctx.GitConfig()
+	if cfg != nil && cfg.GitDir != "" {
+		gitDir = cfg.GitDir
+	} else if dir, err := git.GitDir(); err == nil {
+		gitDir = dir
+	}
+
 	e := &endpointGitFinder{
 		gitConfig:   ctx.GitConfig(),
 		gitEnv:      ctx.GitEnv(),
 		gitProtocol: "https",
+		gitDir:      gitDir,
 		aliases:     make(map[string]string),
 		pushAliases: make(map[string]string),
 		urlAccess:   make(map[string]creds.AccessMode),
 	}
 
+	remotes, _ := git.RemoteList()
+	e.remoteList = remotes
+
 	e.urlConfig = config.NewURLConfig(e.gitEnv)
 	if v, ok := e.gitEnv.Get("lfs.gitprotocol"); ok {
 		e.gitProtocol = v
@@ -124,11 +138,10 @@ func (e *endpointGitFinder) RemoteEndpoint(operation, remote string) lfshttp.End
 		return e.NewEndpointFromCloneURL(operation, url)
 	}
 
-	gitDir, err := git.GitDir()
 	// Finally, fall back on .git/FETCH_HEAD but only if it exists and no specific remote was requested
 	// We can't know which remote FETCH_HEAD is pointing to
-	if err == nil && remote == defaultRemote {
-		url, err := parseFetchHead(strings.Join([]string{gitDir, "FETCH_HEAD"}, "/"))
+	if e.gitDir != "" && remote == defaultRemote {
+		url, err := parseFetchHead(strings.Join([]string{e.gitDir, "FETCH_HEAD"}, "/"))
 		if err == nil {
 			endpoint := e.NewEndpointFromCloneURL("download", url)
 			return endpoint
@@ -187,7 +200,7 @@ func (e *endpointGitFinder) GitRemoteURL(remote string, forpush bool) string {
 		}
 	}
 
-	if err := git.ValidateRemote(remote); err == nil {
+	if err := git.ValidateRemoteFromList(e.remoteList, remote); err == nil {
 		return remote
 	}
 

locking/locks.go

@@ -67,6 +67,8 @@ func NewClient(remote string, lfsClient *lfsapi.Client, cfg *config.Configuratio
 		cache:              &nilLockCacher{},
 		cfg:                cfg,
 		ModifyIgnoredFiles: lfsClient.GitEnv().Bool("lfs.lockignoredfiles", false),
+		LocalWorkingDir:    cfg.LocalWorkingDir(),
+		LocalGitDir:        cfg.LocalGitDir(),
 	}, nil
 }
 
@@ -120,7 +122,7 @@ func (c *Client) LockFile(path string) (Lock, error) {
 		return Lock{}, errors.Wrap(err, tr.Tr.Get("lock cache"))
 	}
 
-	abs, err := getAbsolutePath(path)
+	abs, err := c.getAbsolutePath(path)
 	if err != nil {
 		return Lock{}, errors.Wrap(err, tr.Tr.Get("make lock path absolute"))
 	}
@@ -141,13 +143,8 @@ func (c *Client) LockFile(path string) (Lock, error) {
 // dir/foo/bar.txt, getAbsolutePath will return:
 //
 //	/usr/local/src/my-repo/dir/foo/bar.txt
-func getAbsolutePath(p string) (string, error) {
-	root, err := git.RootDir()
-	if err != nil {
-		return "", err
-	}
-
-	return filepath.Join(root, p), nil
+func (c *Client) getAbsolutePath(p string) (string, error) {
+	return filepath.Join(c.LocalWorkingDir, p), nil
 }
 
 // UnlockFile attempts to unlock a file on the current remote
@@ -182,7 +179,7 @@ func (c *Client) UnlockFileById(id string, force bool) error {
 	}
 
 	if unlockRes.Lock != nil {
-		abs, err := getAbsolutePath(unlockRes.Lock.Path)
+		abs, err := c.getAbsolutePath(unlockRes.Lock.Path)
 		if err != nil {
 			return errors.Wrap(err, tr.Tr.Get("make lock path absolute"))
 		}