* [PATCH] gc --auto: release pack files before auto packing @ 2018-06-30 13:38 Kim Gybels 2018-06-30 14:58 ` Duy Nguyen 2018-07-04 20:16 ` [PATCH v2] gc --auto: clear repository " Kim Gybels 0 siblings, 2 replies; 11+ messages in thread From: Kim Gybels @ 2018-06-30 13:38 UTC (permalink / raw) To: git Cc: Kim Gybels, Adam Borowski, Johannes Schindelin, Michael J Gruber, SZEDER Gábor, Nguyễn Thái Ngọc Duy, Junio C Hamano, Jeff King, Torsten Bögershausen Teach gc --auto to release pack files before auto packing the repository to prevent failures when removing them. Also teach the test 'fetching with auto-gc does not lock up' to complain when it is no longer triggering an auto packing of the repository. Fixes https://github.com/git-for-windows/git/issues/500 Signed-off-by: Kim Gybels <kgybels@infogroep.be> --- Patch based on master, since problem doesn't reproduce on maint, however, the improvement to the test might be valuable on maint. builtin/gc.c | 1 + t/t5510-fetch.sh | 2 ++ 2 files changed, 3 insertions(+) diff --git a/builtin/gc.c b/builtin/gc.c index ccfb1ceaeb..63b62ab57c 100644 --- a/builtin/gc.c +++ b/builtin/gc.c @@ -612,6 +612,7 @@ int cmd_gc(int argc, const char **argv, const char *prefix) return -1; if (!repository_format_precious_objects) { + close_all_packs(the_repository->objects); if (run_command_v_opt(repack.argv, RUN_GIT_CMD)) return error(FAILED_RUN, repack.argv[0]); diff --git a/t/t5510-fetch.sh b/t/t5510-fetch.sh index e402aee6a2..b91637e48f 100755 --- a/t/t5510-fetch.sh +++ b/t/t5510-fetch.sh @@ -828,9 +828,11 @@ test_expect_success 'fetching with auto-gc does not lock up' ' test_commit test2 && ( cd auto-gc && + git config fetch.unpackLimit 1 && git config gc.autoPackLimit 1 && git config gc.autoDetach false && GIT_ASK_YESNO="$D/askyesno" git fetch >fetch.out 2>&1 && + grep "Auto packing the repository" fetch.out && ! grep "Should I try again" fetch.out ) ' -- 2.18.0.windows.1 ^ permalink raw reply related [flat|nested] 11+ messages in thread
* Re: [PATCH] gc --auto: release pack files before auto packing 2018-06-30 13:38 [PATCH] gc --auto: release pack files before auto packing Kim Gybels @ 2018-06-30 14:58 ` Duy Nguyen 2018-07-06 16:39 ` Junio C Hamano 2018-07-04 20:16 ` [PATCH v2] gc --auto: clear repository " Kim Gybels 1 sibling, 1 reply; 11+ messages in thread From: Duy Nguyen @ 2018-06-30 14:58 UTC (permalink / raw) To: Kim Gybels Cc: git, Adam Borowski, Johannes Schindelin, Michael J Gruber, SZEDER Gábor, Junio C Hamano, Jeff King, Torsten Bögershausen On Sat, Jun 30, 2018 at 03:38:21PM +0200, Kim Gybels wrote: > Teach gc --auto to release pack files before auto packing the repository > to prevent failures when removing them. > > Also teach the test 'fetching with auto-gc does not lock up' to complain > when it is no longer triggering an auto packing of the repository. > > Fixes https://github.com/git-for-windows/git/issues/500 > > Signed-off-by: Kim Gybels <kgybels@infogroep.be> > --- > > Patch based on master, since problem doesn't reproduce on maint, > however, the improvement to the test might be valuable on maint. > > builtin/gc.c | 1 + > t/t5510-fetch.sh | 2 ++ > 2 files changed, 3 insertions(+) > > diff --git a/builtin/gc.c b/builtin/gc.c > index ccfb1ceaeb..63b62ab57c 100644 > --- a/builtin/gc.c > +++ b/builtin/gc.c > @@ -612,6 +612,7 @@ int cmd_gc(int argc, const char **argv, const char *prefix) > return -1; > > if (!repository_format_precious_objects) { > + close_all_packs(the_repository->objects); We have repo_clear() which does this and potentially closing file descriptors on other things as well. I suggest we use it, and before any external command is run. Something like diff --git a/builtin/gc.c b/builtin/gc.c index ccfb1ceaeb..a852c71da6 100644 --- a/builtin/gc.c +++ b/builtin/gc.c @@ -473,6 +473,13 @@ static int report_last_gc_error(void) static int gc_before_repack(void) { + /* + * Shut down everything, we should have all the info we need + * at this point. Leaving some file descriptors open may + * prevent them from being removed on Windows. + */ + repo_clear(the_repository); + if (pack_refs && run_command_v_opt(pack_refs_cmd.argv, RUN_GIT_CMD)) return error(FAILED_RUN, pack_refs_cmd.argv[0]); > if (run_command_v_opt(repack.argv, RUN_GIT_CMD)) > return error(FAILED_RUN, repack.argv[0]); > > diff --git a/t/t5510-fetch.sh b/t/t5510-fetch.sh > index e402aee6a2..b91637e48f 100755 > --- a/t/t5510-fetch.sh > +++ b/t/t5510-fetch.sh > @@ -828,9 +828,11 @@ test_expect_success 'fetching with auto-gc does not lock up' ' > test_commit test2 && > ( > cd auto-gc && > + git config fetch.unpackLimit 1 && > git config gc.autoPackLimit 1 && > git config gc.autoDetach false && > GIT_ASK_YESNO="$D/askyesno" git fetch >fetch.out 2>&1 && > + grep "Auto packing the repository" fetch.out && Not sure if this should be test_i18ngrep > ! grep "Should I try again" fetch.out > ) > ' > -- > 2.18.0.windows.1 > ^ permalink raw reply related [flat|nested] 11+ messages in thread
* Re: [PATCH] gc --auto: release pack files before auto packing 2018-06-30 14:58 ` Duy Nguyen @ 2018-07-06 16:39 ` Junio C Hamano 2018-07-07 9:40 ` SZEDER Gábor 0 siblings, 1 reply; 11+ messages in thread From: Junio C Hamano @ 2018-07-06 16:39 UTC (permalink / raw) To: Duy Nguyen Cc: Kim Gybels, git, Adam Borowski, Johannes Schindelin, Michael J Gruber, SZEDER Gábor, Jeff King, Torsten Bögershausen Duy Nguyen <pclouds@gmail.com> writes: > On Sat, Jun 30, 2018 at 03:38:21PM +0200, Kim Gybels wrote: >> Teach gc --auto to release pack files before auto packing the repository >> to prevent failures when removing them. >> >> Also teach the test 'fetching with auto-gc does not lock up' to complain >> when it is no longer triggering an auto packing of the repository. >> >> Fixes https://github.com/git-for-windows/git/issues/500 >> >> Signed-off-by: Kim Gybels <kgybels@infogroep.be> >> --- >> >> Patch based on master, since problem doesn't reproduce on maint, >> however, the improvement to the test might be valuable on maint. >> >> builtin/gc.c | 1 + >> t/t5510-fetch.sh | 2 ++ >> 2 files changed, 3 insertions(+) >> >> diff --git a/builtin/gc.c b/builtin/gc.c >> index ccfb1ceaeb..63b62ab57c 100644 >> --- a/builtin/gc.c >> +++ b/builtin/gc.c >> @@ -612,6 +612,7 @@ int cmd_gc(int argc, const char **argv, const char *prefix) >> return -1; >> >> if (!repository_format_precious_objects) { >> + close_all_packs(the_repository->objects); > > We have repo_clear() which does this and potentially closing file > descriptors on other things as well. I suggest we use it, and before > any external command is run. Something like > > diff --git a/builtin/gc.c b/builtin/gc.c > index ccfb1ceaeb..a852c71da6 100644 > --- a/builtin/gc.c > +++ b/builtin/gc.c > @@ -473,6 +473,13 @@ static int report_last_gc_error(void) > > static int gc_before_repack(void) > { > + /* > + * Shut down everything, we should have all the info we need > + * at this point. Leaving some file descriptors open may > + * prevent them from being removed on Windows. > + */ > + repo_clear(the_repository); > + > if (pack_refs && run_command_v_opt(pack_refs_cmd.argv, RUN_GIT_CMD)) > return error(FAILED_RUN, pack_refs_cmd.argv[0]); With https://public-inbox.org/git/20180509170409.13666-1-pclouds@gmail.com/ the call to repo_clear() on the_repository itself may be safe [*1*], but if command line parsing code etc. has pointers to in-core object etc. obtained before this function was called, the codeflow after this function returns will be quite disturbed. Has anybody in this review thread audited the codeflow _after_ this function is run to make sure that invalidating in-core repository here does not cause us problems? Just checking, because I won't queue this change until I hear that somebody familiar with the codepath actually did so (I may get around to do so myself later). Thanks. [Footnote] *1* ... and of course changing the_index to &the_repo->index would make it even safer ;-) ^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [PATCH] gc --auto: release pack files before auto packing 2018-07-06 16:39 ` Junio C Hamano @ 2018-07-07 9:40 ` SZEDER Gábor 2018-07-07 23:16 ` Kim Gybels 0 siblings, 1 reply; 11+ messages in thread From: SZEDER Gábor @ 2018-07-07 9:40 UTC (permalink / raw) To: Junio C Hamano Cc: Nguyễn Thái Ngọc Duy, kgybels, Git mailing list, kilobyte, Johannes Schindelin, Michael J Gruber, Jeff King, tboegi On Fri, Jul 6, 2018 at 6:39 PM Junio C Hamano <gitster@pobox.com> wrote: > > Duy Nguyen <pclouds@gmail.com> writes: > > > On Sat, Jun 30, 2018 at 03:38:21PM +0200, Kim Gybels wrote: > >> Teach gc --auto to release pack files before auto packing the repository > >> to prevent failures when removing them. > >> > >> Also teach the test 'fetching with auto-gc does not lock up' to complain > >> when it is no longer triggering an auto packing of the repository. > >> > >> Fixes https://github.com/git-for-windows/git/issues/500 > >> > >> Signed-off-by: Kim Gybels <kgybels@infogroep.be> > >> --- > >> > >> Patch based on master, since problem doesn't reproduce on maint, > >> however, the improvement to the test might be valuable on maint. > >> > >> builtin/gc.c | 1 + > >> t/t5510-fetch.sh | 2 ++ > >> 2 files changed, 3 insertions(+) > >> > >> diff --git a/builtin/gc.c b/builtin/gc.c > >> index ccfb1ceaeb..63b62ab57c 100644 > >> --- a/builtin/gc.c > >> +++ b/builtin/gc.c > >> @@ -612,6 +612,7 @@ int cmd_gc(int argc, const char **argv, const char *prefix) > >> return -1; > >> > >> if (!repository_format_precious_objects) { > >> + close_all_packs(the_repository->objects); > > > > We have repo_clear() which does this and potentially closing file > > descriptors on other things as well. I suggest we use it, and before > > any external command is run. Something like > > > > diff --git a/builtin/gc.c b/builtin/gc.c > > index ccfb1ceaeb..a852c71da6 100644 > > --- a/builtin/gc.c > > +++ b/builtin/gc.c > > @@ -473,6 +473,13 @@ static int report_last_gc_error(void) > > > > static int gc_before_repack(void) > > { > > + /* > > + * Shut down everything, we should have all the info we need > > + * at this point. Leaving some file descriptors open may > > + * prevent them from being removed on Windows. > > + */ > > + repo_clear(the_repository); > > + > > if (pack_refs && run_command_v_opt(pack_refs_cmd.argv, RUN_GIT_CMD)) > > return error(FAILED_RUN, pack_refs_cmd.argv[0]); > > With > > https://public-inbox.org/git/20180509170409.13666-1-pclouds@gmail.com/ > > the call to repo_clear() on the_repository itself may be safe [*1*], > but if command line parsing code etc. has pointers to in-core object > etc. obtained before this function was called, the codeflow after > this function returns will be quite disturbed. Has anybody in this > review thread audited the codeflow _after_ this function is run to > make sure that invalidating in-core repository here does not cause > us problems? It does create us problems, unfortunately. Among other things, a call to repo_clear(the_repository) does this: raw_object_store_clear(repo->objects); FREE_AND_NULL(repo->objects); Later on cmd_gc() calls reprepare_packed_git(the_repository), which then attempts to: r->objects->approximate_object_count_valid = 0; r->objects->packed_git_initialized = 0; but with r->objects being NULL at this point that won't work, and in turn causes failures in several test scripts. FWIW, the original patch appears to be working fine, at least the test suite passes. ^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [PATCH] gc --auto: release pack files before auto packing 2018-07-07 9:40 ` SZEDER Gábor @ 2018-07-07 23:16 ` Kim Gybels 2018-07-09 14:33 ` Duy Nguyen 0 siblings, 1 reply; 11+ messages in thread From: Kim Gybels @ 2018-07-07 23:16 UTC (permalink / raw) To: SZEDER Gábor Cc: Junio C Hamano, Nguyễn Thái Ngọc Duy, Git mailing list, kilobyte, Johannes Schindelin, Michael J Gruber, Jeff King, tboegi On (07/07/18 11:40), SZEDER Gábor wrote: > > On Fri, Jul 6, 2018 at 6:39 PM Junio C Hamano <gitster@pobox.com> wrote: > > > > Duy Nguyen <pclouds@gmail.com> writes: > > > > > On Sat, Jun 30, 2018 at 03:38:21PM +0200, Kim Gybels wrote: > > >> Teach gc --auto to release pack files before auto packing the repository > > >> to prevent failures when removing them. > > >> > > >> Also teach the test 'fetching with auto-gc does not lock up' to complain > > >> when it is no longer triggering an auto packing of the repository. > > >> > > >> Fixes https://github.com/git-for-windows/git/issues/500 > > >> > > >> Signed-off-by: Kim Gybels <kgybels@infogroep.be> > > >> --- > > >> > > >> Patch based on master, since problem doesn't reproduce on maint, > > >> however, the improvement to the test might be valuable on maint. > > >> > > >> builtin/gc.c | 1 + > > >> t/t5510-fetch.sh | 2 ++ > > >> 2 files changed, 3 insertions(+) > > >> > > >> diff --git a/builtin/gc.c b/builtin/gc.c > > >> index ccfb1ceaeb..63b62ab57c 100644 > > >> --- a/builtin/gc.c > > >> +++ b/builtin/gc.c > > >> @@ -612,6 +612,7 @@ int cmd_gc(int argc, const char **argv, const char *prefix) > > >> return -1; > > >> > > >> if (!repository_format_precious_objects) { > > >> + close_all_packs(the_repository->objects); > > > > > > We have repo_clear() which does this and potentially closing file > > > descriptors on other things as well. I suggest we use it, and before > > > any external command is run. Something like > > > > > > diff --git a/builtin/gc.c b/builtin/gc.c > > > index ccfb1ceaeb..a852c71da6 100644 > > > --- a/builtin/gc.c > > > +++ b/builtin/gc.c > > > @@ -473,6 +473,13 @@ static int report_last_gc_error(void) > > > > > > static int gc_before_repack(void) > > > { > > > + /* > > > + * Shut down everything, we should have all the info we need > > > + * at this point. Leaving some file descriptors open may > > > + * prevent them from being removed on Windows. > > > + */ > > > + repo_clear(the_repository); > > > + > > > if (pack_refs && run_command_v_opt(pack_refs_cmd.argv, RUN_GIT_CMD)) > > > return error(FAILED_RUN, pack_refs_cmd.argv[0]); > > > > With > > > > https://public-inbox.org/git/20180509170409.13666-1-pclouds@gmail.com/ > > > > the call to repo_clear() on the_repository itself may be safe [*1*], > > but if command line parsing code etc. has pointers to in-core object > > etc. obtained before this function was called, the codeflow after > > this function returns will be quite disturbed. Has anybody in this > > review thread audited the codeflow _after_ this function is run to > > make sure that invalidating in-core repository here does not cause > > us problems? > > It does create us problems, unfortunately. > > Among other things, a call to repo_clear(the_repository) does this: > > raw_object_store_clear(repo->objects); > FREE_AND_NULL(repo->objects); > > Later on cmd_gc() calls reprepare_packed_git(the_repository), which > then attempts to: > > r->objects->approximate_object_count_valid = 0; > r->objects->packed_git_initialized = 0; > > but with r->objects being NULL at this point that won't work, and in > turn causes failures in several test scripts. > > > FWIW, the original patch appears to be working fine, at least the test > suite passes. Should I post a v3 that goes back to the original fix, but uses test_i18ngrep instead of grep? In addition to not breaking any tests, close_all_packs is already used in a similar way in am and fetch just before running "gc --auto". -Kim ^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [PATCH] gc --auto: release pack files before auto packing 2018-07-07 23:16 ` Kim Gybels @ 2018-07-09 14:33 ` Duy Nguyen 2018-07-09 21:10 ` Junio C Hamano 0 siblings, 1 reply; 11+ messages in thread From: Duy Nguyen @ 2018-07-09 14:33 UTC (permalink / raw) To: Kim Gybels Cc: SZEDER Gábor, Junio C Hamano, Git Mailing List, Adam Borowski, Johannes Schindelin, Michael J Gruber, Jeff King, Torsten Bögershausen On Sun, Jul 8, 2018 at 1:16 AM Kim Gybels <kgybels@infogroep.be> wrote: > Should I post a v3 that goes back to the original fix, but uses > test_i18ngrep instead of grep? Yes please. In my comment I did write we didn't need the repo anymore (or something along that line) which turns out to be wrong. > In addition to not breaking any tests, close_all_packs is already used > in a similar way in am and fetch just before running "gc --auto". > > -Kim -- Duy ^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [PATCH] gc --auto: release pack files before auto packing 2018-07-09 14:33 ` Duy Nguyen @ 2018-07-09 21:10 ` Junio C Hamano 2018-07-11 15:33 ` Duy Nguyen 0 siblings, 1 reply; 11+ messages in thread From: Junio C Hamano @ 2018-07-09 21:10 UTC (permalink / raw) To: Duy Nguyen Cc: Kim Gybels, SZEDER Gábor, Git Mailing List, Adam Borowski, Johannes Schindelin, Michael J Gruber, Jeff King, Torsten Bögershausen Duy Nguyen <pclouds@gmail.com> writes: > On Sun, Jul 8, 2018 at 1:16 AM Kim Gybels <kgybels@infogroep.be> wrote: >> Should I post a v3 that goes back to the original fix, but uses >> test_i18ngrep instead of grep? > > Yes please. In my comment I did write we didn't need the repo anymore > (or something along that line) which turns out to be wrong. > >> In addition to not breaking any tests, close_all_packs is already used >> in a similar way in am and fetch just before running "gc --auto". >> >> -Kim Sound good. I recall that "clear repo should treat the_repository special" was discussed when we saw the patch that became 74373b5f ("repository: fix free problem with repo_clear(the_repository)", 2018-05-10), instead of treating only the index portion specially. Perhaps it was a more correct approach after all? ^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [PATCH] gc --auto: release pack files before auto packing 2018-07-09 21:10 ` Junio C Hamano @ 2018-07-11 15:33 ` Duy Nguyen 0 siblings, 0 replies; 11+ messages in thread From: Duy Nguyen @ 2018-07-11 15:33 UTC (permalink / raw) To: Junio C Hamano Cc: Kim Gybels, SZEDER Gábor, Git Mailing List, Adam Borowski, Johannes Schindelin, Michael J Gruber, Jeff King, Torsten Bögershausen On Mon, Jul 9, 2018 at 11:10 PM Junio C Hamano <gitster@pobox.com> wrote: > > Duy Nguyen <pclouds@gmail.com> writes: > > > On Sun, Jul 8, 2018 at 1:16 AM Kim Gybels <kgybels@infogroep.be> wrote: > >> Should I post a v3 that goes back to the original fix, but uses > >> test_i18ngrep instead of grep? > > > > Yes please. In my comment I did write we didn't need the repo anymore > > (or something along that line) which turns out to be wrong. > > > >> In addition to not breaking any tests, close_all_packs is already used > >> in a similar way in am and fetch just before running "gc --auto". > >> > >> -Kim > > Sound good. > > I recall that "clear repo should treat the_repository special" was > discussed when we saw the patch that became 74373b5f ("repository: > fix free problem with repo_clear(the_repository)", 2018-05-10), > instead of treating only the index portion specially. Perhaps it > was a more correct approach after all? I think it's good that we have a way to "shut down the repo" when we run an external command. But what we lack is "reinitialize the repo" after the external command is done. We could treat the_repository special in this case so shutting down does not require reinitialization. Then repo_clear() should work well here. We could also add a flag in repo_clear() to say "release all the resources you are holding, but keep the repo settings/location..., we're not done with this repo yet" then we don't need to re-initialize the repo afterwards and still don't make the_repository so special. -- Duy ^ permalink raw reply [flat|nested] 11+ messages in thread
* [PATCH v2] gc --auto: clear repository before auto packing 2018-06-30 13:38 [PATCH] gc --auto: release pack files before auto packing Kim Gybels 2018-06-30 14:58 ` Duy Nguyen @ 2018-07-04 20:16 ` Kim Gybels 2018-07-07 9:57 ` SZEDER Gábor 2018-07-09 20:37 ` [PATCH v3] gc --auto: release pack files " Kim Gybels 1 sibling, 2 replies; 11+ messages in thread From: Kim Gybels @ 2018-07-04 20:16 UTC (permalink / raw) To: git Cc: Kim Gybels, Adam Borowski, Johannes Schindelin, Michael J Gruber, SZEDER Gábor, Nguyễn Thái Ngọc Duy, Junio C Hamano, Jeff King, Torsten Bögershausen Teach gc --auto to clear the repository before auto packing it to prevent failures when removing files on Windows. Also teach the test 'fetching with auto-gc does not lock up' to complain when it is no longer triggering an auto packing of the repository. Fixes https://github.com/git-for-windows/git/issues/500 Signed-off-by: Kim Gybels <kgybels@infogroep.be> --- Updated after Duy Nguyen's comments: - use repo_clear instead of close_all_packs, and add the call in gc_before_repack intead of just before executing git repack - use test_i18ngrep instead of grep in updated test builtin/gc.c | 7 +++++++ t/t5510-fetch.sh | 4 +++- 2 files changed, 10 insertions(+), 1 deletion(-) diff --git a/builtin/gc.c b/builtin/gc.c index ccfb1ceaeb..22a6fc4863 100644 --- a/builtin/gc.c +++ b/builtin/gc.c @@ -473,6 +473,13 @@ static int report_last_gc_error(void) static int gc_before_repack(void) { + /* + * Shut down everything, we should have all the info we need + * at this point. Leaving some file descriptors open may + * prevent them from being removed on Windows. + */ + repo_clear(the_repository); + if (pack_refs && run_command_v_opt(pack_refs_cmd.argv, RUN_GIT_CMD)) return error(FAILED_RUN, pack_refs_cmd.argv[0]); diff --git a/t/t5510-fetch.sh b/t/t5510-fetch.sh index e402aee6a2..ef599c11cd 100755 --- a/t/t5510-fetch.sh +++ b/t/t5510-fetch.sh @@ -828,10 +828,12 @@ test_expect_success 'fetching with auto-gc does not lock up' ' test_commit test2 && ( cd auto-gc && + git config fetch.unpackLimit 1 && git config gc.autoPackLimit 1 && git config gc.autoDetach false && GIT_ASK_YESNO="$D/askyesno" git fetch >fetch.out 2>&1 && - ! grep "Should I try again" fetch.out + test_i18ngrep "Auto packing the repository" fetch.out && + test_i18ngrep ! "Should I try again" fetch.out ) ' -- 2.18.0.windows.1 ^ permalink raw reply related [flat|nested] 11+ messages in thread
* Re: [PATCH v2] gc --auto: clear repository before auto packing 2018-07-04 20:16 ` [PATCH v2] gc --auto: clear repository " Kim Gybels @ 2018-07-07 9:57 ` SZEDER Gábor 2018-07-09 20:37 ` [PATCH v3] gc --auto: release pack files " Kim Gybels 1 sibling, 0 replies; 11+ messages in thread From: SZEDER Gábor @ 2018-07-07 9:57 UTC (permalink / raw) To: kgybels Cc: Git mailing list, kilobyte, Johannes Schindelin, Michael J Gruber, Nguyễn Thái Ngọc Duy, Junio C Hamano, Jeff King, tboegi On Wed, Jul 4, 2018 at 10:16 PM Kim Gybels <kgybels@infogroep.be> wrote: > diff --git a/t/t5510-fetch.sh b/t/t5510-fetch.sh > index e402aee6a2..ef599c11cd 100755 > --- a/t/t5510-fetch.sh > +++ b/t/t5510-fetch.sh > @@ -828,10 +828,12 @@ test_expect_success 'fetching with auto-gc does not lock up' ' > test_commit test2 && > ( > cd auto-gc && > + git config fetch.unpackLimit 1 && > git config gc.autoPackLimit 1 && > git config gc.autoDetach false && > GIT_ASK_YESNO="$D/askyesno" git fetch >fetch.out 2>&1 && > - ! grep "Should I try again" fetch.out > + test_i18ngrep "Auto packing the repository" fetch.out && > + test_i18ngrep ! "Should I try again" fetch.out The messages containing "Auto packing the repository" are indeed translated, thus they have to be checked with 'test_i18ngrep', good. However, none of the "Should I try again" messages are translated, so checking them with bare 'grep' is fine and it shouldn't be changed. > ) > ' > > -- > 2.18.0.windows.1 > ^ permalink raw reply [flat|nested] 11+ messages in thread
* [PATCH v3] gc --auto: release pack files before auto packing 2018-07-04 20:16 ` [PATCH v2] gc --auto: clear repository " Kim Gybels 2018-07-07 9:57 ` SZEDER Gábor @ 2018-07-09 20:37 ` Kim Gybels 1 sibling, 0 replies; 11+ messages in thread From: Kim Gybels @ 2018-07-09 20:37 UTC (permalink / raw) To: git Cc: Kim Gybels, Adam Borowski, Johannes Schindelin, Michael J Gruber, SZEDER Gábor, Nguyễn Thái Ngọc Duy, Junio C Hamano, Jeff King, Torsten Bögershausen Teach gc --auto to release pack files before auto packing the repository to prevent failures when removing them. Also teach the test 'fetching with auto-gc does not lock up' to complain when it is no longer triggering an auto packing of the repository. Fixes https://github.com/git-for-windows/git/issues/500 Signed-off-by: Kim Gybels <kgybels@infogroep.be> --- Changes since v2: - revert fix back to v1: use close_all_packs instead of repo_clear - use test_i18ngrep only for translated string builtin/gc.c | 1 + t/t5510-fetch.sh | 2 ++ 2 files changed, 3 insertions(+) diff --git a/builtin/gc.c b/builtin/gc.c index ccfb1ceaeb..63b62ab57c 100644 --- a/builtin/gc.c +++ b/builtin/gc.c @@ -612,6 +612,7 @@ int cmd_gc(int argc, const char **argv, const char *prefix) return -1; if (!repository_format_precious_objects) { + close_all_packs(the_repository->objects); if (run_command_v_opt(repack.argv, RUN_GIT_CMD)) return error(FAILED_RUN, repack.argv[0]); diff --git a/t/t5510-fetch.sh b/t/t5510-fetch.sh index e402aee6a2..6f1450eb69 100755 --- a/t/t5510-fetch.sh +++ b/t/t5510-fetch.sh @@ -828,9 +828,11 @@ test_expect_success 'fetching with auto-gc does not lock up' ' test_commit test2 && ( cd auto-gc && + git config fetch.unpackLimit 1 && git config gc.autoPackLimit 1 && git config gc.autoDetach false && GIT_ASK_YESNO="$D/askyesno" git fetch >fetch.out 2>&1 && + test_i18ngrep "Auto packing the repository" fetch.out && ! grep "Should I try again" fetch.out ) ' -- 2.18.0.windows.1 ^ permalink raw reply related [flat|nested] 11+ messages in thread
end of thread, other threads:[~2018-07-11 15:34 UTC | newest] Thread overview: 11+ messages (download: mbox.gz / follow: Atom feed) -- links below jump to the message on this page -- 2018-06-30 13:38 [PATCH] gc --auto: release pack files before auto packing Kim Gybels 2018-06-30 14:58 ` Duy Nguyen 2018-07-06 16:39 ` Junio C Hamano 2018-07-07 9:40 ` SZEDER Gábor 2018-07-07 23:16 ` Kim Gybels 2018-07-09 14:33 ` Duy Nguyen 2018-07-09 21:10 ` Junio C Hamano 2018-07-11 15:33 ` Duy Nguyen 2018-07-04 20:16 ` [PATCH v2] gc --auto: clear repository " Kim Gybels 2018-07-07 9:57 ` SZEDER Gábor 2018-07-09 20:37 ` [PATCH v3] gc --auto: release pack files " Kim Gybels
Code repositories for project(s) associated with this public inbox https://80x24.org/mirrors/git.git This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox; as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).