remote: move the tagopt setting to the fetch options

This is another option which we should not be keeping in the remote, but
is specific to each particular operation.

include/git2/remote.h

@@ -526,6 +526,31 @@ typedef enum {
 	GIT_FETCH_NO_PRUNE,
 } git_fetch_prune_t;
 
+/**
+ * Automatic tag following option
+ *
+ * Lets us select the --tags option to use.
+ */
+typedef enum {
+	/**
+	 * Use the setting from the configuration.
+	 */
+	GIT_REMOTE_DOWNLOAD_TAGS_FALLBACK = 0,
+	/**
+	 * Ask the server for tags pointing to objects we're already
+	 * downloading.
+	 */
+	GIT_REMOTE_DOWNLOAD_TAGS_AUTO,
+	/**
+	 * Don't ask for any tags beyond the refspecs.
+	 */
+	GIT_REMOTE_DOWNLOAD_TAGS_NONE,
+	/**
+	 * Ask for the all the tags.
+	 */
+	GIT_REMOTE_DOWNLOAD_TAGS_ALL,
+} git_remote_autotag_option_t;
+
 typedef struct {
 	int version;
 
@@ -544,6 +569,15 @@ typedef struct {
 	 * on. Leave this default in order to behave like git.
 	 */
 	int update_fetchhead;
+
+	/**
+	 * Determines how to behave regarding tags on the remote, such
+	 * as auto-downloading tags for objects we're downloading or
+	 * downloading all of them.
+	 *
+	 * The default is to auto-follow tags.
+	 */
+	git_remote_autotag_option_t download_tags;
 } git_fetch_options;
 
 #define GIT_FETCH_OPTIONS_VERSION 1
@@ -643,12 +677,15 @@ GIT_EXTERN(int) git_remote_upload(git_remote *remote, const git_strarray *refspe
  * parameter is ignored when pushing.
  * @param callbacks  pointer to the callback structure to use
  * @param update_fetchhead whether to write to FETCH_HEAD. Pass 1 to behave like git.
+ * @param download_tags what the behaviour for downloading tags is for this fetch. This is
+ * ignored for push. This must be the same value passed to `git_remote_download()`.
  * @return 0 or an error code
  */
 GIT_EXTERN(int) git_remote_update_tips(
 		git_remote *remote,
 		const git_remote_callbacks *callbacks,
 		int update_fetchhead,
+		git_remote_autotag_option_t download_tags,
 		const char *reflog_message);
 
 /**
@@ -700,17 +737,6 @@ GIT_EXTERN(int) git_remote_push(git_remote *remote,
 GIT_EXTERN(const git_transfer_progress *) git_remote_stats(git_remote *remote);
 
 /**
- * Automatic tag following option
- *
- * Lets us select the --tags option to use.
- */
-typedef enum {
-	GIT_REMOTE_DOWNLOAD_TAGS_AUTO = 0,
-	GIT_REMOTE_DOWNLOAD_TAGS_NONE = 1,
-	GIT_REMOTE_DOWNLOAD_TAGS_ALL = 2
-} git_remote_autotag_option_t;
-
-/**
  * Retrieve the tag auto-follow setting
  *
  * @param remote the remote to query
@@ -719,15 +745,16 @@ typedef enum {
 GIT_EXTERN(git_remote_autotag_option_t) git_remote_autotag(const git_remote *remote);
 
 /**
- * Set the tag auto-follow setting
+ * Set the remote's tag following setting.
+ *
+ * The change will be made in the configuration. No loaded remotes
+ * will be affected.
  *
- * @param remote the remote to configure
- * @param value a GIT_REMOTE_DOWNLOAD_TAGS value
+ * @param repo the repository in which to make the change
+ * @param remote the name of the remote
+ * @param value the new value to take.
  */
-GIT_EXTERN(void) git_remote_set_autotag(
-	git_remote *remote,
-	git_remote_autotag_option_t value);
-
+GIT_EXTERN(int) git_remote_set_autotag(git_repository *repo, const char *remote, git_remote_autotag_option_t value);
 /**
  * Retrieve the ref-prune setting
  *

src/fetch.c

@@ -19,14 +19,14 @@
 #include "repository.h"
 #include "refs.h"
 
-static int maybe_want(git_remote *remote, git_remote_head *head, git_odb *odb, git_refspec *tagspec)
+static int maybe_want(git_remote *remote, git_remote_head *head, git_odb *odb, git_refspec *tagspec, git_remote_autotag_option_t tagopt)
 {
 	int match = 0;
 
 	if (!git_reference_is_valid_name(head->name))
 		return 0;
 
-	if (remote->download_tags == GIT_REMOTE_DOWNLOAD_TAGS_ALL) {
+	if (tagopt == GIT_REMOTE_DOWNLOAD_TAGS_ALL) {
 		/*
 		 * If tagopt is --tags, always request tags
 		 * in addition to the remote's refspecs
@@ -51,13 +51,17 @@ static int maybe_want(git_remote *remote, git_remote_head *head, git_odb *odb, g
 	return git_vector_insert(&remote->refs, head);
 }
 
-static int filter_wants(git_remote *remote)
+static int filter_wants(git_remote *remote, const git_fetch_options *opts)
 {
 	git_remote_head **heads;
 	git_refspec tagspec, head;
 	int error = 0;
 	git_odb *odb;
 	size_t i, heads_len;
+	git_remote_autotag_option_t tagopt = remote->download_tags;
+
+	if (opts && opts->download_tags != GIT_REMOTE_DOWNLOAD_TAGS_FALLBACK)
+		tagopt = opts->download_tags;
 
 	git_vector_clear(&remote->refs);
 	if ((error = git_refspec__parse(&tagspec, GIT_REFSPEC_TAGS, true)) < 0)
@@ -87,7 +91,7 @@ static int filter_wants(git_remote *remote)
 		goto cleanup;
 
 	for (i = 0; i < heads_len; i++) {
-		if ((error = maybe_want(remote, heads[i], odb, &tagspec)) < 0)
+		if ((error = maybe_want(remote, heads[i], odb, &tagspec, tagopt)) < 0)
 			break;
 	}
 
@@ -102,13 +106,13 @@ cleanup:
  * them out. When we get an ACK we hide that commit and continue
  * traversing until we're done
  */
-int git_fetch_negotiate(git_remote *remote)
+int git_fetch_negotiate(git_remote *remote, const git_fetch_options *opts)
 {
 	git_transport *t = remote->transport;
 
-    remote->need_pack = 0;
+	remote->need_pack = 0;
 
-	if (filter_wants(remote) < 0) {
+	if (filter_wants(remote, opts) < 0) {
 		giterr_set(GITERR_NET, "Failed to filter the reference list for wants");
 		return -1;
 	}

src/fetch.h

@@ -9,7 +9,7 @@
 
 #include "netops.h"
 
-int git_fetch_negotiate(git_remote *remote);
+int git_fetch_negotiate(git_remote *remote, const git_fetch_options *opts);
 
 int git_fetch_download_pack(git_remote *remote, const git_remote_callbacks *callbacks);
 

src/remote.c

@@ -23,6 +23,7 @@
 #define CONFIG_URL_FMT "remote.%s.url"
 #define CONFIG_PUSHURL_FMT "remote.%s.pushurl"
 #define CONFIG_FETCH_FMT "remote.%s.fetch"
+#define CONFIG_TAGOPT_FMT "remote.%s.tagopt"
 
 static int dwim_refspecs(git_vector *out, git_vector *refspecs, git_vector *refs);
 static int lookup_remote_prune_config(git_remote *remote, git_config *config, const char *name);
@@ -60,7 +61,6 @@ static int download_tags_value(git_remote *remote, git_config *cfg)
 	git_buf buf = GIT_BUF_INIT;
 	int error;
 
-	/* The 0 value is the default (auto), let's see if we need to change it */
 	if (git_buf_printf(&buf, "remote.%s.tagopt", remote->name) < 0)
 		return -1;
 
@@ -192,9 +192,12 @@ static int create_internal(git_remote **out, git_repository *repo, const char *n
 			goto on_error;
 	}
 
+	/* A remote without a name doesn't download tags */
 	if (!name)
-		/* A remote without a name doesn't download tags */
 		remote->download_tags = GIT_REMOTE_DOWNLOAD_TAGS_NONE;
+	else
+		remote->download_tags = GIT_REMOTE_DOWNLOAD_TAGS_AUTO;
+
 
 	git_buf_free(&var);
 
@@ -419,6 +422,7 @@ int git_remote_lookup(git_remote **out, git_repository *repo, const char *name)
 	optional_setting_found |= found;
 
 	remote->repo = repo;
+	remote->download_tags = GIT_REMOTE_DOWNLOAD_TAGS_AUTO;
 
 	if (found && strlen(val) > 0) {
 		remote->url = git__strdup(val);
@@ -558,7 +562,6 @@ int git_remote_save(const git_remote *remote)
 {
 	int error;
 	git_config *cfg;
-	const char *tagopt = NULL;
 	git_buf buf = GIT_BUF_INIT;
 	git_config_entry *existing = NULL;
 
@@ -583,37 +586,6 @@ int git_remote_save(const git_remote *remote)
 	if ((error = update_config_refspec(remote, cfg, GIT_DIRECTION_PUSH)) < 0)
 		goto cleanup;
 
-	/*
-	 * What action to take depends on the old and new values. This
-	 * is describes by the table below. tagopt means whether the
-	 * is already a value set in the config
-	 *
-	 *            AUTO     ALL or NONE
-	 *         +-----------------------+
-	 *  tagopt | remove  |     set     |
-	 *         +---------+-------------|
-	 * !tagopt | nothing |     set     |
-	 *         +---------+-------------+
-	 */
-
-	git_buf_clear(&buf);
-	if ((error = git_buf_printf(&buf, "remote.%s.tagopt", remote->name)) < 0)
-		goto cleanup;
-
-	if ((error = git_config__lookup_entry(
-			&existing, cfg, git_buf_cstr(&buf), false)) < 0)
-		goto cleanup;
-
-	if (remote->download_tags == GIT_REMOTE_DOWNLOAD_TAGS_ALL)
-		tagopt = "--tags";
-	else if (remote->download_tags == GIT_REMOTE_DOWNLOAD_TAGS_NONE)
-		tagopt = "--no-tags";
-	else if (existing != NULL)
-		tagopt = NULL;
-
-	error = git_config__update_entry(
-		cfg, git_buf_cstr(&buf), tagopt, true, false);
-
 cleanup:
 	git_config_entry_free(existing);
 	git_buf_free(&buf);
@@ -951,7 +923,7 @@ int git_remote_download(git_remote *remote, const git_strarray *refspecs, const
 		remote->push = NULL;
 	}
 
-	if ((error = git_fetch_negotiate(remote)) < 0)
+	if ((error = git_fetch_negotiate(remote, opts)) < 0)
 		return error;
 
 	return git_fetch_download_pack(remote, cbs);
@@ -970,6 +942,7 @@ int git_remote_fetch(
 		const char *reflog_message)
 {
 	int error, update_fetchhead = 1;
+	git_remote_autotag_option_t tagopt = remote->download_tags;
 	bool prune = false;
 	git_buf reflog_msg_buf = GIT_BUF_INIT;
 	const git_remote_callbacks *cbs = NULL;
@@ -978,6 +951,7 @@ int git_remote_fetch(
 		GITERR_CHECK_VERSION(&opts->callbacks, GIT_REMOTE_CALLBACKS_VERSION, "git_remote_callbacks");
 		cbs = &opts->callbacks;
 		update_fetchhead = opts->update_fetchhead;
+		tagopt = opts->download_tags;
 	}
 
 	/* Connect and download everything */
@@ -1002,7 +976,7 @@ int git_remote_fetch(
 	}
 
 	/* Create "remote/foo" branches for all remote branches */
-	error = git_remote_update_tips(remote, cbs, update_fetchhead, git_buf_cstr(&reflog_msg_buf));
+	error = git_remote_update_tips(remote, cbs, update_fetchhead, tagopt, git_buf_cstr(&reflog_msg_buf));
 	git_buf_free(&reflog_msg_buf);
 	if (error < 0)
 		return error;
@@ -1319,6 +1293,7 @@ static int update_tips_for_spec(
 		git_remote *remote,
 		const git_remote_callbacks *callbacks,
 		int update_fetchhead,
+		git_remote_autotag_option_t tagopt,
 		git_refspec *spec,
 		git_vector *refs,
 		const char *log_message)
@@ -1354,9 +1329,9 @@ static int update_tips_for_spec(
 			continue;
 
 		if (git_refspec_src_matches(&tagspec, head->name)) {
-			if (remote->download_tags != GIT_REMOTE_DOWNLOAD_TAGS_NONE) {
+			if (tagopt != GIT_REMOTE_DOWNLOAD_TAGS_NONE) {
 
-				if (remote->download_tags == GIT_REMOTE_DOWNLOAD_TAGS_AUTO)
+				if (tagopt == GIT_REMOTE_DOWNLOAD_TAGS_AUTO)
 					autotag = 1;
 
 				git_buf_clear(&refname);
@@ -1519,10 +1494,12 @@ int git_remote_update_tips(
 		git_remote *remote,
 		const git_remote_callbacks *callbacks,
 		int update_fetchhead,
+		git_remote_autotag_option_t download_tags,
 		const char *reflog_message)
 {
 	git_refspec *spec, tagspec;
 	git_vector refs = GIT_VECTOR_INIT;
+	git_remote_autotag_option_t tagopt;
 	int error;
 	size_t i;
 
@@ -1538,8 +1515,13 @@ int git_remote_update_tips(
 	if ((error = ls_to_vector(&refs, remote)) < 0)
 		goto out;
 
-	if (remote->download_tags == GIT_REMOTE_DOWNLOAD_TAGS_ALL) {
-		if ((error = update_tips_for_spec(remote, callbacks, update_fetchhead, &tagspec, &refs, reflog_message)) < 0)
+	if (download_tags == GIT_REMOTE_DOWNLOAD_TAGS_FALLBACK)
+		tagopt = remote->download_tags;
+	else
+		tagopt = download_tags;
+
+	if (tagopt == GIT_REMOTE_DOWNLOAD_TAGS_ALL) {
+		if ((error = update_tips_for_spec(remote, callbacks, update_fetchhead, tagopt, &tagspec, &refs, reflog_message)) < 0)
 			goto out;
 	}
 
@@ -1547,7 +1529,7 @@ int git_remote_update_tips(
 		if (spec->push)
 			continue;
 
-		if ((error = update_tips_for_spec(remote, callbacks, update_fetchhead, spec, &refs, reflog_message)) < 0)
+		if ((error = update_tips_for_spec(remote, callbacks, update_fetchhead, tagopt, spec, &refs, reflog_message)) < 0)
 			goto out;
 	}
 
@@ -1675,9 +1657,42 @@ git_remote_autotag_option_t git_remote_autotag(const git_remote *remote)
 	return remote->download_tags;
 }
 
-void git_remote_set_autotag(git_remote *remote, git_remote_autotag_option_t value)
+int git_remote_set_autotag(git_repository *repo, const char *remote, git_remote_autotag_option_t value)
 {
-	remote->download_tags = value;
+	git_buf var = GIT_BUF_INIT;
+	git_config *config;
+	int error;
+
+	assert(repo && remote);
+
+	if ((error = ensure_remote_name_is_valid(remote)) < 0)
+		return error;
+
+	if ((error = git_repository_config__weakptr(&config, repo)) < 0)
+		return error;
+
+	if ((error = git_buf_printf(&var, CONFIG_TAGOPT_FMT, remote)))
+		return error;
+
+	switch (value) {
+	case GIT_REMOTE_DOWNLOAD_TAGS_NONE:
+		error = git_config_set_string(config, var.ptr, "--no-tags");
+		break;
+	case GIT_REMOTE_DOWNLOAD_TAGS_ALL:
+		error = git_config_set_string(config, var.ptr, "--tags");
+		break;
+	case GIT_REMOTE_DOWNLOAD_TAGS_AUTO:
+		error = git_config_delete_entry(config, var.ptr);
+		if (error == GIT_ENOTFOUND)
+			error = 0;
+		break;
+	default:
+		giterr_set(GITERR_INVALID, "Invalid value for the tagopt setting");
+		error = -1;
+	}
+
+	git_buf_free(&var);
+	return error;
 }
 
 int git_remote_prune_refs(const git_remote *remote)
@@ -2404,7 +2419,7 @@ int git_remote_push(git_remote *remote, const git_strarray *refspecs, const git_
 	if ((error = git_remote_upload(remote, refspecs, opts)) < 0)
 		return error;
 
-	error = git_remote_update_tips(remote, cbs, 0, NULL);
+	error = git_remote_update_tips(remote, cbs, 0, 0, NULL);
 
 	git_remote_disconnect(remote);
 	return error;

tests/network/remote/local.c

@@ -165,26 +165,26 @@ void test_network_remote_local__shorthand_fetch_refspec1(void)
 
 	cl_git_pass(git_remote_fetch(remote, &array, NULL, NULL));
 
-	cl_git_fail(git_reference_lookup(&ref, repo, "refs/remotes/master"));
-
+	cl_git_fail(git_reference_lookup(&ref, repo, "refs/remotes/origin/master"));
 	cl_git_fail(git_reference_lookup(&ref, repo, "refs/tags/hard_tag"));
 }
 
 void test_network_remote_local__tagopt(void)
 {
 	git_reference *ref;
+	git_fetch_options fetch_opts = GIT_FETCH_OPTIONS_INIT;
 
 	cl_git_pass(git_remote_create(&remote, repo, "tagopt", cl_git_path_url(cl_fixture("testrepo.git"))));
-	git_remote_set_autotag(remote, GIT_REMOTE_DOWNLOAD_TAGS_ALL);
-	cl_git_pass(git_remote_fetch(remote, NULL, NULL, NULL));
+	fetch_opts.download_tags = GIT_REMOTE_DOWNLOAD_TAGS_ALL;
+	cl_git_pass(git_remote_fetch(remote, NULL, &fetch_opts, NULL));
 
 	cl_git_pass(git_reference_lookup(&ref, repo, "refs/remotes/tagopt/master"));
 	git_reference_free(ref);
 	cl_git_pass(git_reference_lookup(&ref, repo, "refs/tags/hard_tag"));
 	git_reference_free(ref);
 
-	git_remote_set_autotag(remote, GIT_REMOTE_DOWNLOAD_TAGS_AUTO);
-	cl_git_pass(git_remote_fetch(remote, NULL, NULL, NULL));
+	fetch_opts.download_tags = GIT_REMOTE_DOWNLOAD_TAGS_AUTO;
+	cl_git_pass(git_remote_fetch(remote, NULL, &fetch_opts, NULL));
 	cl_git_pass(git_reference_lookup(&ref, repo, "refs/remotes/tagopt/master"));
 	git_reference_free(ref);
 }
@@ -240,9 +240,7 @@ void test_network_remote_local__push_to_bare_remote_with_file_url(void)
 
 	/* Get some commits */
 	connect_to_local_repository(cl_fixture("testrepo.git"));
-	cl_git_pass(git_remote_download(remote, &array, NULL));
-	cl_git_pass(git_remote_update_tips(remote, NULL, 1, NULL));
-	git_remote_disconnect(remote);
+	cl_git_pass(git_remote_fetch(remote, &array, NULL, NULL));
 
 	/* Set up an empty bare repo to push into */
 	{
@@ -278,12 +276,11 @@ void test_network_remote_local__push_to_non_bare_remote(void)
 	};
 	/* Shouldn't be able to push to a non-bare remote */
 	git_remote *localremote;
+	git_fetch_options fetch_opts = GIT_FETCH_OPTIONS_INIT;
 
 	/* Get some commits */
 	connect_to_local_repository(cl_fixture("testrepo.git"));
-	cl_git_pass(git_remote_download(remote, &array, NULL));
-	cl_git_pass(git_remote_update_tips(remote, NULL, 1, NULL));
-	git_remote_disconnect(remote);
+	cl_git_pass(git_remote_fetch(remote, &array, &fetch_opts, NULL));
 
 	/* Set up an empty non-bare repo to push into */
 	{
@@ -349,8 +346,7 @@ void test_network_remote_local__reflog(void)
 
 	connect_to_local_repository(cl_fixture("testrepo.git"));
 
-	cl_git_pass(git_remote_download(remote, &array, NULL));
-	cl_git_pass(git_remote_update_tips(remote, NULL, 1, "UPDAAAAAATE!!"));
+	cl_git_pass(git_remote_fetch(remote, &array, NULL, "UPDAAAAAATE!!"));
 
 	cl_git_pass(git_reflog_read(&log, repo, "refs/remotes/sloppy/master"));
 	cl_assert_equal_i(1, git_reflog_entrycount(log));

tests/network/remote/remotes.c

@@ -395,16 +395,15 @@ void test_network_remote_remotes__cannot_add_a_remote_with_an_invalid_name(void)
 
 void test_network_remote_remotes__tagopt(void)
 {
-	git_remote_set_autotag(_remote, GIT_REMOTE_DOWNLOAD_TAGS_ALL);
-	cl_git_pass(git_remote_save(_remote));
+	const char *name = git_remote_name(_remote);
+
+	git_remote_set_autotag(_repo, name, GIT_REMOTE_DOWNLOAD_TAGS_ALL);
 	assert_config_entry_value(_repo, "remote.test.tagopt", "--tags");
 
-	git_remote_set_autotag(_remote, GIT_REMOTE_DOWNLOAD_TAGS_NONE);
-	cl_git_pass(git_remote_save(_remote));
+	git_remote_set_autotag(_repo, name, GIT_REMOTE_DOWNLOAD_TAGS_NONE);
 	assert_config_entry_value(_repo, "remote.test.tagopt", "--no-tags");
 
-	git_remote_set_autotag(_remote, GIT_REMOTE_DOWNLOAD_TAGS_AUTO);
-	cl_git_pass(git_remote_save(_remote));
+	git_remote_set_autotag(_repo, name, GIT_REMOTE_DOWNLOAD_TAGS_AUTO);
 	assert_config_entry_existence(_repo, "remote.test.tagopt", false);
 }
 

tests/online/fetch.c

@@ -41,10 +41,10 @@ static void do_fetch(const char *url, git_remote_autotag_option_t flag, int n)
 	options.callbacks.transfer_progress = progress;
 	options.callbacks.update_tips = update_tips;
 	options.callbacks.payload = &bytes_received;
+	options.download_tags = flag;
 	counter = 0;
 
 	cl_git_pass(git_remote_create(&remote, _repo, "test", url));
-	git_remote_set_autotag(remote, flag);
 	cl_git_pass(git_remote_fetch(remote, NULL, &options, NULL));
 	cl_assert_equal_i(counter, n);
 	cl_assert(bytes_received > 0);
@@ -127,7 +127,7 @@ void test_online_fetch__doesnt_retrieve_a_pack_when_the_repository_is_up_to_date
 
 	cl_assert_equal_i(false, invoked);
 
-	cl_git_pass(git_remote_update_tips(remote, &options.callbacks, 1, NULL));
+	cl_git_pass(git_remote_update_tips(remote, &options.callbacks, 1, options.download_tags, NULL));
 	git_remote_disconnect(remote);
 
 	git_remote_free(remote);

tests/online/fetchhead.c

@@ -38,12 +38,13 @@ static void fetchhead_test_clone(void)
 static void fetchhead_test_fetch(const char *fetchspec, const char *expected_fetchhead)
 {
 	git_remote *remote;
+	git_fetch_options fetch_opts = GIT_FETCH_OPTIONS_INIT;
 	git_buf fetchhead_buf = GIT_BUF_INIT;
 	int equals = 0;
 	git_strarray array, *active_refs = NULL;
 
 	cl_git_pass(git_remote_lookup(&remote, g_repo, "origin"));
-	git_remote_set_autotag(remote, GIT_REMOTE_DOWNLOAD_TAGS_AUTO);
+	fetch_opts.download_tags = GIT_REMOTE_DOWNLOAD_TAGS_AUTO;
 
 	if(fetchspec != NULL) {
 		array.count = 1;
@@ -51,7 +52,7 @@ static void fetchhead_test_fetch(const char *fetchspec, const char *expected_fet
 		active_refs = &array;
 	}
 
-	cl_git_pass(git_remote_fetch(remote, active_refs, NULL, NULL));
+	cl_git_pass(git_remote_fetch(remote, active_refs, &fetch_opts, NULL));
 	git_remote_free(remote);
 
 	cl_git_pass(git_futils_readbuffer(&fetchhead_buf, "./foo/.git/FETCH_HEAD"));