| Message ID | ed8559b91b7a3b51a5950d62e78fc726ed5b44c2.1540245934.git.gitgitgadget@gmail.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | repack -ad: fix after fetch --prune in a shallow repository | expand |
"Johannes Schindelin via GitGitGadget" <gitgitgadget@gmail.com> writes: > From: Johannes Schindelin <johannes.schindelin@gmx.de> > > A `git fetch --prune` can turn previously-reachable objects unreachable, > even commits that are in the `shallow` list. A subsequent `git repack > -ad` will then unceremoniously drop those unreachable commits, and the > `shallow` list will become stale. This means that when we try to fetch > with a larger `--depth` the next time, we may end up with: > > fatal: error in object: unshallow <commit-hash> Nicely analysed and described. One minor thing nagged me in the implementation but it is not a big issue. > +... > + d="$(git -C shallow-server rev-parse --verify D)" && > + git -C shallow-server checkout master && > + > +... > + ! grep $d shallow-client/.git/shallow && We know D (and $d) is not a tag, but perhaps the place that assigns to $d (way above) can do the rev-parse of D^0, not D, to make it more clear what is going on, especially given that... > + git -C shallow-server branch branch-orig D^0 && ... this does that D^0 thing here to makes us wonder if D needs unwrapping before using it as a commit (not commit-ish). If we did so, we could use $d here instead of D^0. > + git -C shallow-client fetch --prune --depth=2 \ > + origin "+refs/heads/*:refs/remotes/origin/*" > +' > + > . "$TEST_DIRECTORY"/lib-httpd.sh > start_httpd
Hi Junio, On Wed, 24 Oct 2018, Junio C Hamano wrote: > "Johannes Schindelin via GitGitGadget" <gitgitgadget@gmail.com> > writes: > > > +... > > + d="$(git -C shallow-server rev-parse --verify D)" && > > + git -C shallow-server checkout master && > > + > > +... > > + ! grep $d shallow-client/.git/shallow && > > We know D (and $d) is not a tag, Actually, it is... the `test_commit D` creates that tag, and that is what I use here. > but perhaps the place that assigns to $d (way above) can do the > rev-parse of D^0, not D, to make it more clear what is going on, > especially given that... ... that the `grep` really wants to test for the absence of the *commit*, not the *tag* in .git/shallow? Yes, you are right ;-) So why did my test do the right thing, if it looked at a tag, but did not dereference it via ^0? The answer is: the `test_commit` function creates light-weight tags, i.e. no tag objects. And therefore, the $d^0 you found below, that's just confusing. I will change it so that the `rev-parse` call uses ^0 (even if it is technically not necessary), to avoid said confusion. Thanks, Dscho > > > + git -C shallow-server branch branch-orig D^0 && > > ... this does that D^0 thing here to makes us wonder if D needs > unwrapping before using it as a commit (not commit-ish). > > If we did so, we could use $d here instead of D^0. > > > + git -C shallow-client fetch --prune --depth=2 \ > > + origin "+refs/heads/*:refs/remotes/origin/*" > > +' > > + > > . "$TEST_DIRECTORY"/lib-httpd.sh > > start_httpd > >
Hi Junio, me again. I was wrong. On Wed, 24 Oct 2018, Johannes Schindelin wrote: > On Wed, 24 Oct 2018, Junio C Hamano wrote: > > > "Johannes Schindelin via GitGitGadget" <gitgitgadget@gmail.com> > > writes: > > > > > +... > > > + d="$(git -C shallow-server rev-parse --verify D)" && > > > + git -C shallow-server checkout master && > > > + > > > +... > > > + ! grep $d shallow-client/.git/shallow && > > > > We know D (and $d) is not a tag, > > Actually, it is... the `test_commit D` creates that tag, and that is what > I use here. > > > but perhaps the place that assigns to $d (way above) can do the > > rev-parse of D^0, not D, to make it more clear what is going on, > > especially given that... > > ... that the `grep` really wants to test for the absence of the *commit*, > not the *tag* in .git/shallow? > > Yes, you are right ;-) > > So why did my test do the right thing, if it looked at a tag, but did not > dereference it via ^0? The answer is: the `test_commit` function creates > light-weight tags, i.e. no tag objects. And therefore, the $d^0 you found > below, that's just confusing. What I was referring to was the call test_must_fail git -C shallow-client rev-parse --verify $d^0 However, here we *have* to append ^0, otherwise `rev-parse --verify` will (and quite confusingly so) *succeed*. I *repeatedly* fall into that trap that `git rev-parse --verify 0000000000000000000000000000000000000000` will succeed. Why? Because that is a valid 40-digit hex string. Not because the object exists. Because it does not. So I managed to confuse myself again into believing that I only need to append ^0 to the earlier rev-parse call, but can remove it from this one, when in reality, I have to append it to both. In the first case, to avoid having to think about dereferencing a tag, in the second case, to force rev-parse to look for the object. Ciao, Dscho > > I will change it so that the `rev-parse` call uses ^0 (even if it is > technically not necessary), to avoid said confusion. > > Thanks, > Dscho > > > > > > + git -C shallow-server branch branch-orig D^0 && > > > > ... this does that D^0 thing here to makes us wonder if D needs > > unwrapping before using it as a commit (not commit-ish). > > > > If we did so, we could use $d here instead of D^0. > > > > > + git -C shallow-client fetch --prune --depth=2 \ > > > + origin "+refs/heads/*:refs/remotes/origin/*" > > > +' > > > + > > > . "$TEST_DIRECTORY"/lib-httpd.sh > > > start_httpd > > > > >
diff --git a/t/t5537-fetch-shallow.sh b/t/t5537-fetch-shallow.sh index 7045685e2..2d0031703 100755 --- a/t/t5537-fetch-shallow.sh +++ b/t/t5537-fetch-shallow.sh @@ -186,6 +186,33 @@ EOF test_cmp expect actual ' +test_expect_failure '.git/shallow is edited by repack' ' + git init shallow-server && + test_commit -C shallow-server A && + test_commit -C shallow-server B && + git -C shallow-server checkout -b branch && + test_commit -C shallow-server C && + test_commit -C shallow-server E && + test_commit -C shallow-server D && + d="$(git -C shallow-server rev-parse --verify D)" && + git -C shallow-server checkout master && + + git clone --depth=1 --no-tags --no-single-branch \ + "file://$PWD/shallow-server" shallow-client && + + : now remove the branch and fetch with prune && + git -C shallow-server branch -D branch && + git -C shallow-client fetch --prune --depth=1 \ + origin "+refs/heads/*:refs/remotes/origin/*" && + git -C shallow-client repack -adfl && + test_must_fail git -C shallow-client rev-parse --verify $d^0 && + ! grep $d shallow-client/.git/shallow && + + git -C shallow-server branch branch-orig D^0 && + git -C shallow-client fetch --prune --depth=2 \ + origin "+refs/heads/*:refs/remotes/origin/*" +' + . "$TEST_DIRECTORY"/lib-httpd.sh start_httpd