FETCH_HEAD files and mirrored repos

FETCH_HEAD files and mirrored repos

[Konstantin Ryabitsev]

Hi, all:

Are there any downsides to deleting FETCH_HEAD after performing "git 
remote update" in repos that are only used for mirrored hosting? E.g.  
source.codeaurora.org has thousands of repos where various Android/CAF 
automation creates tons of refs, so FETCH_HEAD files are routinely many 
MB in size:

$ ls -ahl FETCH_HEAD
-rw-rw-r--. 1 mricon mricon 4.4M Jul 11 04:28 FETCH_HEAD
$ wc -l FETCH_HEAD
29122 FETCH_HEAD

As far as I know, FETCH_HEAD info is only used in local-repo operations, 
so there should be no downside to deleting these files when git is only 
used for mirroring -- but I wanted to check in case I'm wrong.

TIA,
-K

Re: FETCH_HEAD files and mirrored repos

[Junio C Hamano]

Konstantin Ryabitsev <konstantin@linuxfoundation.org> writes:

> Are there any downsides to deleting FETCH_HEAD after performing "git 
> remote update" in repos that are only used for mirrored hosting?

The only time Git itself uses FETCH_HEAD is during "git pull".

"git pull" is a two step process---it first calls "git fetch" and
then calls "git merge" (or "git rebase") to integrate the fetched
history with the history of your current branch.  "git fetch" leaves
what it got in that file, and "git pull" figures out what parameters
and message to use to drive the second step.  After that, FETCH_HEAD
is not used by Git.  If you call "git fetch" yourself, FETCH_HEAD
left by that process is not used by Git itself, but you can use what
is in it to manually execute the second step of what "git pull"
would do.

So, unless your script depends on the presence and/or the contents
of FETCH_HEAD, you can safely remove it.

Re: FETCH_HEAD files and mirrored repos

[Konstantin Ryabitsev]

On Sat, Jul 11, 2020 at 02:07:01PM -0700, Junio C Hamano wrote:
> So, unless your script depends on the presence and/or the contents
> of FETCH_HEAD, you can safely remove it.

Excellent, that just saved me 20G per each mirror. :)

Thanks,
-K

Re: FETCH_HEAD files and mirrored repos

[Junio C Hamano]

Konstantin Ryabitsev <konstantin@linuxfoundation.org> writes:

> On Sat, Jul 11, 2020 at 02:07:01PM -0700, Junio C Hamano wrote:
>> So, unless your script depends on the presence and/or the contents
>> of FETCH_HEAD, you can safely remove it.
>
> Excellent, that just saved me 20G per each mirror. :)

Meaning that minimum 4k per file adds up that much due to 5 millions
of repositories, or something?

Re: FETCH_HEAD files and mirrored repos

[Konstantin Ryabitsev]

On Sun, Jul 12, 2020 at 10:33:09AM -0700, Junio C Hamano wrote:
> >> So, unless your script depends on the presence and/or the contents
> >> of FETCH_HEAD, you can safely remove it.
> >
> > Excellent, that just saved me 20G per each mirror. :)
> 
> Meaning that minimum 4k per file adds up that much due to 5 millions
> of repositories, or something?

Gosh, no. Only 28,000 repositories, with many FETCH_HEAD files being a 
few MB in size.

I also discovered that symlinking FETCH_HEAD to /dev/null works as well 
and thus saves a few IO cycles.

-K

Re: FETCH_HEAD files and mirrored repos

[Junio C Hamano]

Konstantin Ryabitsev <konstantin@linuxfoundation.org> writes:

> I also discovered that symlinking FETCH_HEAD to /dev/null works as well 
> and thus saves a few IO cycles.

We could teach "git fetch" a new "--write-fetch-head" option and
teach "git pull" to pass it when calling "git fetch".  It is very
plausible that some folks rely on "git fetch" to leave FETCH_HEAD
without being told, so the option must default to on for a few
development cycles before we flip the default to off, and for those
who want to live in the future a bit earlier, we can also introduce
fetch.writeFetchHEAD configuration variable that determines what
happens when "git fetch" is run without "--write-fetch-head" option
on the command line.  That way, you could drop 

	[fetch]
		writeFetchHEAD = 0

in say /etc/gitconfig or in ~/.gitconfig of the user that runs the
mirroring automation.

Re: FETCH_HEAD files and mirrored repos

[Konstantin Ryabitsev]

On Sun, Jul 12, 2020 at 01:52:17PM -0700, Junio C Hamano wrote:
> Konstantin Ryabitsev <konstantin@linuxfoundation.org> writes:
> 
> > I also discovered that symlinking FETCH_HEAD to /dev/null works as well 
> > and thus saves a few IO cycles.
> 
> We could teach "git fetch" a new "--write-fetch-head" option and
> teach "git pull" to pass it when calling "git fetch".  It is very
> plausible that some folks rely on "git fetch" to leave FETCH_HEAD
> without being told, so the option must default to on for a few
> development cycles before we flip the default to off, and for those
> who want to live in the future a bit earlier, we can also introduce
> fetch.writeFetchHEAD configuration variable that determines what
> happens when "git fetch" is run without "--write-fetch-head" option
> on the command line.  That way, you could drop 
> 
> 	[fetch]
> 		writeFetchHEAD = 0
> 
> in say /etc/gitconfig or in ~/.gitconfig of the user that runs the
> mirroring automation.

Yeah, that would be great for me. We can simply leave it on by default 
to preserve existing behaviour and just provide a way to disable writing 
it for those in fairly unique situations where that matters (like mine).  
One downside of symlinking it to /dev/null is that it creates a 
potential pitfall for software that expects FETCH_HEAD to always be a 
regular file and can behave unpredictably when it finds anything else 
there.

Thanks,
-K

Re: FETCH_HEAD files and mirrored repos

[Johannes Sixt]

Am 12.07.20 um 22:52 schrieb Junio C Hamano:
> We could teach "git fetch" a new "--write-fetch-head" option and
> teach "git pull" to pass it when calling "git fetch".  It is very
> plausible that some folks rely on "git fetch" to leave FETCH_HEAD
> without being told, so the option must default to on for a few
> development cycles before we flip the default to off,

Nah, really??? It's one of the benefits of git-fetch that it writes
FETCH_HEAD and the primary reason in many cases where I use the command!
So, either I don't care that FETCH_HEAD is written, or I do use it. IMO,
not wanting to write FETCH_HEAD is the odd case and would need a
configuration tweak, not the other way round.

-- Hannes

Re: FETCH_HEAD files and mirrored repos

[Junio C Hamano]

Johannes Sixt <j6t@kdbg.org> writes:

> Am 12.07.20 um 22:52 schrieb Junio C Hamano:
>> We could teach "git fetch" a new "--write-fetch-head" option and
>> teach "git pull" to pass it when calling "git fetch".  It is very
>> plausible that some folks rely on "git fetch" to leave FETCH_HEAD
>> without being told, so the option must default to on for a few
>> development cycles before we flip the default to off,
>
> Nah, really??? It's one of the benefits of git-fetch that it writes
> FETCH_HEAD and the primary reason in many cases where I use the command!
> So, either I don't care that FETCH_HEAD is written, or I do use it. IMO,
> not wanting to write FETCH_HEAD is the odd case and would need a
> configuration tweak, not the other way round.

Yeah, that's even easier to arrange.  

Just the "--[no-]write-fetch-head" command line option and the
fetch.writeFetchHEAD configuration variable are introduced and left
off by default forever.

[PATCH] fetch: optionally allow disabling FETCH_HEAD update

[Junio C Hamano]

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 without having 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.

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>
---

    Junio C Hamano <gitster@pobox.com> writes:

    > Johannes Sixt <j6t@kdbg.org> writes:
    >
    >> Nah, really??? It's one of the benefits of git-fetch that it writes
    >> FETCH_HEAD and the primary reason in many cases where I use the command!
    >> So, either I don't care that FETCH_HEAD is written, or I do use it. IMO,
    >> not wanting to write FETCH_HEAD is the odd case and would need a
    >> configuration tweak, not the other way round.
    >
    > Yeah, that's even easier to arrange.  
    >
    > Just the "--[no-]write-fetch-head" command line option and the
    > fetch.writeFetchHEAD configuration variable are introduced and left
    > off by default forever.

    Something like this, perhaps.  Obviously I won't be pushing this
    topic further while in prerelase freeze, but Konstantin or
    anybody else interested can locally apply the patch to their own
    copy of Git to test it out.

    Thanks.

 builtin/fetch.c  | 19 ++++++++++++++++---
 builtin/pull.c   |  3 ++-
 t/t5510-fetch.sh | 39 +++++++++++++++++++++++++++++++++++++--
 3 files changed, 55 insertions(+), 6 deletions(-)

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 &&
 	(
-- 
2.28.0-rc0

Re: [PATCH] fetch: optionally allow disabling FETCH_HEAD update

[Taylor Blau]

On Mon, Jul 13, 2020 at 11:06:09AM -0700, Junio C Hamano wrote:
> 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 without having 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.
>
> 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>
> ---
>
>     Junio C Hamano <gitster@pobox.com> writes:
>
>     > Johannes Sixt <j6t@kdbg.org> writes:
>     >
>     >> Nah, really??? It's one of the benefits of git-fetch that it writes
>     >> FETCH_HEAD and the primary reason in many cases where I use the command!
>     >> So, either I don't care that FETCH_HEAD is written, or I do use it. IMO,
>     >> not wanting to write FETCH_HEAD is the odd case and would need a
>     >> configuration tweak, not the other way round.
>     >
>     > Yeah, that's even easier to arrange.
>     >
>     > Just the "--[no-]write-fetch-head" command line option and the
>     > fetch.writeFetchHEAD configuration variable are introduced and left
>     > off by default forever.
>
>     Something like this, perhaps.  Obviously I won't be pushing this
>     topic further while in prerelase freeze, but Konstantin or
>     anybody else interested can locally apply the patch to their own
>     copy of Git to test it out.
>
>     Thanks.
>
>  builtin/fetch.c  | 19 ++++++++++++++++---
>  builtin/pull.c   |  3 ++-
>  t/t5510-fetch.sh | 39 +++++++++++++++++++++++++++++++++++++--
>  3 files changed, 55 insertions(+), 6 deletions(-)
>
> 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));

Ah, because dry_run ==> !write_fetch_head, so this is an equivalent
translation. Makes sense.

>  	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);

...and here we pass '--write-fetch-head' explicitly, because we don't
want a user who has set 'fetch.writeFetchHead' to 'false' to suddenly
have their 'git pull's stop working. Makes sense.

>
>  	/* 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 &&
>  	(

Test coverage all looks good, thanks for working on this. I don't think
there's anything left, so this would be great after 2.28 is released.

  Reviewed-by: Taylor Blau <me@ttaylorr.com>

> --
> 2.28.0-rc0
>

Thanks,
Taylor

Re: [PATCH] fetch: optionally allow disabling FETCH_HEAD update

[Junio C Hamano]

Taylor Blau <me@ttaylorr.com> writes:

>> -	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));
>
> Ah, because dry_run ==> !write_fetch_head, so this is an equivalent
> translation. Makes sense.

Yup, now dry_run is merely one of the two cases we turn
write_fetch_head off, and the logic to prevent us from writing
FETCH_HEAD (hopefully without any other side effect) was already
there in this function.

> Test coverage all looks good, thanks for working on this. I don't think
> there's anything left, so this would be great after 2.28 is released.
>
>   Reviewed-by: Taylor Blau <me@ttaylorr.com>

Thanks.

Re: FETCH_HEAD files and mirrored repos

[Konstantin Ryabitsev]

On Mon, Jul 13, 2020 at 10:13:28AM -0700, Junio C Hamano wrote:
> > Nah, really??? It's one of the benefits of git-fetch that it writes
> > FETCH_HEAD and the primary reason in many cases where I use the command!
> > So, either I don't care that FETCH_HEAD is written, or I do use it. IMO,
> > not wanting to write FETCH_HEAD is the odd case and would need a
> > configuration tweak, not the other way round.
> 
> Yeah, that's even easier to arrange.  
> 
> Just the "--[no-]write-fetch-head" command line option and the
> fetch.writeFetchHEAD configuration variable are introduced and left
> off by default forever.

Does it make sense to add logic for whether this is done in a bare repo?  
I can't imagine common cases where a FETCH_HEAD would be useful outside 
of a checkout where a merge is likely to happen.

-K

Re: FETCH_HEAD files and mirrored repos

[Junio C Hamano]

Konstantin Ryabitsev <konstantin@linuxfoundation.org> writes:

> Does it make sense to add logic for whether this is done in a bare repo?  
> I can't imagine common cases where a FETCH_HEAD would be useful outside 
> of a checkout where a merge is likely to happen.

It is entirely valid to respond to a one-shot "My work is published
there; could you see if I am doing anything obviously wrong?" with

    git fetch git://k.org/somebody/linux.git check-me-please
    git log ..FETCH_HEAD
    git diff ...FETCH_HEAD

i.e. without touching any of our refs, and possibly in a bare
repository, no?

Re: FETCH_HEAD files and mirrored repos

[Jeff King]

On Mon, Jul 13, 2020 at 01:04:54PM -0700, Junio C Hamano wrote:

> Konstantin Ryabitsev <konstantin@linuxfoundation.org> writes:
> 
> > Does it make sense to add logic for whether this is done in a bare repo?  
> > I can't imagine common cases where a FETCH_HEAD would be useful outside 
> > of a checkout where a merge is likely to happen.
> 
> It is entirely valid to respond to a one-shot "My work is published
> there; could you see if I am doing anything obviously wrong?" with
> 
>     git fetch git://k.org/somebody/linux.git check-me-please
>     git log ..FETCH_HEAD
>     git diff ...FETCH_HEAD
> 
> i.e. without touching any of our refs, and possibly in a bare
> repository, no?

I occasionally use FETCH_HEAD for such things. If we were to stop
writing it automatically, I think the key thing to notice is whether the
result was actually stored anywhere else. Or more accurately, whether
the user asked for any refspecs on the command line (since we'd still
update tracking refs in some cases).

If I do:

  git fetch

or:

  git fetch origin refs/heads/foo:refs/heads/foo

then I probably don't care about FETCH_HEAD. But if I do:

  git fetch origin refs/heads/foo

then I'm probably interested in picking the result out of FETCH_HEAD.

-Peff

Re: FETCH_HEAD files and mirrored repos

[Konstantin Ryabitsev]

On Mon, Jul 13, 2020 at 04:22:11PM -0400, Jeff King wrote:
> I occasionally use FETCH_HEAD for such things. If we were to stop
> writing it automatically, I think the key thing to notice is whether the
> result was actually stored anywhere else. Or more accurately, whether
> the user asked for any refspecs on the command line (since we'd still
> update tracking refs in some cases).
> 
> If I do:
> 
>   git fetch
> 
> or:
> 
>   git fetch origin refs/heads/foo:refs/heads/foo
> 
> then I probably don't care about FETCH_HEAD. But if I do:
> 
>   git fetch origin refs/heads/foo
> 
> then I'm probably interested in picking the result out of FETCH_HEAD.

I think adding all this logic is not worth the effort. For vast numbers 
of people who aren't running Android mirrors, FETCH_HEAD is only going 
to be a few KB in size, so they won't benefit from this change at all.

I'm happy with just an option that I have to enable to turn off writing 
FETCH_HEAD.

-K

Re: FETCH_HEAD files and mirrored repos

[Jeff King]

On Mon, Jul 13, 2020 at 04:34:27PM -0400, Konstantin Ryabitsev wrote:

> I think adding all this logic is not worth the effort. For vast numbers 
> of people who aren't running Android mirrors, FETCH_HEAD is only going 
> to be a few KB in size, so they won't benefit from this change at all.
> 
> I'm happy with just an option that I have to enable to turn off writing 
> FETCH_HEAD.

Yeah, I'm totally fine with that, too. And because it's an incremental
set of steps (add the option, then possibly flip the default), we can
easily stop at step 1 and see if anybody cares enough about step 2 to
continue.

-Peff

Re: FETCH_HEAD files and mirrored repos

[Johannes Sixt]

Am 13.07.20 um 22:22 schrieb Jeff King:
> I occasionally use FETCH_HEAD for such things. If we were to stop
> writing it automatically, I think the key thing to notice is whether the
> result was actually stored anywhere else. Or more accurately, whether
> the user asked for any refspecs on the command line (since we'd still
> update tracking refs in some cases).
> 
> If I do:
> 
>   git fetch
> 
> or:
> 
>   git fetch origin refs/heads/foo:refs/heads/foo
> 
> then I probably don't care about FETCH_HEAD. But if I do:
> 
>   git fetch origin refs/heads/foo
> 
> then I'm probably interested in picking the result out of FETCH_HEAD.

Very good observation. The latter is exactly the "many cases" I
mentioned where I do care about FETCH_HEAD.

-- Hannes

Re: FETCH_HEAD files and mirrored repos

[Jonathan Nieder]

Johannes Sixt wrote:
> Am 12.07.20 um 22:52 schrieb Junio C Hamano:

>> We could teach "git fetch" a new "--write-fetch-head" option and
>> teach "git pull" to pass it when calling "git fetch".  It is very
>> plausible that some folks rely on "git fetch" to leave FETCH_HEAD
>> without being told, so the option must default to on for a few
>> development cycles before we flip the default to off,
>
> Nah, really??? It's one of the benefits of git-fetch that it writes
> FETCH_HEAD and the primary reason in many cases where I use the command!
> So, either I don't care that FETCH_HEAD is written, or I do use it. IMO,
> not wanting to write FETCH_HEAD is the odd case and would need a
> configuration tweak, not the other way round.

Speaking for myself as a user, I also rely on (the first line of)
FETCH_HEAD being written so that I can run commands like "git log
FETCH_HEAD".  What I don't rely on is the massive amounts of data that
may get written after that first line.

Thanks,
Jonathan