[RFC/PATCH] fetch: make --prune configurable

[RFC/PATCH] fetch: make --prune configurable

[Michael Schubert]

$gmane/201715 brought up the idea to fetch --prune by default.
Since --prune is a "potentially destructive operation" (Git doesn't
keep reflogs for deleted references yet), we don't want to prune
without users consent.

To accommodate users who want to either prune always or when fetching
from a particular remote, add two new configuration variables
"fetch.prune" and "remote.<name>.prune":

 - "fetch.prune" allows to enable prune for all fetch operations.

 - "remote.<name>.prune" allows to change the behaviour per remote.

Signed-off-by: Michael Schubert <mschub@elegosoft.com>
---

http://thread.gmane.org/gmane.comp.version-control.git/201715


 Documentation/config.txt |  9 +++++++++
 builtin/fetch.c          | 28 +++++++++++++++++++++++++---
 remote.c                 |  2 ++
 remote.h                 |  1 +
 t/t5510-fetch.sh         | 38 ++++++++++++++++++++++++++++++++++++++
 5 files changed, 75 insertions(+), 3 deletions(-)

diff --git a/Documentation/config.txt b/Documentation/config.txt
index b4d4887..74e8026 100644
--- a/Documentation/config.txt
+++ b/Documentation/config.txt
@@ -1067,6 +1067,10 @@ fetch.unpackLimit::
 	especially on slow filesystems.  If not set, the value of
 	`transfer.unpackLimit` is used instead.
 
+fetch.prune::
+	If true, fetch will automatically behave as if the `--prune`
+	option was given on the command line.
+
 format.attach::
 	Enable multipart/mixed attachments as the default for
 	'format-patch'.  The value can also be a double quoted string
@@ -2010,6 +2014,11 @@ remote.<name>.vcs::
 	Setting this to a value <vcs> will cause Git to interact with
 	the remote with the git-remote-<vcs> helper.
 
+remote.<name>.prune::
+	When set to true, fetching from this remote by default will also
+	remove any remote-tracking branches which no longer exist on the
+	remote (as if the `--prune` option was give on the command line).
+
 remotes.<group>::
 	The list of remotes which are fetched by "git remote update
 	<group>".  See linkgit:git-remote[1].
diff --git a/builtin/fetch.c b/builtin/fetch.c
index d784b2e..3953317 100644
--- a/builtin/fetch.c
+++ b/builtin/fetch.c
@@ -30,7 +30,14 @@ enum {
 	TAGS_SET = 2
 };
 
-static int all, append, dry_run, force, keep, multiple, prune, update_head_ok, verbosity;
+enum {
+	PRUNE_UNSET = 0,
+	PRUNE_DEFAULT = 1,
+	PRUNE_FORCE = 2
+};
+
+static int prune = PRUNE_DEFAULT;
+static int all, append, dry_run, force, keep, multiple, update_head_ok, verbosity;
 static int progress = -1, recurse_submodules = RECURSE_SUBMODULES_DEFAULT;
 static int tags = TAGS_DEFAULT, unshallow;
 static const char *depth;
@@ -54,6 +61,17 @@ static int option_parse_recurse_submodules(const struct option *opt,
 	return 0;
 }
 
+static int git_fetch_config(const char *k, const char *v, void *cb)
+{
+	if (!strcmp(k, "fetch.prune")) {
+		int boolval = git_config_bool(k, v);
+		if (boolval)
+			prune = PRUNE_FORCE;
+		return 0;
+	}
+	return git_default_config(k, v, cb);
+}
+
 static struct option builtin_fetch_options[] = {
 	OPT__VERBOSITY(&verbosity),
 	OPT_BOOLEAN(0, "all", &all,
@@ -770,7 +788,7 @@ static int do_fetch(struct transport *transport,
 		retcode = 1;
 		goto cleanup;
 	}
-	if (prune) {
+	if (prune == PRUNE_FORCE || (transport->remote->prune && prune)) {
 		/* If --tags was specified, pretend the user gave us the canonical tags refspec */
 		if (tags == TAGS_SET) {
 			const char *tags_str = "refs/tags/*:refs/tags/*";
@@ -882,8 +900,10 @@ static void add_options_to_argv(struct argv_array *argv)
 {
 	if (dry_run)
 		argv_array_push(argv, "--dry-run");
-	if (prune)
+	if (prune == PRUNE_FORCE)
 		argv_array_push(argv, "--prune");
+	else if (prune == PRUNE_UNSET)
+		argv_array_push(argv, "--no-prune");
 	if (update_head_ok)
 		argv_array_push(argv, "--update-head-ok");
 	if (force)
@@ -1007,6 +1027,8 @@ int cmd_fetch(int argc, const char **argv, const char *prefix)
 	for (i = 1; i < argc; i++)
 		strbuf_addf(&default_rla, " %s", argv[i]);
 
+	git_config(git_fetch_config, NULL);
+
 	argc = parse_options(argc, argv, prefix,
 			     builtin_fetch_options, builtin_fetch_usage, 0);
 
diff --git a/remote.c b/remote.c
index 6f57830..e6f2acb 100644
--- a/remote.c
+++ b/remote.c
@@ -404,6 +404,8 @@ static int handle_config(const char *key, const char *value, void *cb)
 		remote->skip_default_update = git_config_bool(key, value);
 	else if (!strcmp(subkey, ".skipfetchall"))
 		remote->skip_default_update = git_config_bool(key, value);
+	else if (!strcmp(subkey, ".prune"))
+		remote->prune = git_config_bool(key, value);
 	else if (!strcmp(subkey, ".url")) {
 		const char *v;
 		if (git_config_string(&v, key, value))
diff --git a/remote.h b/remote.h
index cf56724..4db3498 100644
--- a/remote.h
+++ b/remote.h
@@ -40,6 +40,7 @@ struct remote {
 	int fetch_tags;
 	int skip_default_update;
 	int mirror;
+	int prune;
 
 	const char *receivepack;
 	const char *uploadpack;
diff --git a/t/t5510-fetch.sh b/t/t5510-fetch.sh
index fde6891..f3d63ca 100755
--- a/t/t5510-fetch.sh
+++ b/t/t5510-fetch.sh
@@ -497,6 +497,44 @@ test_expect_success "should be able to fetch with duplicate refspecs" '
 	)
 '
 
+test_expect_success 'fetch should prune when fetch.prune is true' '
+  cd "$D" &&
+  git branch somebranch &&
+  (
+    cd one &&
+    git fetch &&
+    test -f .git/refs/remotes/origin/somebranch
+  ) &&
+  git branch -d somebranch &&
+  (
+    cd one &&
+    git config fetch.prune true &&
+    git fetch --no-prune &&
+    test -f .git/refs/remotes/origin/somebranch &&
+    git fetch &&
+    ! test -f .git/refs/remotes/origin/somebranch
+  )
+'
+
+test_expect_success 'fetch should prune when remote.<name>.prune is true' '
+  cd "$D" &&
+  git branch somebranch &&
+  (
+    cd one &&
+    git fetch &&
+    test -f .git/refs/remotes/origin/somebranch
+  ) &&
+  git branch -d somebranch &&
+  (
+    cd one &&
+    git config remote.origin.prune true &&
+    git fetch --no-prune &&
+    test -f .git/refs/remotes/origin/somebranch &&
+    git fetch &&
+    ! test -f .git/refs/remotes/origin/somebranch
+  )
+'
+
 test_expect_success 'all boundary commits are excluded' '
 	test_commit base &&
 	test_commit oneside &&
-- 
1.8.3.2.734.gbcd3b20

Re: [RFC/PATCH] fetch: make --prune configurable

[Junio C Hamano]

Michael Schubert <mschub@elegosoft.com> writes:

> $gmane/201715 brought up the idea to fetch --prune by default.

When you can summarize it in a few lines, e.g.

    Without "git fetch --prune", remote-tracking branches for a branch
    the other side already has removed will stay forever.  Some people
    want to always run "git fetch --prune".

please refrain from forcing people to go to the web while reading
logs.

> Since --prune is a "potentially destructive operation" (Git doesn't
> keep reflogs for deleted references yet), we don't want to prune
> without users consent.
>
> To accommodate users who want to either prune always or when fetching
> from a particular remote, add two new configuration variables
> "fetch.prune" and "remote.<name>.prune":
>
>  - "fetch.prune" allows to enable prune for all fetch operations.
>
>  - "remote.<name>.prune" allows to change the behaviour per remote.
>

Add:

    "git fetch --no-prune" from the command line will defeat the
    configured default for safety.

(I didn't check if your patch already does so, though).

Other than that, the log message looks good.

Thanks for starting to work on this.

> diff --git a/Documentation/config.txt b/Documentation/config.txt
> index b4d4887..74e8026 100644
> --- a/Documentation/config.txt
> +++ b/Documentation/config.txt
> @@ -1067,6 +1067,10 @@ fetch.unpackLimit::
>  	especially on slow filesystems.  If not set, the value of
>  	`transfer.unpackLimit` is used instead.
>  
> +fetch.prune::
> +	If true, fetch will automatically behave as if the `--prune`
> +	option was given on the command line.
> +
>  format.attach::
>  	Enable multipart/mixed attachments as the default for
>  	'format-patch'.  The value can also be a double quoted string
> @@ -2010,6 +2014,11 @@ remote.<name>.vcs::
>  	Setting this to a value <vcs> will cause Git to interact with
>  	the remote with the git-remote-<vcs> helper.
>  
> +remote.<name>.prune::
> +	When set to true, fetching from this remote by default will also
> +	remove any remote-tracking branches which no longer exist on the
> +	remote (as if the `--prune` option was give on the command line).

We may want to say something about interaction between the two
variables.  E.g. fetch.prune=true, remote.origin.prune=false would
hopefully not to prune when you are fetching from your 'origin', and
fetch.prune=false, remote.origin.prune=true would.

> diff --git a/builtin/fetch.c b/builtin/fetch.c
> index d784b2e..3953317 100644
> --- a/builtin/fetch.c
> +++ b/builtin/fetch.c
> @@ -30,7 +30,14 @@ enum {
>  	TAGS_SET = 2
>  };
>  
> -static int all, append, dry_run, force, keep, multiple, prune, update_head_ok, verbosity;
> +enum {
> +	PRUNE_UNSET = 0,
> +	PRUNE_DEFAULT = 1,
> +	PRUNE_FORCE = 2
> +};
> +
> +static int prune = PRUNE_DEFAULT;

I find this unconventional in that usually _UNSET means "the user
hasn't explicitly said anything about what she wants" (hence
typically a variable is initialized to that value).  Also I am not
sure what "FORCE" means.

If this were 

enum {
	PRUNE_UNSET = -1,
	PRUNE_NO = 0,
	PRUNE_YES = 1
};

then I would understand, but at that point, that is a typical
setting for a boolean variable, so we could just use -1/0/1.

> +static int all, append, dry_run, force, keep, multiple, update_head_ok, verbosity;
>  static int progress = -1, recurse_submodules = RECURSE_SUBMODULES_DEFAULT;
>  static int tags = TAGS_DEFAULT, unshallow;
>  static const char *depth;
> @@ -54,6 +61,17 @@ static int option_parse_recurse_submodules(const struct option *opt,
>  	return 0;
>  }
>  
> +static int git_fetch_config(const char *k, const char *v, void *cb)
> +{
> +	if (!strcmp(k, "fetch.prune")) {
> +		int boolval = git_config_bool(k, v);
> +		if (boolval)
> +			prune = PRUNE_FORCE;
> +		return 0;

This is not good, is it?  Imagine fetch.prune=true in ~/.gitconfig
and fetch.prune=false in $GIT_DIR/config; I'd expect the more
specific one to set "prune" back to non-FORCE value.

As you do not have transport available before you process
parse_options(), I think you need two variables, "prune" that is
used to determine what happens (same as in the code before your
patch) and a new one "fetch_prune_config" that records what we read
from the fetch.prune configuration.

So I _suspect_ the interaction between the configuration parser and
the command line option parser should look more like this, in order
to implement the correct order of precedence:

	static int fetch_prune_config = -1; /* unspecified */
        static int prune = -1; /* unspecified */
	#define PRUNE_BY_DEFAULT 0

	...

        /* set "fetch_prune_config" */
	git_config(git_fetch_config);
	--> git_fetch_config():
		if (!strcmp(k, "fetch.prune")) {
			fetch_prune_config = git_config_bool(k, v);
			return 0;
		}

	...

        /* set "prune" */
        parse_options();

        --> fetch_one();
		transport_get();

                if (prune < 0) {
			/* no command line request; combine configs */
			if (0 <= transport->remote->prune)
				prune = transport->remote->prune;
			else if (0 <= fetch_prune_config)
                        	prune = fetch_prune_config;
			else
                        	prune = PRUNE_BY_DEFAULT;
		}

You would need to update make_remote() to initialise its .prune
member with -1 (unspecified).

You also need to change OPT_BOOLEAN() to OPT_BOOL() for the command
line processing for "prune", as the former is a "count up" that is
useful for things like "-v", "-v -v", etc.

> +	}
> +	return git_default_config(k, v, cb);

What kind of random configuration are we expecting to read from and
affect our execution by falling back to the default config?

>  static struct option builtin_fetch_options[] = {
>  	OPT__VERBOSITY(&verbosity),
>  	OPT_BOOLEAN(0, "all", &all,
> @@ -770,7 +788,7 @@ static int do_fetch(struct transport *transport,
>  		retcode = 1;
>  		goto cleanup;
>  	}
> -	if (prune) {
> +	if (prune == PRUNE_FORCE || (transport->remote->prune && prune)) {

I cannot offhand see how this is correct with your code (without the
above "you need two variables" suggestion).  It reads to me:

	If fetch.prune is set, we set it to PRUNE_FORCE in the
        configuration parser, and in that case we ignore everything
        else and go ahead to prune.

which does not sound right.  fetch.prune=yes remote.$name.prune=no
should not prune, fetch.prune=yes with --no-prune from the command
line should not prune, etc.  But I may be misreading this complex
boolean expression in the patch.

And with the suggested change, this can stay 

	if (prune)

without having to do any mental gymnastics here.

> @@ -882,8 +900,10 @@ static void add_options_to_argv(struct argv_array *argv)
>  {
>  	if (dry_run)
>  		argv_array_push(argv, "--dry-run");
> -	if (prune)
> +	if (prune == PRUNE_FORCE)
>  		argv_array_push(argv, "--prune");
> +	else if (prune == PRUNE_UNSET)
> +		argv_array_push(argv, "--no-prune");
>  	if (update_head_ok)
>  		argv_array_push(argv, "--update-head-ok");
>  	if (force)

And I think this hunk can go with the suggested change.

> @@ -1007,6 +1027,8 @@ int cmd_fetch(int argc, const char **argv, const char *prefix)
>  	for (i = 1; i < argc; i++)
>  		strbuf_addf(&default_rla, " %s", argv[i]);
>  
> +	git_config(git_fetch_config, NULL);
> +
>  	argc = parse_options(argc, argv, prefix,
>  			     builtin_fetch_options, builtin_fetch_usage, 0);
>  
> diff --git a/remote.c b/remote.c
> index 6f57830..e6f2acb 100644
> --- a/remote.c
> +++ b/remote.c
> @@ -404,6 +404,8 @@ static int handle_config(const char *key, const char *value, void *cb)
>  		remote->skip_default_update = git_config_bool(key, value);
>  	else if (!strcmp(subkey, ".skipfetchall"))
>  		remote->skip_default_update = git_config_bool(key, value);
> +	else if (!strcmp(subkey, ".prune"))
> +		remote->prune = git_config_bool(key, value);
>  	else if (!strcmp(subkey, ".url")) {
>  		const char *v;
>  		if (git_config_string(&v, key, value))
> diff --git a/remote.h b/remote.h
> index cf56724..4db3498 100644
> --- a/remote.h
> +++ b/remote.h
> @@ -40,6 +40,7 @@ struct remote {
>  	int fetch_tags;
>  	int skip_default_update;
>  	int mirror;
> +	int prune;
>  
>  	const char *receivepack;
>  	const char *uploadpack;
> diff --git a/t/t5510-fetch.sh b/t/t5510-fetch.sh
> index fde6891..f3d63ca 100755
> --- a/t/t5510-fetch.sh
> +++ b/t/t5510-fetch.sh
> @@ -497,6 +497,44 @@ test_expect_success "should be able to fetch with duplicate refspecs" '
>  	)
>  '
>  
> +test_expect_success 'fetch should prune when fetch.prune is true' '
> +  cd "$D" &&
> +  git branch somebranch &&
> +  (
> +    cd one &&
> +    git fetch &&
> +    test -f .git/refs/remotes/origin/somebranch
> +  ) &&
> +  git branch -d somebranch &&
> +  (
> +    cd one &&
> +    git config fetch.prune true &&
> +    git fetch --no-prune &&
> +    test -f .git/refs/remotes/origin/somebranch &&
> +    git fetch &&
> +    ! test -f .git/refs/remotes/origin/somebranch
> +  )
> +'

OK, "fetch.prune < --no-prune" and  "fetch.prune alone" are tested
with this.  "fetch.prune=no" with "--prune" is not tested.

> +test_expect_success 'fetch should prune when remote.<name>.prune is true' '
> +  cd "$D" &&
> +  git branch somebranch &&
> +  (
> +    cd one &&
> +    git fetch &&
> +    test -f .git/refs/remotes/origin/somebranch

Depending on the success/failure of the previous test, we do not
know what value fetch.prune is set in this repository.  What are we
testing here?  By using "test_unconfig" to clear the variables you
care about before starting each test, you would make it clear what
exactly you are testing.

There are at least 9 combinations of settings we should be testing.
A naive matrix:

	fetch.prune		set to false or not set, set to true
        remote.origin.prune	set to false or not set, set to true
	command line		--no-prune, --prune, (neither)

will give us 12 combinations (for completeness, you may want to test
fetch.prune that is not set at all and fetch.prune explicitly set to
false separately, but let's not go overboard), but we presumably
have been testing cases where neither fetch.prune or remote.*.prune
is set, so we would need to only test cases where at least one of
the configuration variables is set (that would make 3 cases),
multiplied with the command line combinations.


> +  ) &&
> +  git branch -d somebranch &&
> +  (
> +    cd one &&
> +    git config remote.origin.prune true &&
> +    git fetch --no-prune &&
> +    test -f .git/refs/remotes/origin/somebranch &&
> +    git fetch &&
> +    ! test -f .git/refs/remotes/origin/somebranch
> +  )
> +'
> +
>  test_expect_success 'all boundary commits are excluded' '
>  	test_commit base &&
>  	test_commit oneside &&

Re: [RFC/PATCH] fetch: make --prune configurable

[Junio C Hamano]

Here is my previous review comments in a squashable patch form.  The
result seems to pass all 27 combinations (fetch.prune, remote.*.prune
and command line all are tristate yes/no/unspecified).

Without the fix-up in *.c files, three combinations seem to fail.

 Documentation/config.txt |   3 +-
 builtin/fetch.c          |  41 +++++++++-------
 remote.c                 |   1 +
 t/t5510-fetch.sh         | 118 ++++++++++++++++++++++++++++++++---------------
 4 files changed, 108 insertions(+), 55 deletions(-)

diff --git a/Documentation/config.txt b/Documentation/config.txt
index fc39f3a..e4ce7c4 100644
--- a/Documentation/config.txt
+++ b/Documentation/config.txt
@@ -1051,7 +1051,7 @@ fetch.unpackLimit::
 
 fetch.prune::
 	If true, fetch will automatically behave as if the `--prune`
-	option was given on the command line.
+	option was given on the command line.  See also `remote.<name>.prune`.
 
 format.attach::
 	Enable multipart/mixed attachments as the default for
@@ -1992,6 +1992,7 @@ remote.<name>.prune::
 	When set to true, fetching from this remote by default will also
 	remove any remote-tracking branches which no longer exist on the
 	remote (as if the `--prune` option was give on the command line).
+	Overrides `fetch.prune` settings, if any.
 
 remotes.<group>::
 	The list of remotes which are fetched by "git remote update
diff --git a/builtin/fetch.c b/builtin/fetch.c
index 082450b..08ab948 100644
--- a/builtin/fetch.c
+++ b/builtin/fetch.c
@@ -30,13 +30,10 @@ enum {
 	TAGS_SET = 2
 };
 
-enum {
-	PRUNE_UNSET = 0,
-	PRUNE_DEFAULT = 1,
-	PRUNE_FORCE = 2
-};
+static int fetch_prune_config = -1; /* unspecified */
+static int prune = -1; /* unspecified */
+#define PRUNE_BY_DEFAULT 0 /* do we prune by default? */
 
-static int prune = PRUNE_DEFAULT;
 static int all, append, dry_run, force, keep, multiple, update_head_ok, verbosity;
 static int progress = -1, recurse_submodules = RECURSE_SUBMODULES_DEFAULT;
 static int tags = TAGS_DEFAULT, unshallow;
@@ -64,12 +61,10 @@ static int option_parse_recurse_submodules(const struct option *opt,
 static int git_fetch_config(const char *k, const char *v, void *cb)
 {
 	if (!strcmp(k, "fetch.prune")) {
-		int boolval = git_config_bool(k, v);
-		if (boolval)
-			prune = PRUNE_FORCE;
+		fetch_prune_config = git_config_bool(k, v);
 		return 0;
 	}
-	return git_default_config(k, v, cb);
+	return 0;
 }
 
 static struct option builtin_fetch_options[] = {
@@ -87,8 +82,8 @@ static struct option builtin_fetch_options[] = {
 		    N_("fetch all tags and associated objects"), TAGS_SET),
 	OPT_SET_INT('n', NULL, &tags,
 		    N_("do not fetch all tags (--no-tags)"), TAGS_UNSET),
-	OPT_BOOLEAN('p', "prune", &prune,
-		    N_("prune remote-tracking branches no longer on remote")),
+	OPT_BOOL('p', "prune", &prune,
+		 N_("prune remote-tracking branches no longer on remote")),
 	{ OPTION_CALLBACK, 0, "recurse-submodules", NULL, N_("on-demand"),
 		    N_("control recursive fetching of submodules"),
 		    PARSE_OPT_OPTARG, option_parse_recurse_submodules },
@@ -756,8 +751,11 @@ static int do_fetch(struct transport *transport,
 		free_refs(ref_map);
 		return 1;
 	}
-	if (prune == PRUNE_FORCE || (transport->remote->prune && prune)) {
-		/* If --tags was specified, pretend the user gave us the canonical tags refspec */
+	if (prune) {
+		/*
+		 * If --tags was specified, pretend that the user gave us
+		 * the canonical tags refspec
+		 */
 		if (tags == TAGS_SET) {
 			const char *tags_str = "refs/tags/*:refs/tags/*";
 			struct refspec *tags_refspec, *refspec;
@@ -866,10 +864,8 @@ static void add_options_to_argv(struct argv_array *argv)
 {
 	if (dry_run)
 		argv_array_push(argv, "--dry-run");
-	if (prune == PRUNE_FORCE)
+	if (prune > 0)
 		argv_array_push(argv, "--prune");
-	else if (prune == PRUNE_UNSET)
-		argv_array_push(argv, "--no-prune");
 	if (update_head_ok)
 		argv_array_push(argv, "--update-head-ok");
 	if (force)
@@ -936,6 +932,17 @@ static int fetch_one(struct remote *remote, int argc, const char **argv)
 		    "remote name from which new revisions should be fetched."));
 
 	transport = transport_get(remote, NULL);
+
+	if (prune < 0) {
+		/* no command line request */
+		if (0 <= transport->remote->prune)
+			prune = transport->remote->prune;
+		else if (0 <= fetch_prune_config)
+			prune = fetch_prune_config;
+		else
+			prune = PRUNE_BY_DEFAULT;
+	}
+
 	transport_set_verbosity(transport, verbosity, progress);
 	if (upload_pack)
 		set_option(TRANS_OPT_UPLOADPACK, upload_pack);
diff --git a/remote.c b/remote.c
index d0ddbef..89be211 100644
--- a/remote.c
+++ b/remote.c
@@ -148,6 +148,7 @@ static struct remote *make_remote(const char *name, int len)
 	}
 
 	ret = xcalloc(1, sizeof(struct remote));
+	ret->prune = -1;  /* unspecified */
 	ALLOC_GROW(remotes, remotes_nr + 1, remotes_alloc);
 	remotes[remotes_nr++] = ret;
 	if (len)
diff --git a/t/t5510-fetch.sh b/t/t5510-fetch.sh
index 33fe3d5..019535f 100755
--- a/t/t5510-fetch.sh
+++ b/t/t5510-fetch.sh
@@ -471,43 +471,87 @@ test_expect_success "should be able to fetch with duplicate refspecs" '
 	)
 '
 
-test_expect_success 'fetch should prune when fetch.prune is true' '
-  cd "$D" &&
-  git branch somebranch &&
-  (
-    cd one &&
-    git fetch &&
-    test -f .git/refs/remotes/origin/somebranch
-  ) &&
-  git branch -d somebranch &&
-  (
-    cd one &&
-    git config fetch.prune true &&
-    git fetch --no-prune &&
-    test -f .git/refs/remotes/origin/somebranch &&
-    git fetch &&
-    ! test -f .git/refs/remotes/origin/somebranch
-  )
-'
-
-test_expect_success 'fetch should prune when remote.<name>.prune is true' '
-  cd "$D" &&
-  git branch somebranch &&
-  (
-    cd one &&
-    git fetch &&
-    test -f .git/refs/remotes/origin/somebranch
-  ) &&
-  git branch -d somebranch &&
-  (
-    cd one &&
-    git config remote.origin.prune true &&
-    git fetch --no-prune &&
-    test -f .git/refs/remotes/origin/somebranch &&
-    git fetch &&
-    ! test -f .git/refs/remotes/origin/somebranch
-  )
-'
+# configured prune tests
+
+set_config_tristate () {
+	# var=$1 val=$2
+	case "$2" in
+	unset)  test_unconfig "$1" ;;
+	*)	git config "$1" "$2" ;;
+	esac
+}
+
+test_configured_prune () {
+	fetch_prune=$1 remote_origin_prune=$2 cmdline=$3 expected=$4
+
+	test_expect_success "prune fetch.prune=$1 remote.origin.prune=$2${3:+ $3}; $4" '
+		# make sure a newbranch is there in . and also in one
+		git branch -f newbranch &&
+		(
+			cd one &&
+			test_unconfig fetch.prune &&
+			test_unconfig remote.origin.prune &&
+			git fetch &&
+			git rev-parse --verify refs/remotes/origin/newbranch
+		)
+
+		# now remove it
+		git branch -d newbranch &&
+
+		# then test
+		(
+			cd one &&
+			set_config_tristate fetch.prune $fetch_prune &&
+			set_config_tristate remote.origin.prune $remote_origin_prune &&
+
+			git fetch $cmdline &&
+			case "$expected" in
+			pruned)
+				test_must_fail git rev-parse --verify refs/remotes/origin/newbranch
+				;;
+			kept)
+				git rev-parse --verify refs/remotes/origin/newbranch
+				;;
+			esac
+		)
+	'
+}
+
+test_configured_prune unset unset ""		kept
+test_configured_prune unset unset "--no-prune"	kept
+test_configured_prune unset unset "--prune"	pruned
+
+test_configured_prune false unset ""		kept
+test_configured_prune false unset "--no-prune"	kept
+test_configured_prune false unset "--prune"	pruned
+
+test_configured_prune true  unset ""		pruned
+test_configured_prune true  unset "--prune"	pruned
+test_configured_prune true  unset "--no-prune"	kept
+
+test_configured_prune unset false ""		kept
+test_configured_prune unset false "--no-prune"	kept
+test_configured_prune unset false "--prune"	pruned
+
+test_configured_prune false false ""		kept
+test_configured_prune false false "--no-prune"	kept
+test_configured_prune false false "--prune"	pruned
+
+test_configured_prune true  false ""		kept
+test_configured_prune true  false "--prune"	pruned
+test_configured_prune true  false "--no-prune"	kept
+
+test_configured_prune unset true  ""		pruned
+test_configured_prune unset true  "--no-prune"	kept
+test_configured_prune unset true  "--prune"	pruned
+
+test_configured_prune false true  ""		pruned
+test_configured_prune false true  "--no-prune"	kept
+test_configured_prune false true  "--prune"	pruned
+
+test_configured_prune true  true  ""		pruned
+test_configured_prune true  true  "--prune"	pruned
+test_configured_prune true  true  "--no-prune"	kept
 
 test_expect_success 'all boundary commits are excluded' '
 	test_commit base &&
-- 
1.8.3.2-941-gda9c3c8

Re: [RFC/PATCH] fetch: make --prune configurable

[John Keeping]

On Mon, Jul 08, 2013 at 02:56:57PM +0200, Michael Schubert wrote:
> $gmane/201715 brought up the idea to fetch --prune by default.
> Since --prune is a "potentially destructive operation" (Git doesn't
> keep reflogs for deleted references yet), we don't want to prune
> without users consent.
> 
> To accommodate users who want to either prune always or when fetching
> from a particular remote, add two new configuration variables
> "fetch.prune" and "remote.<name>.prune":
> 
>  - "fetch.prune" allows to enable prune for all fetch operations.
> 
>  - "remote.<name>.prune" allows to change the behaviour per remote.

Should this be "remote.<name>.pruneFetch"?  I'd quite like to be able to
configure --prune for git-push as well (I just haven't got around to
actually doing anything about it yet...) and it might be better to be
explicit in the remote.<name> section from the start.

I'm not sure it's necessary since we already have "remote" and
"pushremote" so we could have "prune" and "pushprune" but perhaps it's
worth considering.

Re: [RFC/PATCH] fetch: make --prune configurable

[Jeff King]

On Mon, Jul 08, 2013 at 02:56:57PM +0200, Michael Schubert wrote:

> $gmane/201715 brought up the idea to fetch --prune by default.
> Since --prune is a "potentially destructive operation" (Git doesn't
> keep reflogs for deleted references yet), we don't want to prune
> without users consent.
> 
> To accommodate users who want to either prune always or when fetching
> from a particular remote, add two new configuration variables
> "fetch.prune" and "remote.<name>.prune":
> 
>  - "fetch.prune" allows to enable prune for all fetch operations.
> 
>  - "remote.<name>.prune" allows to change the behaviour per remote.

Thanks. As the person who brought up the destructive nature of --prune
in the thread you mentioned, I have no problem at all with doing
something like this, where the user gets to make the choice. And it is
even a good building block if we later do have deleted-branch reflogs;
we can just flip the default from "off" to "on".

In the meantime, I don't know if it is worth mentioning in the
documentation that the remote branches are hard to get back. On the one
hand, it is the (or at least a) reason why the default is not "on". But
it is also far from the only place refs get deleted, so I don't know if
it is worth calling attention to it specifically.

-Peff