| Message ID | 20230308100438.908471-1-e@80x24.org (mailing list archive) |
|---|---|
| State | Superseded |
| Headers | show |
| Series | fetch: pass --no-write-fetch-head to subprocesses | expand |
Eric Wong <e@80x24.org> writes: > Subject: Re: [PATCH] fetch: pass --no-write-fetch-head to subprocesses I read the title as saying that "git fetch --recurse-submodules --no-write-fetch-head" should propagate the latter option down to fetches done in submodules, but looking at the added test, you are addressing a different use case, aren't you? Or are you covering both "fetch: honor --no-write-fetch-head when fetching from multiple remotes" and "fetch: pass --no-write-fetch-head down to submodules"? > It seems a user would expect this option would work regardless > of whether it's fetching from a single remote or many. This hints that it is only the latter, but if we are covering both (1) the title we have here may be alright. (2) the proposed log message should state the change affects both (in a good way). (3) the other half may want to be tested in new test as well. Thanks. > Signed-off-by: Eric Wong <e@80x24.org> > --- > I haven't checked if there's other suitable options which could > go into add_options_to_argv(); hopefully someone else can check :> > > builtin/fetch.c | 2 ++ > t/t5514-fetch-multiple.sh | 7 +++++++ > 2 files changed, 9 insertions(+) > > diff --git a/builtin/fetch.c b/builtin/fetch.c > index a09606b472..78513f1708 100644 > --- a/builtin/fetch.c > +++ b/builtin/fetch.c > @@ -1880,6 +1880,8 @@ static void add_options_to_argv(struct strvec *argv) > strvec_push(argv, "--ipv4"); > else if (family == TRANSPORT_FAMILY_IPV6) > strvec_push(argv, "--ipv6"); > + if (!write_fetch_head) > + strvec_push(argv, "--no-write-fetch-head"); > } > > /* Fetch multiple remotes in parallel */ > diff --git a/t/t5514-fetch-multiple.sh b/t/t5514-fetch-multiple.sh > index 54f422ced3..98f034aa77 100755 > --- a/t/t5514-fetch-multiple.sh > +++ b/t/t5514-fetch-multiple.sh > @@ -58,6 +58,13 @@ test_expect_success 'git fetch --all' ' > test_cmp expect output) > ' > > +test_expect_success 'git fetch --all --no-write-fetch-head' ' > + (cd test && > + rm -f .git/FETCH_HEAD && > + git fetch --all --no-write-fetch-head && > + test_path_is_missing .git/FETCH_HEAD) > +' > + > test_expect_success 'git fetch --all should continue if a remote has errors' ' > (git clone one test2 && > cd test2 &&
On Wed, Mar 08, 2023 at 10:04:38AM +0000, Eric Wong wrote: > It seems a user would expect this option would work regardless > of whether it's fetching from a single remote or many. > > Signed-off-by: Eric Wong <e@80x24.org> > --- > I haven't checked if there's other suitable options which could > go into add_options_to_argv(); hopefully someone else can check :> There's at least one that came up before: https://lore.kernel.org/git/DM5PR1701MB1724CCBB1AC5CF342BA9ADD5898E9@DM5PR1701MB1724.namprd17.prod.outlook.com/ but it never got turned into a real patch. This is obviously an error-prone mechanism. It would be nice if there was a way to avoid it, but after some discussion in this thread, we didn't come up with anything clever: https://lore.kernel.org/git/20200914121906.GD4705@pflmari/ -Peff
diff --git a/builtin/fetch.c b/builtin/fetch.c index a09606b472..78513f1708 100644 --- a/builtin/fetch.c +++ b/builtin/fetch.c @@ -1880,6 +1880,8 @@ static void add_options_to_argv(struct strvec *argv) strvec_push(argv, "--ipv4"); else if (family == TRANSPORT_FAMILY_IPV6) strvec_push(argv, "--ipv6"); + if (!write_fetch_head) + strvec_push(argv, "--no-write-fetch-head"); } /* Fetch multiple remotes in parallel */ diff --git a/t/t5514-fetch-multiple.sh b/t/t5514-fetch-multiple.sh index 54f422ced3..98f034aa77 100755 --- a/t/t5514-fetch-multiple.sh +++ b/t/t5514-fetch-multiple.sh @@ -58,6 +58,13 @@ test_expect_success 'git fetch --all' ' test_cmp expect output) ' +test_expect_success 'git fetch --all --no-write-fetch-head' ' + (cd test && + rm -f .git/FETCH_HEAD && + git fetch --all --no-write-fetch-head && + test_path_is_missing .git/FETCH_HEAD) +' + test_expect_success 'git fetch --all should continue if a remote has errors' ' (git clone one test2 && cd test2 &&
It seems a user would expect this option would work regardless of whether it's fetching from a single remote or many. Signed-off-by: Eric Wong <e@80x24.org> --- I haven't checked if there's other suitable options which could go into add_options_to_argv(); hopefully someone else can check :> builtin/fetch.c | 2 ++ t/t5514-fetch-multiple.sh | 7 +++++++ 2 files changed, 9 insertions(+)