| Message ID | 616b73a6556824fb94753cfcc62bf01d36b8b311.1610940216.git.gitgitgadget@gmail.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | Two cleanups around 'prefetch' refs | expand |
On Mon, Jan 18, 2021 at 03:23:36AM +0000, Derrick Stolee via GitGitGadget wrote: > From: Derrick Stolee <dstolee@microsoft.com> > > The tests for the 'prefetch' task create remotes and fetch refs into > 'refs/prefetch/<remote>/' and tags into 'refs/tags/'. These tests use > the remotes to create objects not intended to be seen by the "local" > repository. > > In that sense, the incrmental-repack tasks did not have these objects > and refs in mind. That test replaces the object directory with a > specific pack-file layout for testing the batch-size logic. However, > this causes some operations to start showing warnings such as: > > error: refs/prefetch/remote1/one does not point to a valid object! > error: refs/tags/one does not point to a valid object! > > This only shows up if you run the tests verbosely and watch the output. > It caught my eye and I _thought_ that there was a bug where 'git gc' or > 'git repack' wouldn't check 'refs/prefetch/' before pruning objects. > That is incorrect. Those commands do handle 'refs/prefetch/' correctly. Do you think it would be worth checking that 'does not point to a valid object' doesn't appear in the output? > All that is left is to clean up the tests in t7900-maintenance.sh to > remove these tags and refs that are not being repacked for the > incremental-repack tests. > > Signed-off-by: Derrick Stolee <dstolee@microsoft.com> > --- > t/t7900-maintenance.sh | 5 +++++ > 1 file changed, 5 insertions(+) > > diff --git a/t/t7900-maintenance.sh b/t/t7900-maintenance.sh > index f9031cbb44b..6be9d42767a 100755 > --- a/t/t7900-maintenance.sh > +++ b/t/t7900-maintenance.sh > @@ -256,6 +256,11 @@ test_expect_success 'incremental-repack task' ' > HEAD > ^HEAD~1 > EOF > + > + # Replace the object directory with this pack layout. > + # However, it does not include all objects from the remotes. > + rm -rf .git/refs/prefetch && > + rm -rf .git/refs/tags && Hmm. Makes sense, but this will certainly need to be updated to work with reftables, and it would break if you ran 'git pack-refs'. Perhaps instead: git for-each-ref --format='delete %(refname)' \ refs/prefetch refs/tags >refs && git update-ref --stdin <refs ? Thanks, Taylor
On 1/18/2021 11:04 AM, Taylor Blau wrote: > On Mon, Jan 18, 2021 at 03:23:36AM +0000, Derrick Stolee via GitGitGadget wrote: >> This only shows up if you run the tests verbosely and watch the output. >> It caught my eye and I _thought_ that there was a bug where 'git gc' or >> 'git repack' wouldn't check 'refs/prefetch/' before pruning objects. >> That is incorrect. Those commands do handle 'refs/prefetch/' correctly. > > Do you think it would be worth checking that 'does not point to a valid > object' doesn't appear in the output? Perhaps, but I think this isn't an actually interesting behavior to test. It's really the test that was broken, not Git itself. >> All that is left is to clean up the tests in t7900-maintenance.sh to >> remove these tags and refs that are not being repacked for the >> incremental-repack tests. >> >> Signed-off-by: Derrick Stolee <dstolee@microsoft.com> >> --- >> t/t7900-maintenance.sh | 5 +++++ >> 1 file changed, 5 insertions(+) >> >> diff --git a/t/t7900-maintenance.sh b/t/t7900-maintenance.sh >> index f9031cbb44b..6be9d42767a 100755 >> --- a/t/t7900-maintenance.sh >> +++ b/t/t7900-maintenance.sh >> @@ -256,6 +256,11 @@ test_expect_success 'incremental-repack task' ' >> HEAD >> ^HEAD~1 >> EOF >> + >> + # Replace the object directory with this pack layout. >> + # However, it does not include all objects from the remotes. >> + rm -rf .git/refs/prefetch && >> + rm -rf .git/refs/tags && > > Hmm. Makes sense, but this will certainly need to be updated to work > with reftables, and it would break if you ran 'git pack-refs'. > > Perhaps instead: > > git for-each-ref --format='delete %(refname)' \ > refs/prefetch refs/tags >refs && > git update-ref --stdin <refs Deleting refs in a clean way is probably good to do. I'm guessing you didn't use a pipe because it can be hard to diagnose a failure in the chain? That's probably reasonable. Thanks, -Stolee
diff --git a/t/t7900-maintenance.sh b/t/t7900-maintenance.sh index f9031cbb44b..6be9d42767a 100755 --- a/t/t7900-maintenance.sh +++ b/t/t7900-maintenance.sh @@ -256,6 +256,11 @@ test_expect_success 'incremental-repack task' ' HEAD ^HEAD~1 EOF + + # Replace the object directory with this pack layout. + # However, it does not include all objects from the remotes. + rm -rf .git/refs/prefetch && + rm -rf .git/refs/tags && rm -f $packDir/pack-* && rm -f $packDir/loose-* && ls $packDir/*.pack >packs-before &&