Skip to content

Commit

Permalink
commands,git: check for prunable worktrees
Browse files Browse the repository at this point in the history
The GetAllWorktrees() function in our "git" package was introduced as
part of the implementation of the "git lfs prune" command in PR git-lfs#742.
This function examines the contents of a repository's ".git/worktrees"
folder and returns an array of Worktree structures for any linked
working trees it finds, plus one Worktree structure for the main
working tree, if one exists.  The "git lfs prune" command then checks
these working trees' current Git references and indexes for any Git LFS
objects it should retain in the Git LFS object cache.

In subsequent commits in this PR we expect to add an alternative
implementation of the GetAllWorktrees() function which uses the
"git worktree list" command, as this will ensure we remain compatible
with some forthcoming changes in Git that will alter the contents
of the "gitdir" files in ".git/worktrees" hierarchy.  We will need
to retain the legacy implementation of the GetAllWorktrees() function
as well, as prior to Git version 2.36.0 the "git worktree list" command
is either not available or does not support the full set of options we
will require.

The "git worktree list" command reports when a linked working tree
no longer exists and so its data in the ".git/worktrees" hierarchy
could be pruned with the "git worktree prune" command.  Our new
implementation of the GetAllWorktrees() function will therefore be
able to return this prunable state as a flag in the Worktree structure.

The pruneTaskGetRetainedIndex() function run by our "git lfs prune"
command calls the ScanIndex() method of the GitScanner structure in
our "lfs" package, whose internal functions then create two
DiffIndexScanner structures and invoke their Scan() methods repeatedly.
That structure's initialization function, NewDiffIndexScanner(),
uses the DiffIndex() function in our "git" package to start a
"git diff-index" command and pipe its output into a buffer, which
is then consumed by the DiffIndexScanner's Scan() method.

As we run these "git diff-index" commands in each linked working
tree, if it no longer actually exists, the commands simply fail.
We silently ignore these failures because the DiffIndex() function
uses the gitBufferedStdout() function to start the command, and
that discards the stderr stream, we never call the Wait() method
of the underlying Cmd structure from the "os/exec" package, so we
also discard the exit code from the "git diff-index" command.

However, we can avoid running unnecessary "git diff-index" commands
entirely if we detect that a linked working tree no longer exists.
As this will be straightforward in our new implementation of the
GetAllWorktrees() function, we first update our existing version
of the function to perform a check similar to one made by Git,
and return a true flag value in our Worktree structure if the
working tree is missing.

We add a Prunable flag to the Worktree structure, and check it
in the pruneTaskGetRetainedWorktree() function so we only run the
pruneTaskGetRetainedIndex() function if a linked working tree's
Prunable flag is false.

We then expand our existing TestWorktrees() test function to
validate that when a linked working tree is removed, the Prunable
flag is set as we expect.

Note that our "prune worktree" test in the t/t-prune-worktree.sh test
script already performs checks of our "git lfs prune" command after
a linked working tree is removed.  These checks pass without the
changes in this commit because the "git diff-index" commands we try to
run in the working tree simply fail, and we then ignore those failures.
With the changes in this commit, the behaviour of the "git lfs prune"
command remains the same, but it no longer tries to run "git diff-index"
in non-extant working trees.
  • Loading branch information
chrisd8088 committed Oct 28, 2024
1 parent 54d4358 commit 86d727f
Show file tree
Hide file tree
Showing 3 changed files with 57 additions and 12 deletions.
8 changes: 5 additions & 3 deletions commands/command_prune.go
Original file line number Diff line number Diff line change
Expand Up @@ -536,9 +536,11 @@ func pruneTaskGetRetainedWorktree(gitscanner *lfs.GitScanner, fetchconf lfs.Fetc
go pruneTaskGetRetainedAtRef(gitscanner, worktree.Ref.Sha, retainChan, errorChan, waitg, sem)
}

// Always scan the index of the worktree
waitg.Add(1)
go pruneTaskGetRetainedIndex(gitscanner, worktree.Ref.Sha, worktree.Dir, retainChan, errorChan, waitg, sem)
if !worktree.Prunable {
// Always scan the index of the worktree if it exists
waitg.Add(1)
go pruneTaskGetRetainedIndex(gitscanner, worktree.Ref.Sha, worktree.Dir, retainChan, errorChan, waitg, sem)
}
}
}

Expand Down
30 changes: 25 additions & 5 deletions git/git.go
Original file line number Diff line number Diff line change
Expand Up @@ -902,10 +902,11 @@ func GitCommonDir() (string, error) {
return tools.CanonicalizePath(path, false)
}

// A git worktree (ref + path)
// A git worktree (ref + path + flags)
type Worktree struct {
Ref Ref
Dir string
Ref Ref
Dir string
Prunable bool
}

// GetAllWorktrees returns the refs that all worktrees are using as HEADs plus the worktree's path.
Expand Down Expand Up @@ -947,7 +948,22 @@ func GetAllWorktrees(storageDir string) ([]*Worktree, error) {
continue
}

worktrees = append(worktrees, &Worktree{*ref, filepath.Dir(dir)})
// Check if the worktree exists.
dir = filepath.Dir(dir)
var prunable bool
if _, err := os.Stat(dir); err != nil {
if os.IsNotExist(err) {
prunable = true
} else {
tracerx.Printf("Error checking worktree directory %s: %v", dir, err)
}
}

worktrees = append(worktrees, &Worktree{
Ref: *ref,
Dir: dir,
Prunable: prunable,
})
}
}
}
Expand All @@ -961,7 +977,11 @@ func GetAllWorktrees(storageDir string) ([]*Worktree, error) {
if err == nil {
dir, err := RootDir()
if err == nil {
worktrees = append(worktrees, &Worktree{*ref, dir})
worktrees = append(worktrees, &Worktree{
Ref: *ref,
Dir: dir,
Prunable: false,
})
} else { // ok if not exists, probably bare repo
tracerx.Printf("Error getting toplevel for main checkout, skipping: %v", err)
}
Expand Down
31 changes: 27 additions & 4 deletions git/git_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -381,6 +381,13 @@ func TestWorktrees(t *testing.T) {
{Filename: "file1.txt", Size: 40},
},
},
{ // 5
NewBranch: "branch5",
ParentBranches: []string{"master"}, // back on master
Files: []*test.FileInput{
{Filename: "file1.txt", Size: 50},
},
},
}
outputs := repo.AddCommits(inputs)
// Checkout master again otherwise can't create a worktree from branch4 if we're on it here
Expand All @@ -392,6 +399,9 @@ func TestWorktrees(t *testing.T) {
test.RunGitCommand(t, true, "worktree", "add", "tag1_wt", "tag1")
test.RunGitCommand(t, true, "worktree", "add", "branch2_wt", "branch2")
test.RunGitCommand(t, true, "worktree", "add", "branch4_wt", "branch4")
test.RunGitCommand(t, true, "worktree", "add", "branch5_wt", "branch5")

os.RemoveAll(filepath.Join(repoDir, "branch5_wt"))

worktrees, err := GetAllWorktrees(filepath.Join(repo.Path, ".git"))
assert.NoError(t, err)
Expand All @@ -402,31 +412,44 @@ func TestWorktrees(t *testing.T) {
Type: RefTypeOther,
Sha: outputs[0].Sha,
},
Dir: filepath.Join(repoDir, "tag1_wt"),
Dir: filepath.Join(repoDir, "tag1_wt"),
Prunable: false,
},
{
Ref: Ref{
Name: "master",
Type: RefTypeLocalBranch,
Sha: outputs[1].Sha,
},
Dir: repoDir,
Dir: repoDir,
Prunable: false,
},
{
Ref: Ref{
Name: "branch2",
Type: RefTypeLocalBranch,
Sha: outputs[2].Sha,
},
Dir: filepath.Join(repoDir, "branch2_wt"),
Dir: filepath.Join(repoDir, "branch2_wt"),
Prunable: false,
},
{
Ref: Ref{
Name: "branch4",
Type: RefTypeLocalBranch,
Sha: outputs[4].Sha,
},
Dir: filepath.Join(repoDir, "branch4_wt"),
Dir: filepath.Join(repoDir, "branch4_wt"),
Prunable: false,
},
{
Ref: Ref{
Name: "branch5",
Type: RefTypeLocalBranch,
Sha: outputs[5].Sha,
},
Dir: filepath.Join(repoDir, "branch5_wt"),
Prunable: true,
},
}
// Need to sort for consistent comparison
Expand Down

0 comments on commit 86d727f

Please sign in to comment.