Skip to content

Commit

Permalink
locks: cache repository metadata
Browse files Browse the repository at this point in the history
When we lock or unlock multiple files, we compute the same data for each
path, resulting in possibly calling multiple different commands.  The
repository on which we're working is not going to change while the
command is running (and if it does get moved or removed, then dying is
fine), so let's cache this data to try to avoid the expense of calling
`git rev-parse` command multiple times.

Make some changes here to use the existing values in the config object
instead of calling the `git` package ourselves.  That will compute both
of these values once instead of recomputing them again and again, and
also moves the error handling out of this codepath.  In addition, move
these calls up to the top of their respective functions so that we can
cache these values before further calls, eliminating more useless `git
rev-parse` invocations.
  • Loading branch information
bk2204 committed Nov 27, 2023
1 parent df9b79d commit 86f0826
Show file tree
Hide file tree
Showing 3 changed files with 49 additions and 27 deletions.
59 changes: 36 additions & 23 deletions commands/command_lock.go
Original file line number Diff line number Diff line change
Expand Up @@ -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()
Expand All @@ -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
Expand Down Expand Up @@ -67,49 +72,57 @@ 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.
//
// 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
}
Expand Down
10 changes: 7 additions & 3 deletions commands/command_locks.go
Original file line number Diff line number Diff line change
Expand Up @@ -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))
}
Expand Down Expand Up @@ -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
}
Expand Down
7 changes: 6 additions & 1 deletion commands/command_unlock.go
Original file line number Diff line number Diff line change
Expand Up @@ -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()
Expand All @@ -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())))
Expand Down

0 comments on commit 86f0826

Please sign in to comment.