Skip to content

Commit 4ff0308

Browse files
chrisd8088chrisd8088
committed
commands,lfs,t: scan refs by tree when pruning
In commit d2221dc of PR git-lfs#2851 the "git lfs prune" command was changed to respect the "lfs.fetchexclude" configuration option such that objects would always be pruned if they were referenced by files whose paths matched one of the patterns in the configuration option (unless they were referenced by an unpushed commit). However, this filter is applied using the GitScanner.ScanRef() method, which indirectly utilizes the internal scanRefsToChan() function, and that function only visits unique OIDs a single time each, even if they are referenced by multiple tree entries (i.e., if there are multiple files with the same content). This means that if an LFS object appears in both a file that matches a pattern from "lfs.fetchexclude" and in a file that does not match, the object may be pruned if the file path seen during the scan is the matching one regardless of whether the non-matching file would otherwise have its object retained. To resolve this we change the pruneTaskGetRetainedAtRef() function to use the GitScanner.ScanTree() method instead of ScanRef(), because ScanTree() visits all file paths in each commit. We need to pass our callback to the ScanTree() method so that we can save all non-matching files' OIDs into our list of OIDs to be retained; therefore we need to add a callback argument to ScanTree() in the same manner as is done for ScanRef() and various other GitScanner methods. We also introduce additional checks in our "prune all excluded paths" test to ensure that we always retain objects when they appear in a commit to be retained and at least one of the files referencing that object ID does not match the "lfs.fetchexclude" filter.
1 parent 0df26c1 commit 4ff0308

File tree

8 files changed

+32
-10
lines changed

8 files changed

+32
-10
lines changed

‎commands/command_checkout.go‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -73,7 +73,7 @@ func checkoutCommand(cmd *cobra.Command, args []string) {
7373

7474
chgitscanner.Filter = filepathfilter.New(rootedPaths(args), nil, filepathfilter.GitIgnore)
7575

76-
if err := chgitscanner.ScanTree(ref.Sha); err != nil {
76+
if err := chgitscanner.ScanTree(ref.Sha, nil); err != nil {
7777
ExitWithError(err)
7878
}
7979
chgitscanner.Close()

‎commands/command_dedup.go‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -87,7 +87,7 @@ func dedupCommand(cmd *cobra.Command, args []string) {
8787
})
8888
defer gitScanner.Close()
8989

90-
if err := gitScanner.ScanTree("HEAD"); err != nil {
90+
if err := gitScanner.ScanTree("HEAD", nil); err != nil {
9191
ExitWithError(err)
9292
}
9393

‎commands/command_fetch.go‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -135,7 +135,7 @@ func pointersToFetchForRef(ref string, filter *filepathfilter.Filter) ([]*lfs.Wr
135135

136136
tempgitscanner.Filter = filter
137137

138-
if err := tempgitscanner.ScanTree(ref); err != nil {
138+
if err := tempgitscanner.ScanTree(ref, nil); err != nil {
139139
return nil, err
140140
}
141141

‎commands/command_ls_files.go‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -137,7 +137,7 @@ func lsFilesCommand(cmd *cobra.Command, args []string) {
137137
} else if scanRange {
138138
err = gitscanner.ScanRefRange(includeRef, ref, nil)
139139
} else {
140-
err = gitscanner.ScanTree(ref)
140+
err = gitscanner.ScanTree(ref, nil)
141141
}
142142

143143
if err != nil {

‎commands/command_prune.go‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -352,7 +352,7 @@ func pruneTaskGetRetainedAtRef(gitscanner *lfs.GitScanner, ref string, retainCha
352352
defer sem.Release(1)
353353
defer waitg.Done()
354354

355-
err := gitscanner.ScanRef(ref, func(p *lfs.WrappedPointer, err error) {
355+
err := gitscanner.ScanTree(ref, func(p *lfs.WrappedPointer, err error) {
356356
if err != nil {
357357
errorChan <- err
358358
return

‎commands/command_pull.go‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -87,7 +87,7 @@ func pull(filter *filepathfilter.Filter) {
8787
}()
8888

8989
processQueue := time.Now()
90-
if err := gitscanner.ScanTree(ref.Sha); err != nil {
90+
if err := gitscanner.ScanTree(ref.Sha, nil); err != nil {
9191
singleCheckout.Close()
9292
ExitWithError(err)
9393
}

‎lfs/gitscanner.go‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -208,8 +208,8 @@ func (s *GitScanner) ScanAll(cb GitScannerFoundPointer) error {
208208
// ScanTree takes a ref and returns WrappedPointer objects in the tree at that
209209
// ref. Differs from ScanRefs in that multiple files in the tree with the same
210210
// content are all reported.
211-
func (s *GitScanner) ScanTree(ref string) error {
212-
callback, err := firstGitScannerCallback(s.FoundPointer)
211+
func (s *GitScanner) ScanTree(ref string, cb GitScannerFoundPointer) error {
212+
callback, err := firstGitScannerCallback(cb, s.FoundPointer)
213213
if err != nil {
214214
return err
215215
}

‎t/t-prune.sh‎

Lines changed: 24 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -112,10 +112,14 @@ begin_test "prune all excluded paths"
112112
content_oldandexcluded="To delete: pushed and too old and excluded by filter"
113113
content_oldandunchanged="Keep: pushed and created a while ago, but still current"
114114
content_prevandexcluded="To delete: pushed and in previous commit to HEAD but excluded by filter"
115+
content_unreferencedandexcluded="To delete: unreferenced in deleted branch and pushed and excluded by filter"
116+
content_includedandexcluded="Keep: pushed and both excluded by filter and included by another path"
115117
content_excluded="To delete: pushed and in HEAD but excluded by filter"
116118
oid_oldandexcluded=$(calc_oid "$content_oldandexcluded")
117119
oid_oldandunchanged=$(calc_oid "$content_oldandunchanged")
118120
oid_prevandexcluded=$(calc_oid "$content_prevandexcluded")
121+
oid_unreferencedandexcluded=$(calc_oid "$content_unreferencedandexcluded")
122+
oid_includedandexcluded=$(calc_oid "$content_includedandexcluded")
119123
oid_excluded=$(calc_oid "$content_excluded")
120124

121125
echo "[
@@ -131,12 +135,23 @@ begin_test "prune all excluded paths"
131135
{\"Filename\":\"foo/oldandexcluded.dat\",\"Size\":${#content_prevandexcluded}, \"Data\":\"$content_prevandexcluded\"}]
132136
},
133137
{
138+
\"CommitDate\":\"$(get_date -4d)\",
139+
\"NewBranch\":\"branch_to_delete\",
140+
\"Files\":[
141+
{\"Filename\":\"unreferenced.dat\",\"Size\":${#content_unreferencedandexcluded}, \"Data\":\"$content_unreferencedandexcluded\"}]
142+
},
143+
{
144+
\"ParentBranches\":[\"main\"],
134145
\"Files\":[
146+
{\"Filename\":\"foo/unreferencedandexcluded.dat\",\"Size\":${#content_unreferencedandexcluded}, \"Data\":\"$content_unreferencedandexcluded\"},
147+
{\"Filename\":\"foo/includedandexcluded.dat\",\"Size\":${#content_includedandexcluded}, \"Data\":\"$content_includedandexcluded\"},
148+
{\"Filename\":\"included.dat\",\"Size\":${#content_includedandexcluded}, \"Data\":\"$content_includedandexcluded\"},
135149
{\"Filename\":\"foo/oldandexcluded.dat\",\"Size\":${#content_excluded}, \"Data\":\"$content_excluded\"}]
136150
}
137151
]" | lfstest-testutils addcommits
138152

139153
git push origin main
154+
git branch -D branch_to_delete
140155

141156
git config lfs.fetchrecentrefsdays 5
142157
git config lfs.fetchrecentremoterefs true
@@ -148,16 +163,23 @@ begin_test "prune all excluded paths"
148163

149164
git lfs prune --dry-run --verbose 2>&1 | tee prune.log
150165

151-
grep "prune: 4 local objects, 1 retained" prune.log
152-
grep "prune: 3 files would be pruned" prune.log
166+
grep "prune: 6 local objects, 2 retained" prune.log
167+
grep "prune: 4 files would be pruned" prune.log
153168
grep "$oid_oldandexcluded" prune.log
154169
grep "$oid_prevandexcluded" prune.log
170+
grep "$oid_unreferencedandexcluded" prune.log
155171
grep "$oid_excluded" prune.log
156172

173+
assert_local_object "$oid_oldandexcluded" "${#content_oldandexcluded}"
174+
assert_local_object "$oid_prevandexcluded" "${#content_prevandexcluded}"
175+
assert_local_object "$oid_unreferencedandexcluded" "${#content_unreferencedandexcluded}"
176+
assert_local_object "$oid_excluded" "${#content_excluded}"
157177
git lfs prune
158178
refute_local_object "$oid_oldandexcluded" "${#content_oldandexcluded}"
159179
assert_local_object "$oid_oldandunchanged" "${#content_oldandunchanged}"
160180
refute_local_object "$oid_prevandexcluded" "${#content_prevandexcluded}"
181+
refute_local_object "$oid_unreferencedandexcluded" "${#content_unreferencedandexcluded}"
182+
assert_local_object "$oid_includedandexcluded" "${#content_includedandexcluded}"
161183
refute_local_object "$oid_excluded" "${#content_excluded}"
162184
)
163185
end_test

0 commit comments

Comments
 (0)