| Message ID | 20241204104003.514905-3-bence@ferdinandy.com (mailing list archive) |
|---|---|
| State | Superseded |
| Headers | show |
| Series | [v2,1/3] fetch set_head: move warn advice into advise_if_enabled | expand |
Hi On Wed, Dec 4, 2024, at 11:39, Bence Ferdinandy wrote: > diff --git a/t/t5505-remote.sh b/t/t5505-remote.sh > index 7411aa770d..daf70406be 100755 > --- a/t/t5505-remote.sh > +++ b/t/t5505-remote.sh > @@ -504,6 +504,17 @@ test_expect_success 'set-head --auto has no > problem w/multiple HEADs' ' > ) > ' > > +test_expect_success 'set-head changes followRemoteHEAD always to warn' ' > + ( > + cd test && I think you need to `cd` in a subshell here. See `t/README`, “Don't chdir around in tests.”. > + git config set remote.origin.followRemoteHEAD "always" && > + git remote set-head --auto origin && > + git config get remote.origin.followRemoteHEAD >output && Nit: maybe `actual` instead of `output`? Just for uniformity.
"Kristoffer Haugsbakk" <kristofferhaugsbakk@fastmail.com> writes: > On Wed, Dec 4, 2024, at 11:39, Bence Ferdinandy wrote: >> diff --git a/t/t5505-remote.sh b/t/t5505-remote.sh >> index 7411aa770d..daf70406be 100755 >> --- a/t/t5505-remote.sh >> +++ b/t/t5505-remote.sh >> @@ -504,6 +504,17 @@ test_expect_success 'set-head --auto has no >> problem w/multiple HEADs' ' >> ) >> ' >> >> +test_expect_success 'set-head changes followRemoteHEAD always to warn' ' >> + ( >> + cd test && > > I think you need to `cd` in a subshell here. See `t/README`, “Don't > chdir around in tests.”. Puzzled. Isn't this inside a (subshell) already? >> + git config set remote.origin.followRemoteHEAD "always" && >> + git remote set-head --auto origin && >> + git config get remote.origin.followRemoteHEAD >output && > > Nit: maybe `actual` instead of `output`? Just for uniformity. t5505 is already heavily contaminated with the pattern to compare "expect" with "output", not with "actual", but that does not make it a good idea to make it even worse by adding more instances ;-)
On Wed, Dec 4, 2024, at 21:40, Junio C Hamano wrote: > "Kristoffer Haugsbakk" <kristofferhaugsbakk@fastmail.com> writes: > >> On Wed, Dec 4, 2024, at 11:39, Bence Ferdinandy wrote: >>> diff --git a/t/t5505-remote.sh b/t/t5505-remote.sh >>> index 7411aa770d..daf70406be 100755 >>> --- a/t/t5505-remote.sh >>> +++ b/t/t5505-remote.sh >>> @@ -504,6 +504,17 @@ test_expect_success 'set-head --auto has no >>> problem w/multiple HEADs' ' >>> ) >>> ' >>> >>> +test_expect_success 'set-head changes followRemoteHEAD always to warn' ' >>> + ( >>> + cd test && >> >> I think you need to `cd` in a subshell here. See `t/README`, “Don't >> chdir around in tests.”. > > Puzzled. Isn't this inside a (subshell) already? Aha, then I didn’t read the context properly.
On Wed Dec 04, 2024 at 21:44, Kristoffer Haugsbakk <kristofferhaugsbakk@fastmail.com> wrote: > On Wed, Dec 4, 2024, at 21:40, Junio C Hamano wrote: >> "Kristoffer Haugsbakk" <kristofferhaugsbakk@fastmail.com> writes: >> >>> On Wed, Dec 4, 2024, at 11:39, Bence Ferdinandy wrote: >>>> diff --git a/t/t5505-remote.sh b/t/t5505-remote.sh >>>> index 7411aa770d..daf70406be 100755 >>>> --- a/t/t5505-remote.sh >>>> +++ b/t/t5505-remote.sh >>>> @@ -504,6 +504,17 @@ test_expect_success 'set-head --auto has no >>>> problem w/multiple HEADs' ' >>>> ) >>>> ' >>>> >>>> +test_expect_success 'set-head changes followRemoteHEAD always to warn' ' >>>> + ( >>>> + cd test && >>> >>> I think you need to `cd` in a subshell here. See `t/README`, “Don't >>> chdir around in tests.”. >> >> Puzzled. Isn't this inside a (subshell) already? > > Aha, then I didn’t read the context properly. Thanks for the review! What I gathered is that v3 needs an s/output/actual and we're good. I'll send that soonish. Thanks, Bence
diff --git a/builtin/remote.c b/builtin/remote.c index d5b81445f2..32d02ca4a0 100644 --- a/builtin/remote.c +++ b/builtin/remote.c @@ -1438,6 +1438,7 @@ static int set_head(int argc, const char **argv, const char *prefix, b_local_head = STRBUF_INIT; char *head_name = NULL; struct ref_store *refs = get_main_ref_store(the_repository); + struct remote *remote; struct option options[] = { OPT_BOOL('a', "auto", &opt_a, @@ -1448,8 +1449,10 @@ static int set_head(int argc, const char **argv, const char *prefix, }; argc = parse_options(argc, argv, prefix, options, builtin_remote_sethead_usage, 0); - if (argc) + if (argc) { strbuf_addf(&b_head, "refs/remotes/%s/HEAD", argv[0]); + remote = remote_get(argv[0]); + } if (!opt_a && !opt_d && argc == 2) { head_name = xstrdup(argv[1]); @@ -1488,6 +1491,13 @@ static int set_head(int argc, const char **argv, const char *prefix, } if (opt_a) report_set_head_auto(argv[0], head_name, &b_local_head, was_detached); + if (remote->follow_remote_head == FOLLOW_REMOTE_ALWAYS) { + struct strbuf config_name = STRBUF_INIT; + strbuf_addf(&config_name, + "remote.%s.followremotehead", remote->name); + git_config_set(config_name.buf, "warn"); + strbuf_release(&config_name); + } cleanup: free(head_name); diff --git a/t/t5505-remote.sh b/t/t5505-remote.sh index 7411aa770d..daf70406be 100755 --- a/t/t5505-remote.sh +++ b/t/t5505-remote.sh @@ -504,6 +504,17 @@ test_expect_success 'set-head --auto has no problem w/multiple HEADs' ' ) ' +test_expect_success 'set-head changes followRemoteHEAD always to warn' ' + ( + cd test && + git config set remote.origin.followRemoteHEAD "always" && + git remote set-head --auto origin && + git config get remote.origin.followRemoteHEAD >output && + echo "warn" >expect && + test_cmp expect output + ) +' + cat >test/expect <<\EOF refs/remotes/origin/side2 EOF
When running "remote set-head" manually it is unlikely, that the user would actually like to have "fetch" always update the remote/HEAD. On the contrary, it is more likely, that the user would expect remote/HEAD to stay the way they manually set it, and just forgot about having "followRemoteHEAD" set to "always". When "followRemoteHEAD" is set to "always" make running "remote set-head" change the config to "warn". Signed-off-by: Bence Ferdinandy <bence@ferdinandy.com> --- builtin/remote.c | 12 +++++++++++- t/t5505-remote.sh | 11 +++++++++++ 2 files changed, 22 insertions(+), 1 deletion(-)