git@vger.kernel.org list mirror (unofficial, one of many)
 help / color / mirror / Atom feed
From: Junio C Hamano <gitster@pobox.com>
To: Michael Schubert <mschub@elegosoft.com>
Cc: git@vger.kernel.org
Subject: Re: [RFC/PATCH] fetch: make --prune configurable
Date: Fri, 12 Jul 2013 15:38:46 -0700
Message-ID: <7vppunietl.fsf@alter.siamese.dyndns.org> (raw)
In-Reply-To: <7vk3l0zypa.fsf@alter.siamese.dyndns.org> (Junio C. Hamano's message of "Mon, 08 Jul 2013 11:36:01 -0700")

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

  reply	other threads:[~2013-07-12 22:39 UTC|newest]

Thread overview: 5+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2013-07-08 12:56 Michael Schubert
2013-07-08 18:36 ` Junio C Hamano
2013-07-12 22:38   ` Junio C Hamano [this message]
2013-07-08 19:01 ` John Keeping
2013-07-09  3:50 ` Jeff King

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

  List information: http://vger.kernel.org/majordomo-info.html

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=7vppunietl.fsf@alter.siamese.dyndns.org \
    --to=gitster@pobox.com \
    --cc=git@vger.kernel.org \
    --cc=mschub@elegosoft.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link

git@vger.kernel.org list mirror (unofficial, one of many)

This inbox may be cloned and mirrored by anyone:

	git clone --mirror https://public-inbox.org/git
	git clone --mirror http://ou63pmih66umazou.onion/git
	git clone --mirror http://czquwvybam4bgbro.onion/git
	git clone --mirror http://hjrcffqmbrq6wope.onion/git

	# If you have public-inbox 1.1+ installed, you may
	# initialize and index your mirror using the following commands:
	public-inbox-init -V1 git git/ https://public-inbox.org/git \
		git@vger.kernel.org
	public-inbox-index git

Example config snippet for mirrors.
Newsgroups are available over NNTP:
	nntp://news.public-inbox.org/inbox.comp.version-control.git
	nntp://ou63pmih66umazou.onion/inbox.comp.version-control.git
	nntp://czquwvybam4bgbro.onion/inbox.comp.version-control.git
	nntp://hjrcffqmbrq6wope.onion/inbox.comp.version-control.git
	nntp://news.gmane.io/gmane.comp.version-control.git
 note: .onion URLs require Tor: https://www.torproject.org/

code repositories for the project(s) associated with this inbox:

	https://80x24.org/mirrors/git.git

AGPL code for this site: git clone https://public-inbox.org/public-inbox.git