Message ID | 1f9ff57d52a72e3795ac4a924e23a64b91b1f83e.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: > diff --git a/builtin/repack.c b/builtin/repack.c > index c6a7943d5..9217fc832 100644 > --- a/builtin/repack.c > +++ b/builtin/repack.c > @@ -549,6 +549,12 @@ int cmd_repack(int argc, const char **argv, const char *prefix) > if (!po_args.quiet && isatty(2)) > opts |= PRUNE_PACKED_VERBOSE; > prune_packed_objects(opts); > + > + if (!keep_unreachable && > + (!(pack_everything & LOOSEN_UNREACHABLE) || > + unpack_unreachable) && > + is_repository_shallow(the_repository)) > + prune_shallow(0, 1); The logic looks correct (and the reasoning behind it, which was explained in the log message, was sound). prune_shallow(0, 1) however is not easily understandable. There are only two callsites of this function after these three patches, and the areas of the code that have these calls are in relatively quiescent parts in the whole tree, so it shouldn't be too distracting to do an update to make the function take a flag word, so that we can make callsites more immediately obvious, i.e. prune_shallow(PRUNE_SHALLOW_QUICK) It of course can be left as a low-hanging fruit loose-end. > diff --git a/t/t5537-fetch-shallow.sh b/t/t5537-fetch-shallow.sh > index 2d0031703..777c9d1dc 100755 > --- a/t/t5537-fetch-shallow.sh > +++ b/t/t5537-fetch-shallow.sh > @@ -186,7 +186,7 @@ EOF > test_cmp expect actual > ' > > -test_expect_failure '.git/shallow is edited by repack' ' > +test_expect_success '.git/shallow is edited by repack' ' > git init shallow-server && > test_commit -C shallow-server A && > test_commit -C shallow-server B &&
Hi Junio, On Wed, 24 Oct 2018, Junio C Hamano wrote: > "Johannes Schindelin via GitGitGadget" <gitgitgadget@gmail.com> > writes: > > > diff --git a/builtin/repack.c b/builtin/repack.c > > index c6a7943d5..9217fc832 100644 > > --- a/builtin/repack.c > > +++ b/builtin/repack.c > > @@ -549,6 +549,12 @@ int cmd_repack(int argc, const char **argv, const char *prefix) > > if (!po_args.quiet && isatty(2)) > > opts |= PRUNE_PACKED_VERBOSE; > > prune_packed_objects(opts); > > + > > + if (!keep_unreachable && > > + (!(pack_everything & LOOSEN_UNREACHABLE) || > > + unpack_unreachable) && > > + is_repository_shallow(the_repository)) > > + prune_shallow(0, 1); > > The logic looks correct (and the reasoning behind it, which was > explained in the log message, was sound). prune_shallow(0, 1) > however is not easily understandable. > > There are only two callsites of this function after these three > patches, and the areas of the code that have these calls are in > relatively quiescent parts in the whole tree, so it shouldn't be too > distracting to do an update to make the function take a flag word, > so that we can make callsites more immediately obvious, i.e. > > prune_shallow(PRUNE_SHALLOW_QUICK) > > It of course can be left as a low-hanging fruit loose-end. I almost did that, but then decided not to clutter the previous patch with this change (and global constant). Having said that, since you also had this idea, I will make that change. It will read nicer, you are right. Ciao, Dscho > > > diff --git a/t/t5537-fetch-shallow.sh b/t/t5537-fetch-shallow.sh > > index 2d0031703..777c9d1dc 100755 > > --- a/t/t5537-fetch-shallow.sh > > +++ b/t/t5537-fetch-shallow.sh > > @@ -186,7 +186,7 @@ EOF > > test_cmp expect actual > > ' > > > > -test_expect_failure '.git/shallow is edited by repack' ' > > +test_expect_success '.git/shallow is edited by repack' ' > > git init shallow-server && > > test_commit -C shallow-server A && > > test_commit -C shallow-server B && >
diff --git a/builtin/repack.c b/builtin/repack.c index c6a7943d5..9217fc832 100644 --- a/builtin/repack.c +++ b/builtin/repack.c @@ -549,6 +549,12 @@ int cmd_repack(int argc, const char **argv, const char *prefix) if (!po_args.quiet && isatty(2)) opts |= PRUNE_PACKED_VERBOSE; prune_packed_objects(opts); + + if (!keep_unreachable && + (!(pack_everything & LOOSEN_UNREACHABLE) || + unpack_unreachable) && + is_repository_shallow(the_repository)) + prune_shallow(0, 1); } if (!no_update_server_info) diff --git a/t/t5537-fetch-shallow.sh b/t/t5537-fetch-shallow.sh index 2d0031703..777c9d1dc 100755 --- a/t/t5537-fetch-shallow.sh +++ b/t/t5537-fetch-shallow.sh @@ -186,7 +186,7 @@ EOF test_cmp expect actual ' -test_expect_failure '.git/shallow is edited by repack' ' +test_expect_success '.git/shallow is edited by repack' ' git init shallow-server && test_commit -C shallow-server A && test_commit -C shallow-server B &&