| Message ID | xmqqr1svegt4.fsf_-_@gitster.c.googlers.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | [v2] fetch: optionally allow disabling FETCH_HEAD update | expand |
On 28/07/2020 17:37, Junio C Hamano wrote: > Derrick Stolee <stolee@gmail.com> writes: > >> On 7/27/2020 12:13 PM, Junio C Hamano wrote: >>> Derrick Stolee <stolee@gmail.com> writes: >>> >>>> I'll rebase onto jc/no-update-fetch-head for the next version, since >>>> that branch is based on v2.28.0-rc0, which is recent enough. >>> >>> I do not think it is wise to base a work on top of unfinished "you >>> could do it this way, perhaps?" demonstration patch the original >>> author does not have much inclination to finish, though. >>> >>> When I am really bored, I may go back to the topic to finish it, but >>> I wouldn't mind if you took ownership of it at all. >> >> Ah. I didn't understand the status of that branch. I'll pull it in >> to this topic. > > So here is with one of the two things that I found missing in the > first iteration of the patch: documentation. > > The other thing that I found iffy (and still missing from this > version) was what should be done when "git pull" is explicitly given > the "--no-write-fetch-head" option. > > I think (but didn't check the recent code) that 'git pull' would > pass only known-to-make-sense command line options to underlying > 'git fetch', so it probably will barf with "unknown option", which > is the best case. We might want to make it sure with a new test in > 5521. On the other hand, if we get anything other than "no such > option", we may want to think if we want to "fix" it or just leave > it inside "if it hurts, don't do it" territory. > > Thanks. > > The patch without doc was Reviewed-by: Taylor Blau <me@ttaylorr.com> > but this round has not been. > > -- >8 -- > > If you run fetch but record the result in remote-tracking branches, > and either if you do nothing with the fetched refs (e.g. you are > merely mirroring) or if you always work from the remote-tracking > refs (e.g. you fetch and then merge origin/branchname separately), > you can get away with having no FETCH_HEAD at all. > > Teach "git fetch" a command line option "--[no-]write-fetch-head" > and "fetch.writeFetchHEAD" configuration variable. Without either, > the default is to write FETCH_HEAD, and the usual rule that the > command line option defeats configured default applies. > > Note that under "--dry-run" mode, FETCH_HEAD is never written; > otherwise you'd see list of objects in the file that you do not > actually have. Passing `--fetch-write-head` Typo, it should be `--write-fetch-head` >does not force `git > fetch` to write the file. > > Also note that this option is explicitly passed when "git pull" > internally invokes "git fetch", so that those who configured their > "git fetch" not to write FETCH_HEAD would not be able to break the > cooperation between these two commands. "git pull" must see what > "git fetch" got recorded in FETCH_HEAD to work correctly. > > Signed-off-by: Junio C Hamano <gitster@pobox.com> > --- > Documentation/config/fetch.txt | 7 ++++++ > Documentation/fetch-options.txt | 10 +++++++++ > builtin/fetch.c | 19 +++++++++++++--- > builtin/pull.c | 3 ++- > t/t5510-fetch.sh | 39 +++++++++++++++++++++++++++++++-- > 5 files changed, 72 insertions(+), 6 deletions(-) > > diff --git a/Documentation/config/fetch.txt b/Documentation/config/fetch.txt > index b20394038d..0aaa05e8c0 100644 > --- a/Documentation/config/fetch.txt > +++ b/Documentation/config/fetch.txt > @@ -91,3 +91,10 @@ fetch.writeCommitGraph:: > merge and the write may take longer. Having an updated commit-graph > file helps performance of many Git commands, including `git merge-base`, > `git push -f`, and `git log --graph`. Defaults to false. > + > +fetch.writeFetchHEAD:: > + Setting it to false tells `git fetch` not to write the list > + of remote refs fetched in the `FETCH_HEAD` file directly > + under `$GIT_DIR`. Can be countermanded from the command > + line with the `--[no-]write-fetch-head` option. Defaults to > + true. > diff --git a/Documentation/fetch-options.txt b/Documentation/fetch-options.txt > index 6e2a160a47..6775e8499f 100644 > --- a/Documentation/fetch-options.txt > +++ b/Documentation/fetch-options.txt > @@ -64,6 +64,16 @@ documented in linkgit:git-config[1]. > --dry-run:: > Show what would be done, without making any changes. > > +ifndef::git-pull[] > +--[no-]write-fetch-head:: > + Write the list of remote refs fetched in the `FETCH_HEAD` > + file directly under `$GIT_DIR`. This is the default unless > + the configuration variable `fetch.writeFetchHEAD` is set to > + false. Passing `--no-write-fetch-head` from the command > + line tells Git not to write the file. Under `--dry-run` > + option, the file is never written. > +endif::git-pull[] > + > -f:: > --force:: > When 'git fetch' is used with `<src>:<dst>` refspec it may > diff --git a/builtin/fetch.c b/builtin/fetch.c > index 82ac4be8a5..3ccf69753f 100644 > --- a/builtin/fetch.c > +++ b/builtin/fetch.c > @@ -56,6 +56,7 @@ static int prune_tags = -1; /* unspecified */ > #define PRUNE_TAGS_BY_DEFAULT 0 /* do we prune tags by default? */ > > static int all, append, dry_run, force, keep, multiple, update_head_ok; > +static int write_fetch_head = 1; > static int verbosity, deepen_relative, set_upstream; > static int progress = -1; > static int enable_auto_gc = 1; > @@ -118,6 +119,10 @@ static int git_fetch_config(const char *k, const char *v, void *cb) > return 0; > } > > + if (!strcmp(k, "fetch.writefetchhead")) { > + write_fetch_head = git_config_bool(k, v); > + return 0; > + } > return git_default_config(k, v, cb); > } > > @@ -162,6 +167,8 @@ static struct option builtin_fetch_options[] = { > PARSE_OPT_OPTARG, option_fetch_parse_recurse_submodules), > OPT_BOOL(0, "dry-run", &dry_run, > N_("dry run")), > + OPT_BOOL(0, "write-fetch-head", &write_fetch_head, > + N_("write fetched references to the FETCH_HEAD file")), > OPT_BOOL('k', "keep", &keep, N_("keep downloaded pack")), > OPT_BOOL('u', "update-head-ok", &update_head_ok, > N_("allow updating of HEAD ref")), > @@ -893,7 +900,9 @@ static int store_updated_refs(const char *raw_url, const char *remote_name, > const char *what, *kind; > struct ref *rm; > char *url; > - const char *filename = dry_run ? "/dev/null" : git_path_fetch_head(the_repository); > + const char *filename = (!write_fetch_head > + ? "/dev/null" > + : git_path_fetch_head(the_repository)); I was suspicious of this as we haven't cleared write_fetch_head in the --dry-run case yet but the test below seems to show that we still don't write FETCH_HEAD if --dry-run is given. That makes we wonder what the point of setting the filename based on the value of write_fetch_head is. The rest looks good to me, though it might be worth having a test to check that pull does indeed reject --no-write-fetch-head Best Wishes Phillip > int want_status; > int summary_width = transport_summary_width(ref_map); > > @@ -1327,7 +1336,7 @@ static int do_fetch(struct transport *transport, > } > > /* if not appending, truncate FETCH_HEAD */ > - if (!append && !dry_run) { > + if (!append && write_fetch_head) { > retcode = truncate_fetch_head(); > if (retcode) > goto cleanup; > @@ -1594,7 +1603,7 @@ static int fetch_multiple(struct string_list *list, int max_children) > int i, result = 0; > struct argv_array argv = ARGV_ARRAY_INIT; > > - if (!append && !dry_run) { > + if (!append && write_fetch_head) { > int errcode = truncate_fetch_head(); > if (errcode) > return errcode; > @@ -1795,6 +1804,10 @@ int cmd_fetch(int argc, const char **argv, const char *prefix) > if (depth || deepen_since || deepen_not.nr) > deepen = 1; > > + /* FETCH_HEAD never gets updated in --dry-run mode */ > + if (dry_run) > + write_fetch_head = 0; > + > if (all) { > if (argc == 1) > die(_("fetch --all does not take a repository argument")); > diff --git a/builtin/pull.c b/builtin/pull.c > index 8159c5d7c9..e988d92b53 100644 > --- a/builtin/pull.c > +++ b/builtin/pull.c > @@ -527,7 +527,8 @@ static int run_fetch(const char *repo, const char **refspecs) > struct argv_array args = ARGV_ARRAY_INIT; > int ret; > > - argv_array_pushl(&args, "fetch", "--update-head-ok", NULL); > + argv_array_pushl(&args, "fetch", "--update-head-ok", > + "--write-fetch-head", NULL); > > /* Shared options */ > argv_push_verbosity(&args); > diff --git a/t/t5510-fetch.sh b/t/t5510-fetch.sh > index a66dbe0bde..3052c2d8d5 100755 > --- a/t/t5510-fetch.sh > +++ b/t/t5510-fetch.sh > @@ -539,13 +539,48 @@ test_expect_success 'fetch into the current branch with --update-head-ok' ' > > ' > > -test_expect_success 'fetch --dry-run' ' > - > +test_expect_success 'fetch --dry-run does not touch FETCH_HEAD' ' > rm -f .git/FETCH_HEAD && > git fetch --dry-run . && > ! test -f .git/FETCH_HEAD > ' > > +test_expect_success '--no-write-fetch-head does not touch FETCH_HEAD' ' > + rm -f .git/FETCH_HEAD && > + git fetch --no-write-fetch-head . && > + ! test -f .git/FETCH_HEAD > +' > + > +test_expect_success '--write-fetch-head gets defeated by --dry-run' ' > + rm -f .git/FETCH_HEAD && > + git fetch --dry-run --write-fetch-head . && > + ! test -f .git/FETCH_HEAD > +' > + > +test_expect_success 'fetch.writeFetchHEAD and FETCH_HEAD' ' > + rm -f .git/FETCH_HEAD && > + git -c fetch.writeFetchHEAD=no fetch . && > + ! test -f .git/FETCH_HEAD > +' > + > +test_expect_success 'fetch.writeFetchHEAD gets defeated by --dry-run' ' > + rm -f .git/FETCH_HEAD && > + git -c fetch.writeFetchHEAD=yes fetch --dry-run . && > + ! test -f .git/FETCH_HEAD > +' > + > +test_expect_success 'fetch.writeFetchHEAD and --no-write-fetch-head' ' > + rm -f .git/FETCH_HEAD && > + git -c fetch.writeFetchHEAD=yes fetch --no-write-fetch-head . && > + ! test -f .git/FETCH_HEAD > +' > + > +test_expect_success 'fetch.writeFetchHEAD and --write-fetch-head' ' > + rm -f .git/FETCH_HEAD && > + git -c fetch.writeFetchHEAD=no fetch --write-fetch-head . && > + test -f .git/FETCH_HEAD > +' > + > test_expect_success "should be able to fetch with duplicate refspecs" ' > mkdir dups && > ( >
On 29/07/2020 10:12, Phillip Wood wrote: > On 28/07/2020 17:37, Junio C Hamano wrote: >> Derrick Stolee <stolee@gmail.com> writes: >> >>> On 7/27/2020 12:13 PM, Junio C Hamano wrote: >>>> Derrick Stolee <stolee@gmail.com> writes: >>>> >>>>> I'll rebase onto jc/no-update-fetch-head for the next version, since >>>>> that branch is based on v2.28.0-rc0, which is recent enough. >>>> >>>> I do not think it is wise to base a work on top of unfinished "you >>>> could do it this way, perhaps?" demonstration patch the original >>>> author does not have much inclination to finish, though. >>>> >>>> When I am really bored, I may go back to the topic to finish it, but >>>> I wouldn't mind if you took ownership of it at all. >>> >>> Ah. I didn't understand the status of that branch. I'll pull it in >>> to this topic. >> >> So here is with one of the two things that I found missing in the >> first iteration of the patch: documentation. >> >> The other thing that I found iffy (and still missing from this >> version) was what should be done when "git pull" is explicitly given >> the "--no-write-fetch-head" option. >> >> I think (but didn't check the recent code) that 'git pull' would >> pass only known-to-make-sense command line options to underlying >> 'git fetch', so it probably will barf with "unknown option", which >> is the best case. We might want to make it sure with a new test in >> 5521. On the other hand, if we get anything other than "no such >> option", we may want to think if we want to "fix" it or just leave >> it inside "if it hurts, don't do it" territory. >> >> Thanks. >> >> The patch without doc was Reviewed-by: Taylor Blau <me@ttaylorr.com> >> but this round has not been. >> >> -- >8 -- >> >> If you run fetch but record the result in remote-tracking branches, >> and either if you do nothing with the fetched refs (e.g. you are >> merely mirroring) or if you always work from the remote-tracking >> refs (e.g. you fetch and then merge origin/branchname separately), >> you can get away with having no FETCH_HEAD at all. >> >> Teach "git fetch" a command line option "--[no-]write-fetch-head" >> and "fetch.writeFetchHEAD" configuration variable. Without either, >> the default is to write FETCH_HEAD, and the usual rule that the >> command line option defeats configured default applies. >> >> Note that under "--dry-run" mode, FETCH_HEAD is never written; >> otherwise you'd see list of objects in the file that you do not >> actually have. Passing `--fetch-write-head` > > Typo, it should be `--write-fetch-head` > >> does not force `git >> fetch` to write the file. >> >> Also note that this option is explicitly passed when "git pull" >> internally invokes "git fetch", so that those who configured their >> "git fetch" not to write FETCH_HEAD would not be able to break the >> cooperation between these two commands. "git pull" must see what >> "git fetch" got recorded in FETCH_HEAD to work correctly. >> >> Signed-off-by: Junio C Hamano <gitster@pobox.com> >> --- >> Documentation/config/fetch.txt | 7 ++++++ >> Documentation/fetch-options.txt | 10 +++++++++ >> builtin/fetch.c | 19 +++++++++++++--- >> builtin/pull.c | 3 ++- >> t/t5510-fetch.sh | 39 +++++++++++++++++++++++++++++++-- >> 5 files changed, 72 insertions(+), 6 deletions(-) >> >> diff --git a/Documentation/config/fetch.txt >> b/Documentation/config/fetch.txt >> index b20394038d..0aaa05e8c0 100644 >> --- a/Documentation/config/fetch.txt >> +++ b/Documentation/config/fetch.txt >> @@ -91,3 +91,10 @@ fetch.writeCommitGraph:: >> merge and the write may take longer. Having an updated commit-graph >> file helps performance of many Git commands, including `git >> merge-base`, >> `git push -f`, and `git log --graph`. Defaults to false. >> + >> +fetch.writeFetchHEAD:: >> + Setting it to false tells `git fetch` not to write the list >> + of remote refs fetched in the `FETCH_HEAD` file directly >> + under `$GIT_DIR`. Can be countermanded from the command >> + line with the `--[no-]write-fetch-head` option. Defaults to >> + true. >> diff --git a/Documentation/fetch-options.txt >> b/Documentation/fetch-options.txt >> index 6e2a160a47..6775e8499f 100644 >> --- a/Documentation/fetch-options.txt >> +++ b/Documentation/fetch-options.txt >> @@ -64,6 +64,16 @@ documented in linkgit:git-config[1]. >> --dry-run:: >> Show what would be done, without making any changes. >> +ifndef::git-pull[] >> +--[no-]write-fetch-head:: >> + Write the list of remote refs fetched in the `FETCH_HEAD` >> + file directly under `$GIT_DIR`. This is the default unless >> + the configuration variable `fetch.writeFetchHEAD` is set to >> + false. Passing `--no-write-fetch-head` from the command >> + line tells Git not to write the file. Under `--dry-run` >> + option, the file is never written. >> +endif::git-pull[] >> + >> -f:: >> --force:: >> When 'git fetch' is used with `<src>:<dst>` refspec it may >> diff --git a/builtin/fetch.c b/builtin/fetch.c >> index 82ac4be8a5..3ccf69753f 100644 >> --- a/builtin/fetch.c >> +++ b/builtin/fetch.c >> @@ -56,6 +56,7 @@ static int prune_tags = -1; /* unspecified */ >> #define PRUNE_TAGS_BY_DEFAULT 0 /* do we prune tags by default? */ >> static int all, append, dry_run, force, keep, multiple, update_head_ok; >> +static int write_fetch_head = 1; >> static int verbosity, deepen_relative, set_upstream; >> static int progress = -1; >> static int enable_auto_gc = 1; >> @@ -118,6 +119,10 @@ static int git_fetch_config(const char *k, const >> char *v, void *cb) >> return 0; >> } >> + if (!strcmp(k, "fetch.writefetchhead")) { >> + write_fetch_head = git_config_bool(k, v); >> + return 0; >> + } >> return git_default_config(k, v, cb); >> } >> @@ -162,6 +167,8 @@ static struct option builtin_fetch_options[] = { >> PARSE_OPT_OPTARG, option_fetch_parse_recurse_submodules), >> OPT_BOOL(0, "dry-run", &dry_run, >> N_("dry run")), >> + OPT_BOOL(0, "write-fetch-head", &write_fetch_head, >> + N_("write fetched references to the FETCH_HEAD file")), >> OPT_BOOL('k', "keep", &keep, N_("keep downloaded pack")), >> OPT_BOOL('u', "update-head-ok", &update_head_ok, >> N_("allow updating of HEAD ref")), >> @@ -893,7 +900,9 @@ static int store_updated_refs(const char *raw_url, >> const char *remote_name, >> const char *what, *kind; >> struct ref *rm; >> char *url; >> - const char *filename = dry_run ? "/dev/null" : >> git_path_fetch_head(the_repository); >> + const char *filename = (!write_fetch_head >> + ? "/dev/null" >> + : git_path_fetch_head(the_repository)); > > I was suspicious of this as we haven't cleared write_fetch_head in the > --dry-run case yet but the test below seems to show that we still don't > write FETCH_HEAD if --dry-run is given. That makes we wonder what the > point of setting the filename based on the value of write_fetch_head is. Sorry ignore that. I misread the hunk header - we have in fact cleared write_fetch_head in the --dry-run case by the time we get here. > The rest looks good to me, though it might be worth having a test to > check that pull does indeed reject --no-write-fetch-head > > Best Wishes > > Phillip > >> int want_status; >> int summary_width = transport_summary_width(ref_map); >> @@ -1327,7 +1336,7 @@ static int do_fetch(struct transport *transport, >> } >> /* if not appending, truncate FETCH_HEAD */ >> - if (!append && !dry_run) { >> + if (!append && write_fetch_head) { >> retcode = truncate_fetch_head(); >> if (retcode) >> goto cleanup; >> @@ -1594,7 +1603,7 @@ static int fetch_multiple(struct string_list >> *list, int max_children) >> int i, result = 0; >> struct argv_array argv = ARGV_ARRAY_INIT; >> - if (!append && !dry_run) { >> + if (!append && write_fetch_head) { >> int errcode = truncate_fetch_head(); >> if (errcode) >> return errcode; >> @@ -1795,6 +1804,10 @@ int cmd_fetch(int argc, const char **argv, >> const char *prefix) >> if (depth || deepen_since || deepen_not.nr) >> deepen = 1; >> + /* FETCH_HEAD never gets updated in --dry-run mode */ >> + if (dry_run) >> + write_fetch_head = 0; >> + >> if (all) { >> if (argc == 1) >> die(_("fetch --all does not take a repository argument")); >> diff --git a/builtin/pull.c b/builtin/pull.c >> index 8159c5d7c9..e988d92b53 100644 >> --- a/builtin/pull.c >> +++ b/builtin/pull.c >> @@ -527,7 +527,8 @@ static int run_fetch(const char *repo, const char >> **refspecs) >> struct argv_array args = ARGV_ARRAY_INIT; >> int ret; >> - argv_array_pushl(&args, "fetch", "--update-head-ok", NULL); >> + argv_array_pushl(&args, "fetch", "--update-head-ok", >> + "--write-fetch-head", NULL); >> /* Shared options */ >> argv_push_verbosity(&args); >> diff --git a/t/t5510-fetch.sh b/t/t5510-fetch.sh >> index a66dbe0bde..3052c2d8d5 100755 >> --- a/t/t5510-fetch.sh >> +++ b/t/t5510-fetch.sh >> @@ -539,13 +539,48 @@ test_expect_success 'fetch into the current >> branch with --update-head-ok' ' >> ' >> -test_expect_success 'fetch --dry-run' ' >> - >> +test_expect_success 'fetch --dry-run does not touch FETCH_HEAD' ' >> rm -f .git/FETCH_HEAD && >> git fetch --dry-run . && >> ! test -f .git/FETCH_HEAD >> ' >> +test_expect_success '--no-write-fetch-head does not touch FETCH_HEAD' ' >> + rm -f .git/FETCH_HEAD && >> + git fetch --no-write-fetch-head . && >> + ! test -f .git/FETCH_HEAD >> +' >> + >> +test_expect_success '--write-fetch-head gets defeated by --dry-run' ' >> + rm -f .git/FETCH_HEAD && >> + git fetch --dry-run --write-fetch-head . && >> + ! test -f .git/FETCH_HEAD >> +' >> + >> +test_expect_success 'fetch.writeFetchHEAD and FETCH_HEAD' ' >> + rm -f .git/FETCH_HEAD && >> + git -c fetch.writeFetchHEAD=no fetch . && >> + ! test -f .git/FETCH_HEAD >> +' >> + >> +test_expect_success 'fetch.writeFetchHEAD gets defeated by --dry-run' ' >> + rm -f .git/FETCH_HEAD && >> + git -c fetch.writeFetchHEAD=yes fetch --dry-run . && >> + ! test -f .git/FETCH_HEAD >> +' >> + >> +test_expect_success 'fetch.writeFetchHEAD and --no-write-fetch-head' ' >> + rm -f .git/FETCH_HEAD && >> + git -c fetch.writeFetchHEAD=yes fetch --no-write-fetch-head . && >> + ! test -f .git/FETCH_HEAD >> +' >> + >> +test_expect_success 'fetch.writeFetchHEAD and --write-fetch-head' ' >> + rm -f .git/FETCH_HEAD && >> + git -c fetch.writeFetchHEAD=no fetch --write-fetch-head . && >> + test -f .git/FETCH_HEAD >> +' >> + >> test_expect_success "should be able to fetch with duplicate refspecs" ' >> mkdir dups && >> ( >>
On 7/28/2020 12:37 PM, Junio C Hamano wrote: > Derrick Stolee <stolee@gmail.com> writes: > >> On 7/27/2020 12:13 PM, Junio C Hamano wrote: >>> Derrick Stolee <stolee@gmail.com> writes: >>> >>>> I'll rebase onto jc/no-update-fetch-head for the next version, since >>>> that branch is based on v2.28.0-rc0, which is recent enough. >>> >>> I do not think it is wise to base a work on top of unfinished "you >>> could do it this way, perhaps?" demonstration patch the original >>> author does not have much inclination to finish, though. >>> >>> When I am really bored, I may go back to the topic to finish it, but >>> I wouldn't mind if you took ownership of it at all. >> >> Ah. I didn't understand the status of that branch. I'll pull it in >> to this topic. > > So here is with one of the two things that I found missing in the > first iteration of the patch: documentation. > > The other thing that I found iffy (and still missing from this > version) was what should be done when "git pull" is explicitly given > the "--no-write-fetch-head" option. > > I think (but didn't check the recent code) that 'git pull' would > pass only known-to-make-sense command line options to underlying > 'git fetch', so it probably will barf with "unknown option", which > is the best case. We might want to make it sure with a new test in > 5521. On the other hand, if we get anything other than "no such > option", we may want to think if we want to "fix" it or just leave > it inside "if it hurts, don't do it" territory. Here is the version I applied and updated. Please let me know what you think. -->8-- From 3f60a0f0fd388447aa9c2b805b5646039df98d0b Mon Sep 17 00:00:00 2001 From: Junio C Hamano <gitster@pobox.com> Date: Tue, 28 Jul 2020 09:37:59 -0700 Subject: [PATCH] fetch: optionally allow disabling FETCH_HEAD update If you run fetch but record the result in remote-tracking branches, and either if you do nothing with the fetched refs (e.g. you are merely mirroring) or if you always work from the remote-tracking refs (e.g. you fetch and then merge origin/branchname separately), you can get away with having no FETCH_HEAD at all. Teach "git fetch" a command line option "--[no-]write-fetch-head" and "fetch.writeFetchHEAD" configuration variable. Without either, the default is to write FETCH_HEAD, and the usual rule that the command line option defeats configured default applies. Note that under "--dry-run" mode, FETCH_HEAD is never written; otherwise you'd see list of objects in the file that you do not actually have. Passing `--write-fetch-head` does not force `git fetch` to write the file. Also note that this option is explicitly passed when "git pull" internally invokes "git fetch", so that those who configured their "git fetch" not to write FETCH_HEAD would not be able to break the cooperation between these two commands. "git pull" must see what "git fetch" got recorded in FETCH_HEAD to work correctly. Signed-off-by: Junio C Hamano <gitster@pobox.com> Signed-off-by: Derrick Stolee <dstolee@microsoft.com> --- Documentation/config/fetch.txt | 7 ++++++ Documentation/fetch-options.txt | 10 +++++++++ builtin/fetch.c | 19 +++++++++++++--- builtin/pull.c | 3 ++- t/t5510-fetch.sh | 39 +++++++++++++++++++++++++++++++-- t/t5521-pull-options.sh | 16 ++++++++++++++ 6 files changed, 88 insertions(+), 6 deletions(-) diff --git a/Documentation/config/fetch.txt b/Documentation/config/fetch.txt index b20394038d1..0aaa05e8c0e 100644 --- a/Documentation/config/fetch.txt +++ b/Documentation/config/fetch.txt @@ -91,3 +91,10 @@ fetch.writeCommitGraph:: merge and the write may take longer. Having an updated commit-graph file helps performance of many Git commands, including `git merge-base`, `git push -f`, and `git log --graph`. Defaults to false. + +fetch.writeFetchHEAD:: + Setting it to false tells `git fetch` not to write the list + of remote refs fetched in the `FETCH_HEAD` file directly + under `$GIT_DIR`. Can be countermanded from the command + line with the `--[no-]write-fetch-head` option. Defaults to + true. diff --git a/Documentation/fetch-options.txt b/Documentation/fetch-options.txt index 495bc8ab5a1..6972ad2522c 100644 --- a/Documentation/fetch-options.txt +++ b/Documentation/fetch-options.txt @@ -64,6 +64,16 @@ documented in linkgit:git-config[1]. --dry-run:: Show what would be done, without making any changes. +ifndef::git-pull[] +--[no-]write-fetch-head:: + Write the list of remote refs fetched in the `FETCH_HEAD` + file directly under `$GIT_DIR`. This is the default unless + the configuration variable `fetch.writeFetchHEAD` is set to + false. Passing `--no-write-fetch-head` from the command + line tells Git not to write the file. Under `--dry-run` + option, the file is never written. +endif::git-pull[] + -f:: --force:: When 'git fetch' is used with `<src>:<dst>` refspec it may diff --git a/builtin/fetch.c b/builtin/fetch.c index d8253f66e4c..1d7194aac26 100644 --- a/builtin/fetch.c +++ b/builtin/fetch.c @@ -56,6 +56,7 @@ static int prune_tags = -1; /* unspecified */ #define PRUNE_TAGS_BY_DEFAULT 0 /* do we prune tags by default? */ static int all, append, dry_run, force, keep, multiple, update_head_ok; +static int write_fetch_head = 1; static int verbosity, deepen_relative, set_upstream; static int progress = -1; static int enable_auto_gc = 1; @@ -118,6 +119,10 @@ static int git_fetch_config(const char *k, const char *v, void *cb) return 0; } + if (!strcmp(k, "fetch.writefetchhead")) { + write_fetch_head = git_config_bool(k, v); + return 0; + } return git_default_config(k, v, cb); } @@ -162,6 +167,8 @@ static struct option builtin_fetch_options[] = { PARSE_OPT_OPTARG, option_fetch_parse_recurse_submodules), OPT_BOOL(0, "dry-run", &dry_run, N_("dry run")), + OPT_BOOL(0, "write-fetch-head", &write_fetch_head, + N_("write fetched references to the FETCH_HEAD file")), OPT_BOOL('k', "keep", &keep, N_("keep downloaded pack")), OPT_BOOL('u', "update-head-ok", &update_head_ok, N_("allow updating of HEAD ref")), @@ -895,7 +902,9 @@ static int store_updated_refs(const char *raw_url, const char *remote_name, const char *what, *kind; struct ref *rm; char *url; - const char *filename = dry_run ? "/dev/null" : git_path_fetch_head(the_repository); + const char *filename = (!write_fetch_head + ? "/dev/null" + : git_path_fetch_head(the_repository)); int want_status; int summary_width = transport_summary_width(ref_map); @@ -1329,7 +1338,7 @@ static int do_fetch(struct transport *transport, } /* if not appending, truncate FETCH_HEAD */ - if (!append && !dry_run) { + if (!append && write_fetch_head) { retcode = truncate_fetch_head(); if (retcode) goto cleanup; @@ -1596,7 +1605,7 @@ static int fetch_multiple(struct string_list *list, int max_children) int i, result = 0; struct argv_array argv = ARGV_ARRAY_INIT; - if (!append && !dry_run) { + if (!append && write_fetch_head) { int errcode = truncate_fetch_head(); if (errcode) return errcode; @@ -1797,6 +1806,10 @@ int cmd_fetch(int argc, const char **argv, const char *prefix) if (depth || deepen_since || deepen_not.nr) deepen = 1; + /* FETCH_HEAD never gets updated in --dry-run mode */ + if (dry_run) + write_fetch_head = 0; + if (all) { if (argc == 1) die(_("fetch --all does not take a repository argument")); diff --git a/builtin/pull.c b/builtin/pull.c index 8159c5d7c96..e988d92b535 100644 --- a/builtin/pull.c +++ b/builtin/pull.c @@ -527,7 +527,8 @@ static int run_fetch(const char *repo, const char **refspecs) struct argv_array args = ARGV_ARRAY_INIT; int ret; - argv_array_pushl(&args, "fetch", "--update-head-ok", NULL); + argv_array_pushl(&args, "fetch", "--update-head-ok", + "--write-fetch-head", NULL); /* Shared options */ argv_push_verbosity(&args); diff --git a/t/t5510-fetch.sh b/t/t5510-fetch.sh index 9850ecde5df..31c91d0ed2e 100755 --- a/t/t5510-fetch.sh +++ b/t/t5510-fetch.sh @@ -539,13 +539,48 @@ test_expect_success 'fetch into the current branch with --update-head-ok' ' ' -test_expect_success 'fetch --dry-run' ' - +test_expect_success 'fetch --dry-run does not touch FETCH_HEAD' ' rm -f .git/FETCH_HEAD && git fetch --dry-run . && ! test -f .git/FETCH_HEAD ' +test_expect_success '--no-write-fetch-head does not touch FETCH_HEAD' ' + rm -f .git/FETCH_HEAD && + git fetch --no-write-fetch-head . && + ! test -f .git/FETCH_HEAD +' + +test_expect_success '--write-fetch-head gets defeated by --dry-run' ' + rm -f .git/FETCH_HEAD && + git fetch --dry-run --write-fetch-head . && + ! test -f .git/FETCH_HEAD +' + +test_expect_success 'fetch.writeFetchHEAD and FETCH_HEAD' ' + rm -f .git/FETCH_HEAD && + git -c fetch.writeFetchHEAD=no fetch . && + ! test -f .git/FETCH_HEAD +' + +test_expect_success 'fetch.writeFetchHEAD gets defeated by --dry-run' ' + rm -f .git/FETCH_HEAD && + git -c fetch.writeFetchHEAD=yes fetch --dry-run . && + ! test -f .git/FETCH_HEAD +' + +test_expect_success 'fetch.writeFetchHEAD and --no-write-fetch-head' ' + rm -f .git/FETCH_HEAD && + git -c fetch.writeFetchHEAD=yes fetch --no-write-fetch-head . && + ! test -f .git/FETCH_HEAD +' + +test_expect_success 'fetch.writeFetchHEAD and --write-fetch-head' ' + rm -f .git/FETCH_HEAD && + git -c fetch.writeFetchHEAD=no fetch --write-fetch-head . && + test -f .git/FETCH_HEAD +' + test_expect_success "should be able to fetch with duplicate refspecs" ' mkdir dups && ( diff --git a/t/t5521-pull-options.sh b/t/t5521-pull-options.sh index 159afa7ac81..1acae3b9a4f 100755 --- a/t/t5521-pull-options.sh +++ b/t/t5521-pull-options.sh @@ -77,6 +77,7 @@ test_expect_success 'git pull -q -v --no-rebase' ' test_must_be_empty out && test -s err) ' + test_expect_success 'git pull --cleanup errors early on invalid argument' ' mkdir clonedcleanup && (cd clonedcleanup && git init && @@ -85,6 +86,21 @@ test_expect_success 'git pull --cleanup errors early on invalid argument' ' test -s err) ' +test_expect_success 'git pull --no-write-fetch-head fails' ' + mkdir clonedwfh && + (cd clonedwfh && git init && + test_must_fail git pull --no-write-fetch-head "../parent" >out 2>err && + test_must_be_empty out && + test_i18ngrep "no-write-fetch-head" err) +' + +test_expect_success 'git pull succeeds with fetch.writeFetchHEAD=false' ' + mkdir clonedwfhconfig && + (cd clonedwfhconfig && git init && + git config fetch.writeFetchHEAD false && + git pull "../parent" >out 2>err && + grep FETCH_HEAD err) +' test_expect_success 'git pull --force' ' mkdir clonedoldstyle &&
diff --git a/Documentation/config/fetch.txt b/Documentation/config/fetch.txt index b20394038d..0aaa05e8c0 100644 --- a/Documentation/config/fetch.txt +++ b/Documentation/config/fetch.txt @@ -91,3 +91,10 @@ fetch.writeCommitGraph:: merge and the write may take longer. Having an updated commit-graph file helps performance of many Git commands, including `git merge-base`, `git push -f`, and `git log --graph`. Defaults to false. + +fetch.writeFetchHEAD:: + Setting it to false tells `git fetch` not to write the list + of remote refs fetched in the `FETCH_HEAD` file directly + under `$GIT_DIR`. Can be countermanded from the command + line with the `--[no-]write-fetch-head` option. Defaults to + true. diff --git a/Documentation/fetch-options.txt b/Documentation/fetch-options.txt index 6e2a160a47..6775e8499f 100644 --- a/Documentation/fetch-options.txt +++ b/Documentation/fetch-options.txt @@ -64,6 +64,16 @@ documented in linkgit:git-config[1]. --dry-run:: Show what would be done, without making any changes. +ifndef::git-pull[] +--[no-]write-fetch-head:: + Write the list of remote refs fetched in the `FETCH_HEAD` + file directly under `$GIT_DIR`. This is the default unless + the configuration variable `fetch.writeFetchHEAD` is set to + false. Passing `--no-write-fetch-head` from the command + line tells Git not to write the file. Under `--dry-run` + option, the file is never written. +endif::git-pull[] + -f:: --force:: When 'git fetch' is used with `<src>:<dst>` refspec it may diff --git a/builtin/fetch.c b/builtin/fetch.c index 82ac4be8a5..3ccf69753f 100644 --- a/builtin/fetch.c +++ b/builtin/fetch.c @@ -56,6 +56,7 @@ static int prune_tags = -1; /* unspecified */ #define PRUNE_TAGS_BY_DEFAULT 0 /* do we prune tags by default? */ static int all, append, dry_run, force, keep, multiple, update_head_ok; +static int write_fetch_head = 1; static int verbosity, deepen_relative, set_upstream; static int progress = -1; static int enable_auto_gc = 1; @@ -118,6 +119,10 @@ static int git_fetch_config(const char *k, const char *v, void *cb) return 0; } + if (!strcmp(k, "fetch.writefetchhead")) { + write_fetch_head = git_config_bool(k, v); + return 0; + } return git_default_config(k, v, cb); } @@ -162,6 +167,8 @@ static struct option builtin_fetch_options[] = { PARSE_OPT_OPTARG, option_fetch_parse_recurse_submodules), OPT_BOOL(0, "dry-run", &dry_run, N_("dry run")), + OPT_BOOL(0, "write-fetch-head", &write_fetch_head, + N_("write fetched references to the FETCH_HEAD file")), OPT_BOOL('k', "keep", &keep, N_("keep downloaded pack")), OPT_BOOL('u', "update-head-ok", &update_head_ok, N_("allow updating of HEAD ref")), @@ -893,7 +900,9 @@ static int store_updated_refs(const char *raw_url, const char *remote_name, const char *what, *kind; struct ref *rm; char *url; - const char *filename = dry_run ? "/dev/null" : git_path_fetch_head(the_repository); + const char *filename = (!write_fetch_head + ? "/dev/null" + : git_path_fetch_head(the_repository)); int want_status; int summary_width = transport_summary_width(ref_map); @@ -1327,7 +1336,7 @@ static int do_fetch(struct transport *transport, } /* if not appending, truncate FETCH_HEAD */ - if (!append && !dry_run) { + if (!append && write_fetch_head) { retcode = truncate_fetch_head(); if (retcode) goto cleanup; @@ -1594,7 +1603,7 @@ static int fetch_multiple(struct string_list *list, int max_children) int i, result = 0; struct argv_array argv = ARGV_ARRAY_INIT; - if (!append && !dry_run) { + if (!append && write_fetch_head) { int errcode = truncate_fetch_head(); if (errcode) return errcode; @@ -1795,6 +1804,10 @@ int cmd_fetch(int argc, const char **argv, const char *prefix) if (depth || deepen_since || deepen_not.nr) deepen = 1; + /* FETCH_HEAD never gets updated in --dry-run mode */ + if (dry_run) + write_fetch_head = 0; + if (all) { if (argc == 1) die(_("fetch --all does not take a repository argument")); diff --git a/builtin/pull.c b/builtin/pull.c index 8159c5d7c9..e988d92b53 100644 --- a/builtin/pull.c +++ b/builtin/pull.c @@ -527,7 +527,8 @@ static int run_fetch(const char *repo, const char **refspecs) struct argv_array args = ARGV_ARRAY_INIT; int ret; - argv_array_pushl(&args, "fetch", "--update-head-ok", NULL); + argv_array_pushl(&args, "fetch", "--update-head-ok", + "--write-fetch-head", NULL); /* Shared options */ argv_push_verbosity(&args); diff --git a/t/t5510-fetch.sh b/t/t5510-fetch.sh index a66dbe0bde..3052c2d8d5 100755 --- a/t/t5510-fetch.sh +++ b/t/t5510-fetch.sh @@ -539,13 +539,48 @@ test_expect_success 'fetch into the current branch with --update-head-ok' ' ' -test_expect_success 'fetch --dry-run' ' - +test_expect_success 'fetch --dry-run does not touch FETCH_HEAD' ' rm -f .git/FETCH_HEAD && git fetch --dry-run . && ! test -f .git/FETCH_HEAD ' +test_expect_success '--no-write-fetch-head does not touch FETCH_HEAD' ' + rm -f .git/FETCH_HEAD && + git fetch --no-write-fetch-head . && + ! test -f .git/FETCH_HEAD +' + +test_expect_success '--write-fetch-head gets defeated by --dry-run' ' + rm -f .git/FETCH_HEAD && + git fetch --dry-run --write-fetch-head . && + ! test -f .git/FETCH_HEAD +' + +test_expect_success 'fetch.writeFetchHEAD and FETCH_HEAD' ' + rm -f .git/FETCH_HEAD && + git -c fetch.writeFetchHEAD=no fetch . && + ! test -f .git/FETCH_HEAD +' + +test_expect_success 'fetch.writeFetchHEAD gets defeated by --dry-run' ' + rm -f .git/FETCH_HEAD && + git -c fetch.writeFetchHEAD=yes fetch --dry-run . && + ! test -f .git/FETCH_HEAD +' + +test_expect_success 'fetch.writeFetchHEAD and --no-write-fetch-head' ' + rm -f .git/FETCH_HEAD && + git -c fetch.writeFetchHEAD=yes fetch --no-write-fetch-head . && + ! test -f .git/FETCH_HEAD +' + +test_expect_success 'fetch.writeFetchHEAD and --write-fetch-head' ' + rm -f .git/FETCH_HEAD && + git -c fetch.writeFetchHEAD=no fetch --write-fetch-head . && + test -f .git/FETCH_HEAD +' + test_expect_success "should be able to fetch with duplicate refspecs" ' mkdir dups && (