fetch: add --no-update-remote-refs

From: Derrick Stolee <[email protected]>

To prevent long blocking time during a 'git fetch' call, a user may want to set up a schedule for background 'git fetch' processes. However, these runs will update the refs/remotes branches, and hence the user will not notice when remote refs are updated during their foreground fetches. In fact, they may _want_ those refs to stay put so they can work with the refs from their last foreground fetch call.

Add a --[no-]update-remote-refs option to 'git fetch' which defaults to the existing behavior of updating the remote refs. This allows a user to run

  git fetch <remote> --no-update-remote-refs +refs/heads/*:refs/hidden/*

to populate a custom ref space and download a pack of the new reachable objects. This kind of call allows a few things to happen:

1. We download a new pack if refs have updated.
2. Since the refs/hidden branches exist, GC will not remove the newly-downloaded data.
3. With fetch.writeCommitGraph enabled, the refs/hidden refs are used to update the commit-graph file.

To avoid the refs/hidden directory from filling without bound, the --prune option can be included. When providing a refspec like this, the --prune option does not delete remote refs and instead only deletes refs in the target refspace.

Note: with the default refpsec, the --prune option will override the --no-update-remote-refs option and will delete the refs that do not exist on the remote.

---
fetch: add --no-update-remote-refs

Here is a new feature for git fetch that hopefully is useful to some users. We've been using a patch like this in microsoft/git for about a month now, and I've been testing it locally using the custom refspec mentioned in the commit message. It's quite refreshing to run git fetch --all in my Git repo and see all the branch updates but not actually wait for any pack downloads.

There is one question about how --prune and --no-update-remote-refs should interact. Since --prune is not the default, and it works the way I'd like with a non-default refspec, I'm currently proposing allowing it to delete remote refs even if --no-update-remote-refs is provided.

Thanks, -Stolee

Published-As: https://github.com/gitgitgadget/git/releases/tag/pr-532%2Fderrickstolee%2Ffetch-no-update-remote-v1
Fetch-It-Via: git fetch https://github.com/gitgitgadget/git pr-532/derrickstolee/fetch-no-update-remote-v1
Pull-Request: https://github.com/gitgitgadget/git/pull/532

 Documentation/fetch-options.txt |  7 +++++++
 builtin/fetch.c                 |  6 ++++++
 t/t5510-fetch.sh                | 24 ++++++++++++++++++++++++
 3 files changed, 37 insertions(+)

index a2f78624a2..0939642dce 100644
@@ -254,6 +254,13 @@ endif::git-pull[]
 	'git-pull' the --ff-only option will still check for forced updates
 	before attempting a fast-forward update. See linkgit:git-config[1].
 
+--no-update-remote-refs::
+	By default, git updates the `refs/remotes/` refspace with the refs
+	advertised by the remotes during a `git fetch` command. With this
+	option, those refs will be ignored. If the `--prune` option is
+	specified and the default refpsec is used, then a ref that does not
+	appear in the remote will still be deleted from refs/remotes.
+
 -4::
 --ipv4::
 	Use IPv4 addresses only, ignoring IPv6 addresses.

index b4c6d921d0..bf8000adaf 100644
@@ -78,6 +78,7 @@ static struct list_objects_filter_options filter_options;
 static struct string_list server_options = STRING_LIST_INIT_DUP;
 static struct string_list negotiation_tip = STRING_LIST_INIT_NODUP;
 static int fetch_write_commit_graph = -1;
+static int update_remote_refs = 1;
 
 static int git_fetch_config(const char *k, const char *v, void *cb)
 {

@@ -201,6 +202,8 @@ static struct option builtin_fetch_options[] = {
 		 N_("check for forced-updates on all updated branches")),
 	OPT_BOOL(0, "write-commit-graph", &fetch_write_commit_graph,
 		 N_("write the commit-graph after fetching")),
+	OPT_BOOL(0, "update-remote-refs", &update_remote_refs,
+		 N_("update the refs/remotes/ refspace")),
 	OPT_END()
 };
 

@@ -746,6 +749,9 @@ static int update_local_ref(struct ref *ref,
 	const char *pretty_ref = prettify_refname(ref->name);
 	int fast_forward = 0;
 
+	if (!update_remote_refs && starts_with(ref->name, "refs/remotes/"))
+		return 0;
+
 	type = oid_object_info(the_repository, &ref->new_oid, NULL);
 	if (type < 0)
 		die(_("object %s not found"), oid_to_hex(&ref->new_oid));

index 4b60282689..35b50b2047 100755
@@ -174,6 +174,30 @@ test_expect_success 'fetch --prune --tags with refspec prunes based on refspec'
 	git rev-parse sometag
 '
 
+test_expect_success 'fetch --no-update-remote-refs keeps existing refs' '
+	cd "$TRASH_DIRECTORY" &&
+	git clone "$D" remote-refs &&
+	git -C remote-refs rev-parse remotes/origin/master >old &&
+	git -C remote-refs update-ref refs/remotes/origin/master master~1 &&
+	git -C remote-refs rev-parse remotes/origin/master >new &&
+	git -C remote-refs fetch --no-update-remote-refs origin &&
+	git -C remote-refs rev-parse remotes/origin/master >actual &&
+	test_cmp new actual &&
+	git -C remote-refs fetch origin &&
+	git -C remote-refs rev-parse remotes/origin/master >actual &&
+	test_cmp old actual
+'
+
+test_expect_success 'fetch --no-update-remote-refs --prune with refspec' '
+	git -C remote-refs update-ref refs/remotes/origin/foo/otherbranch master &&
+	git -C remote-refs update-ref refs/hidden/foo/otherbranch master &&
+	git -C remote-refs fetch --prune --no-update-remote-refs origin +refs/heads/*:refs/hidden/* &&
+	git -C remote-refs rev-parse remotes/origin/foo/otherbranch &&
+	test_must_fail git -C remote-refs rev-parse refs/hidden/foo/otherbranch &&
+	git -C remote-refs fetch --prune --no-update-remote-refs origin &&
+	test_must_fail git -C remote-refs rev-parse remotes/origin/foo/otherbranch
+'
+
 test_expect_success 'fetch tags when there is no tags' '
 
     cd "$D" &&

base-commit: d0654dc308b0ba76dd8ed7bbb33c8d8f7aacd783

gitgitgadget

I've always hated anything that makes the remote-tracking refs "float" and surprise end users. I even hated that 'git push' that pretends as if we immediately turned around and fetched from the remote we just pushed when it was introduced, even though I gave up by now.

So I am OK in principle to make it more difficult to update refs/remotes/* while the end-user is looking the other way, but I had to wonder why "git fetch" is even being done if it is done to silently update/catch-up remote-tracking branches automatically in the first place.

This is more like a "preload" option---without updating the end-user visible set of remote-tracking branches, you can make the data available earlier so that the actual "fetch" the end-user runs in order to update the remote-tracking branches can complete faster and become ready to be used more quickly.

Which makes sense.

Hmph. I have to wonder if this should have been the default. That is, when refs/heads/X on the remote is configured to be copied to refs/remotes/origin/X on this side, and an explicit refspec says it should go some other place (i.e. refs/hidden/X), shouldn't that automatically bypass configured +refs/heads/*:refs/remotes/origin/* refspec? In any case, it is too late to change that now.

Caution. Since you didn't make it "refs/hidden/<remote>/*", you just made the data you fetched the same way with this shiny new "--no-update-remote-tracking-branches" option from another remote unanchored and susceptible to GCs.

I have a moderately strong suspicion that it would be better to make this "--ignore-configured-refspecs" and implemented without special casign the "refs/remotes/" hierarchy like the code does by hardcoding.

I also wonder if auto-following of tags should be disabled at the same time. I have no good argument either way (yet).

Thanks.

It used to be the default. You can blame 2013-me. ;)

Before that, though, we had people complaining the other way ("I just fetched from the remote, but my tracking ref is stale!").

Yeah, I just independently wrote something similar. Your "--refmap" option can accomplish that already.

I _didn't_ think of tag auto-following in my other response. That's a good point. I think he'd probably just want to use "--no-tags" for the background fetch. You'd end up having to fetch the tag objects themselves during the "real" fetch, but presumably they're very small compared to the rest of history.

You could also do:

  git fetch --refmap= <remote> +refs/heads/*:refs/hidden/heads/* +refs/tags/*:refs/hidden/tags/*

to get everything in your pre-fetch (though you actually get more than the real fetch would need, since by default it won't grab tags which don't point to otherwise-reachable history).

-Peff

Yes, we get the pack data earlier, and that's the primary cost of the fetch command. We can also update the commit-graph using the hidden refs.

You're right. I neglected to say "refs/hidden/<remote>/*" in my message, but it _is_ something I've been doing in my background fetches.

Based on this and Peff's response, I think you are pointing me in a better direction with this. It should make the change less of a hack and also make it more general to users with custom refspecs.

Would ignoring the configured refspecs stop auto-following tags? I'll take a look and see. Otherwise, I'll add --no-tags to my background fetch command.

Thanks!
-Stolee

So refs/hidden is basically a parallel namespace that is exactly identical to refs/remotes/origin? It seems like a very roundabout way of solving the problem. Which, AFAICT, is just that you want "git fetch" to print out the set of updates since the last manual fetch. I suppose this also doesn't update what people see when they refer to "origin/master" until they run such a fetch.

I don't know. Part of me wants to say this is overly complicated. If you want to see the difference in refs between two states, then we should have some tool for showing that (even if it's git-fetch) without having to have this weird parallel ref structure. OTOH, I guess any such tool would need to save the ref state, which is equivalent to having all these parallel refs.

  git fetch --refmap= <remote> refs/heads/*:refs/hidden/*

You're absolutely right. There are details at [1].

The tricky thing is that the documentation makes it look like you need a value there, and if you supply a value without a refspec argument, an error is given:

$ git fetch origin --refmap="+refs/heads/*:refs/hidden/origin/*"
fatal: --refmap option is only meaningful with command-line refspec(s).

I think I'll follow up with a documentation patch to point out that an empty --refmap value will ignore the configured refspec and rely only on the provided refspec argument.

Thanks!
-Stolee

[1] https://git-scm.com/docs/git-fetch#_configured_remote_tracking_branches_a_id_crtb_a

From: Derrick Stolee <[email protected]>

To prevent long blocking time during a 'git fetch' call, a user may want to set up a schedule for background 'git fetch' processes. However, these runs will update the refs/remotes branches due to the default refspec set in the config when Git adds a remote. Hence the user will not notice when remote refs are updated during their foreground fetches. In fact, they may _want_ those refs to stay put so they can work with the refs from their last foreground fetch call.

This can be accomplished by overriding the configured refspec using '--refmap=' along with a custom refspec:

  git fetch <remote> --refmap= +refs/heads/*:refs/hidden/<remote>/*

to populate a custom ref space and download a pack of the new reachable objects. This kind of call allows a few things to happen:

1. We download a new pack if refs have updated.
2. Since the refs/hidden branches exist, GC will not remove the newly-downloaded data.
3. With fetch.writeCommitGraph enabled, the refs/hidden refs are used to update the commit-graph file.

To avoid the refs/hidden directory from filling without bound, the --prune option can be included. When providing a refspec like this, the --prune option does not delete remote refs and instead only deletes refs in the target refspace.

Update the documentation to clarify how '--refmap=""' works and create tests to guarantee this behavior remains in the future.

---
fetch: document and test '--refmap=""'

Thanks for all the feedback leading to absolutely no code change. It's good we already have the flexibility for this. I'm a bit embarrassed that I did not discover this, so perhaps this doc change and new tests will help clarify the behavior.

Thanks, -Stolee

Published-As: https://github.com/gitgitgadget/git/releases/tag/pr-532%2Fderrickstolee%2Ffetch-no-update-remote-v2
Fetch-It-Via: git fetch https://github.com/gitgitgadget/git pr-532/derrickstolee/fetch-no-update-remote-v2
Pull-Request: https://github.com/gitgitgadget/git/pull/532

Range-diff vs v1:

1: 26f13ea5f5 ! 1: 51c6221a31 fetch: add --no-update-remote-refs
@@ -1,20 +1,20 @@
Author: Derrick Stolee <[email protected]>

- fetch: add --no-update-remote-refs
+ fetch: document and test --refmap=""

To prevent long blocking time during a 'git fetch' call, a user may want to set up a schedule for background 'git fetch' processes.
- However, these runs will update the refs/remotes branches, and
- hence the user will not notice when remote refs are updated during
+ However, these runs will update the refs/remotes branches due to
+ the default refspec set in the config when Git adds a remote.
+ Hence the user will not notice when remote refs are updated during
their foreground fetches. In fact, they may _want_ those refs to stay put so they can work with the refs from their last foreground fetch call.

- Add a --[no-]update-remote-refs option to 'git fetch' which defaults
- to the existing behavior of updating the remote refs. This allows
- a user to run
+ This can be accomplished by overriding the configured refspec using
+ '--refmap=' along with a custom refspec:

- git fetch <remote> --no-update-remote-refs +refs/heads/*:refs/hidden/*
+ git fetch <remote> --refmap= +refs/heads/*:refs/hidden/<remote>/*

to populate a custom ref space and download a pack of the new reachable objects. This kind of call allows a few things to happen:
@@ -30,9 +30,8 @@
the --prune option does not delete remote refs and instead only deletes refs in the target refspace.

- Note: with the default refpsec, the --prune option will override
- the --no-update-remote-refs option and will delete the refs that
- do not exist on the remote.
+ Update the documentation to clarify how '--refmap=""' works and
+ create tests to guarantee this behavior remains in the future.

@@ -40,50 +39,17 @@
--- a/Documentation/fetch-options.txt
+++ b/Documentation/fetch-options.txt
@@
- 'git-pull' the --ff-only option will still check for forced updates
- before attempting a fast-forward update. See linkgit:git-config[1].
+ specified refspec (can be given more than once) to map the
+ refs to remote-tracking branches, instead of the values of
+ remote.*.fetch configuration variables for the remote
+- repository. See section on "Configured Remote-tracking
++ repository. Providing an empty <refspec> to the
++ --refmap option causes Git to ignore the configured
++ refspecs and rely entirely on the refspecs supplied as
++ command-line arguments. See section on "Configured Remote-tracking
+ Branches" for details.

-+--no-update-remote-refs::
-+ By default, git updates the refs/remotes/ refspace with the refs
-+ advertised by the remotes during a git fetch command. With this
-+ option, those refs will be ignored. If the --prune option is
-+ specified and the default refpsec is used, then a ref that does not
-+ appear in the remote will still be deleted from refs/remotes.
-+
- -4::
- --ipv4::
- Use IPv4 addresses only, ignoring IPv6 addresses.
-
- diff --git a/builtin/fetch.c b/builtin/fetch.c
- --- a/builtin/fetch.c
- +++ b/builtin/fetch.c
-@@
- static struct string_list server_options = STRING_LIST_INIT_DUP;
- static struct string_list negotiation_tip = STRING_LIST_INIT_NODUP;
- static int fetch_write_commit_graph = -1;
-+static int update_remote_refs = 1;
-
- static int git_fetch_config(const char *k, const char *v, void *cb)
- {
-@@
- N_("check for forced-updates on all updated branches")),
- OPT_BOOL(0, "write-commit-graph", &fetch_write_commit_graph,
- N_("write the commit-graph after fetching")),
-+ OPT_BOOL(0, "update-remote-refs", &update_remote_refs,
-+ N_("update the refs/remotes/ refspace")),
- OPT_END()
- };
-
-@@
- const char *pretty_ref = prettify_refname(ref->name);
- int fast_forward = 0;
-
-+ if (!update_remote_refs && starts_with(ref->name, "refs/remotes/"))
-+ return 0;
-+
- type = oid_object_info(the_repository, &ref->new_oid, NULL);
- if (type < 0)
- die(_("object %s not found"), oid_to_hex(&ref->new_oid));
+ -t::

diff --git a/t/t5510-fetch.sh b/t/t5510-fetch.sh --- a/t/t5510-fetch.sh
@@ -92,13 +58,13 @@
git rev-parse sometag
'

-+test_expect_success 'fetch --no-update-remote-refs keeps existing refs' ' ++test_expect_success '--refmap="" ignores configured refspec' '
+ cd "$TRASH_DIRECTORY" &&
+ git clone "$D" remote-refs &&
+ git -C remote-refs rev-parse remotes/origin/master >old &&
+ git -C remote-refs update-ref refs/remotes/origin/master master~1 &&
+ git -C remote-refs rev-parse remotes/origin/master >new &&
-+ git -C remote-refs fetch --no-update-remote-refs origin &&
++ git -C remote-refs fetch --refmap= origin "+refs/heads/*:refs/hidden/origin/*" &&
+ git -C remote-refs rev-parse remotes/origin/master >actual &&
+ test_cmp new actual &&
+ git -C remote-refs fetch origin &&
@@ -106,13 +72,13 @@
+ test_cmp old actual
+'
+
-+test_expect_success 'fetch --no-update-remote-refs --prune with refspec' ' ++test_expect_success '--refmap="" and --prune' '
+ git -C remote-refs update-ref refs/remotes/origin/foo/otherbranch master &&
+ git -C remote-refs update-ref refs/hidden/foo/otherbranch master &&
-+ git -C remote-refs fetch --prune --no-update-remote-refs origin +refs/heads/*:refs/hidden/* &&
++ git -C remote-refs fetch --prune --refmap="" origin +refs/heads/*:refs/hidden/* &&
+ git -C remote-refs rev-parse remotes/origin/foo/otherbranch &&
+ test_must_fail git -C remote-refs rev-parse refs/hidden/foo/otherbranch &&
-+ git -C remote-refs fetch --prune --no-update-remote-refs origin &&
++ git -C remote-refs fetch --prune origin &&
+ test_must_fail git -C remote-refs rev-parse remotes/origin/foo/otherbranch
+'
+

 Documentation/fetch-options.txt |  5 ++++-
 t/t5510-fetch.sh                | 24 ++++++++++++++++++++++++
 2 files changed, 28 insertions(+), 1 deletion(-)

index a2f78624a2..a115a1ae0e 100644
@@ -139,7 +139,10 @@ ifndef::git-pull[]
 	specified refspec (can be given more than once) to map the
 	refs to remote-tracking branches, instead of the values of
 	`remote.*.fetch` configuration variables for the remote
-	repository.  See section on "Configured Remote-tracking
+	repository.  Providing an empty `<refspec>` to the
+	`--refmap` option causes Git to ignore the configured
+	refspecs and rely entirely on the refspecs supplied as
+	command-line arguments. See section on "Configured Remote-tracking
 	Branches" for details.
 
 -t::

index 4b60282689..5f8f1c287f 100755
@@ -174,6 +174,30 @@ test_expect_success 'fetch --prune --tags with refspec prunes based on refspec'
 	git rev-parse sometag
 '
 
+test_expect_success '--refmap="" ignores configured refspec' '
+	cd "$TRASH_DIRECTORY" &&
+	git clone "$D" remote-refs &&
+	git -C remote-refs rev-parse remotes/origin/master >old &&
+	git -C remote-refs update-ref refs/remotes/origin/master master~1 &&
+	git -C remote-refs rev-parse remotes/origin/master >new &&
+	git -C remote-refs fetch --refmap= origin "+refs/heads/*:refs/hidden/origin/*" &&
+	git -C remote-refs rev-parse remotes/origin/master >actual &&
+	test_cmp new actual &&
+	git -C remote-refs fetch origin &&
+	git -C remote-refs rev-parse remotes/origin/master >actual &&
+	test_cmp old actual
+'
+
+test_expect_success '--refmap="" and --prune' '
+	git -C remote-refs update-ref refs/remotes/origin/foo/otherbranch master &&
+	git -C remote-refs update-ref refs/hidden/foo/otherbranch master &&
+	git -C remote-refs fetch --prune --refmap="" origin +refs/heads/*:refs/hidden/* &&
+	git -C remote-refs rev-parse remotes/origin/foo/otherbranch &&
+	test_must_fail git -C remote-refs rev-parse refs/hidden/foo/otherbranch &&
+	git -C remote-refs fetch --prune origin &&
+	test_must_fail git -C remote-refs rev-parse remotes/origin/foo/otherbranch
+'
+
 test_expect_success 'fetch tags when there is no tags' '
 
     cd "$D" &&

base-commit: d0654dc308b0ba76dd8ed7bbb33c8d8f7aacd783

gitgitgadget

Yeah, this looks like a good solution to me.

This isn't strictly related to your patch, but since the rationale here describes the concept of a background job and people might end up using it as a reference, do you want to add in --no-tags to help them out?

If it makes you feel better, I only found --refmap because I was the one who implemented the original "update based on refspecs" logic, and while looking for that commit with "git log --grep=opportunistic" I stumbled onto Junio's commit adding --refmap, which referenced mine. Maybe this also works as a good case study in why we should write good commit messages and refer to related work. :)

Anyway, I wasn't at all sure that a blank --refmap= would do what you want until I tried it. But it was always intended to work that way. From c5558f80c3 (fetch: allow explicit --refmap to override configuration, 2014-05-29):

+static int parse_refmap_arg(const struct option *opt, const char *arg, int unset) +{
+ ALLOC_GROW(refmap_array, refmap_nr + 1, refmap_alloc);
+
+ /*

  +        * "git fetch --refmap='' origin foo"
  +        * can be used to tell the command not to store anywhere
  +        */

+ if (*arg)
+ refmap_array[refmap_nr++] = arg;
+ return 0;
+}

At first I thought the comment was wrong, since we don't actually increment refmap_nr. But the ALLOC_GROW makes refmap_array non-NULL, which is what triggers the "do not use configured refspecs" logic.

This code switched to refspec_append() in e4cffacc80 (fetch: convert refmap to use struct refspec, 2018-05-16), and I think we actually do push an empty string onto the list. Which then triggers the "do not use configured refspecs" logic, but doesn't match anything itself. I'm not sure whether that behavior was entirely planned, or just what the code happens to do. So it's doubly useful to add a test here covering the expected behavior.

The patch looks good to me.

-Peff

That's a good idea. I keep forgetting about that. It's interesting that tags are fetched even though my refpsec does not include refs/tags.

This works due to a subtle arrangement of things, like a non-NULL but "empty" array. That justifies the test even more.

Excellent. Glad I'm not just adding test bloat for now reason.

-Stolee

Yeah, because tag-following is an extra thing outside of the refspecs. I think it would be nice if there were a way to specify a "following" refspec, something like:

~refs/tags/*:refs/tags/*

(where "~" is just a character I picked out of the blue that I think doesn't have any other meaning there now). And then we could specify it in the config alongside other refspecs, override it with a refmap, etc.

As a bonus, it would also give us a stepping stone towards being able to do remote-specific tags like:

[remote "origin"]
url = ...
fetch = +refs/heads/*:refs/remotes/origin/heads/* fetch = ~refs/tags/*:refs/remotes/origin/tags/*

But I'm sure there are a lot of backwards-compatibility gotchas.

-Peff

Also, is the order of the arguments correct? I would have expected to see <remote> after all dashed options and before refspecs.

;-)

Good analysis, and also the answer to my previous question is also there ;-)

Thanks. Looks quite good.

Will queue.