Merge branch 'jk/upload-pack-hook'

"upload-pack" allows a custom "git pack-objects" replacement when
responding to "fetch/clone" via the uploadpack.packObjectsHook.

* jk/upload-pack-hook:
  upload-pack: provide a hook for running pack-objects
  t1308: do not get fooled by symbolic links to the source tree
  config: add a notion of "scope"
  config: return configset value for current_config_ functions
  config: set up config_source for command-line config
  git_config_parse_parameter: refactor cleanup code
  git_config_with_options: drop "found" counting

Documentation/config.txt

@@ -2892,6 +2892,21 @@ uploadpack.keepAlive::
 	`uploadpack.keepAlive` seconds. Setting this option to 0
 	disables keepalive packets entirely. The default is 5 seconds.
 
+uploadpack.packObjectsHook::
+	If this option is set, when `upload-pack` would run
+	`git pack-objects` to create a packfile for a client, it will
+	run this shell command instead.  The `pack-objects` command and
+	arguments it _would_ have run (including the `git pack-objects`
+	at the beginning) are appended to the shell command. The stdin
+	and stdout of the hook are treated as if `pack-objects` itself
+	was run. I.e., `upload-pack` will feed input intended for
+	`pack-objects` to the hook, and expects a completed packfile on
+	stdout.
++
+Note that this configuration variable is ignored if it is seen in the
+repository-level config (this is a safety measure against fetching from
+untrusted repositories).
+
 url.<base>.insteadOf::
 	Any URL that starts with this value will be rewritten to
 	start, instead, with <base>. In cases where some site serves a

cache.h

@@ -1604,6 +1604,16 @@ extern const char *get_log_output_encoding(void);
 extern const char *get_commit_output_encoding(void);
 
 extern int git_config_parse_parameter(const char *, config_fn_t fn, void *data);
+
+enum config_scope {
+	CONFIG_SCOPE_UNKNOWN = 0,
+	CONFIG_SCOPE_SYSTEM,
+	CONFIG_SCOPE_GLOBAL,
+	CONFIG_SCOPE_REPO,
+	CONFIG_SCOPE_CMDLINE,
+};
+
+extern enum config_scope current_config_scope(void);
 extern const char *current_config_origin_type(void);
 extern const char *current_config_name(void);
 
@@ -1696,6 +1706,8 @@ extern int ignore_untracked_cache_config;
 struct key_value_info {
 	const char *filename;
 	int linenr;
+	const char *origin_type;
+	enum config_scope scope;
 };
 
 extern NORETURN void git_die_config(const char *key, const char *err, ...) __attribute__((format(printf, 2, 3)));

config.c

@@ -38,7 +38,33 @@ struct config_source {
 	long (*do_ftell)(struct config_source *c);
 };
 
+/*
+ * These variables record the "current" config source, which
+ * can be accessed by parsing callbacks.
+ *
+ * The "cf" variable will be non-NULL only when we are actually parsing a real
+ * config source (file, blob, cmdline, etc).
+ *
+ * The "current_config_kvi" variable will be non-NULL only when we are feeding
+ * cached config from a configset into a callback.
+ *
+ * They should generally never be non-NULL at the same time. If they are both
+ * NULL, then we aren't parsing anything (and depending on the function looking
+ * at the variables, it's either a bug for it to be called in the first place,
+ * or it's a function which can be reused for non-config purposes, and should
+ * fall back to some sane behavior).
+ */
 static struct config_source *cf;
+static struct key_value_info *current_config_kvi;
+
+/*
+ * Similar to the variables above, this gives access to the "scope" of the
+ * current value (repo, global, etc). For cached values, it can be found via
+ * the current_config_kvi as above. During parsing, the current value can be
+ * found in this variable. It's not part of "cf" because it transcends a single
+ * file (i.e., a file included from .git/config is still in "repo" scope).
+ */
+static enum config_scope current_parsing_scope;
 
 static int zlib_compression_seen;
 
@@ -131,7 +157,9 @@ static int handle_path_include(const char *path, struct config_include_data *inc
 	if (!access_or_die(path, R_OK, 0)) {
 		if (++inc->depth > MAX_INCLUDE_DEPTH)
 			die(include_depth_advice, MAX_INCLUDE_DEPTH, path,
-			    cf && cf->name ? cf->name : "the command line");
+			    !cf ? "<unknown>" :
+			    cf->name ? cf->name :
+			    "the command line");
 		ret = git_config_from_file(git_config_include, path, inc);
 		inc->depth--;
 	}
@@ -205,32 +233,40 @@ int git_config_parse_parameter(const char *text,
 int git_config_from_parameters(config_fn_t fn, void *data)
 {
 	const char *env = getenv(CONFIG_DATA_ENVIRONMENT);
+	int ret = 0;
 	char *envw;
 	const char **argv = NULL;
 	int nr = 0, alloc = 0;
 	int i;
+	struct config_source source;
 
 	if (!env)
 		return 0;
+
+	memset(&source, 0, sizeof(source));
+	source.prev = cf;
+	cf = &source;
+
 	/* sq_dequote will write over it */
 	envw = xstrdup(env);
 
 	if (sq_dequote_to_argv(envw, &argv, &nr, &alloc) < 0) {
-		free(envw);
+		ret = error("bogus format in " CONFIG_DATA_ENVIRONMENT);
-		return error("bogus format in " CONFIG_DATA_ENVIRONMENT);
+		goto out;
 	}
 
 	for (i = 0; i < nr; i++) {
 		if (git_config_parse_parameter(argv[i], fn, data) < 0) {
-			free(argv);
+			ret = -1;
-			free(envw);
+			goto out;
-			return -1;
 		}
 	}
 
+out:
 	free(argv);
 	free(envw);
-	return nr > 0;
+	cf = source.prev;
+	return ret;
 }
 
 static int get_next_char(void)
@@ -1197,47 +1233,36 @@ int git_config_system(void)
 
 static int do_git_config_sequence(config_fn_t fn, void *data)
 {
-	int ret = 0, found = 0;
+	int ret = 0;
 	char *xdg_config = xdg_config_home("config");
 	char *user_config = expand_user_path("~/.gitconfig");
 	char *repo_config = git_pathdup("config");
 
-	if (git_config_system() && !access_or_die(git_etc_gitconfig(), R_OK, 0)) {
+	current_parsing_scope = CONFIG_SCOPE_SYSTEM;
+	if (git_config_system() && !access_or_die(git_etc_gitconfig(), R_OK, 0))
 		ret += git_config_from_file(fn, git_etc_gitconfig(),
 					    data);
-		found += 1;
-	}
 
-	if (xdg_config && !access_or_die(xdg_config, R_OK, ACCESS_EACCES_OK)) {
+	current_parsing_scope = CONFIG_SCOPE_GLOBAL;
+	if (xdg_config && !access_or_die(xdg_config, R_OK, ACCESS_EACCES_OK))
 		ret += git_config_from_file(fn, xdg_config, data);
-		found += 1;
-	}
 
-	if (user_config && !access_or_die(user_config, R_OK, ACCESS_EACCES_OK)) {
+	if (user_config && !access_or_die(user_config, R_OK, ACCESS_EACCES_OK))
 		ret += git_config_from_file(fn, user_config, data);
-		found += 1;
-	}
 
-	if (repo_config && !access_or_die(repo_config, R_OK, 0)) {
+	current_parsing_scope = CONFIG_SCOPE_REPO;
+	if (repo_config && !access_or_die(repo_config, R_OK, 0))
 		ret += git_config_from_file(fn, repo_config, data);
-		found += 1;
-	}
 
-	switch (git_config_from_parameters(fn, data)) {
+	current_parsing_scope = CONFIG_SCOPE_CMDLINE;
-	case -1: /* error */
+	if (git_config_from_parameters(fn, data) < 0)
 		die(_("unable to parse command-line config"));
-		break;
-	case 0: /* found nothing */
-		break;
-	default: /* found at least one item */
-		found++;
-		break;
-	}
 
+	current_parsing_scope = CONFIG_SCOPE_UNKNOWN;
 	free(xdg_config);
 	free(user_config);
 	free(repo_config);
-	return ret == 0 ? found : ret;
+	return ret;
 }
 
 int git_config_with_options(config_fn_t fn, void *data,
@@ -1272,7 +1297,7 @@ static void git_config_raw(config_fn_t fn, void *data)
 	if (git_config_with_options(fn, data, NULL, 1) < 0)
 		/*
 		 * git_config_with_options() normally returns only
-		 * positive values, as most errors are fatal, and
+		 * zero, as most errors are fatal, and
 		 * non-fatal potential errors are guarded by "if"
 		 * statements that are entered only when no error is
 		 * possible.
@@ -1290,16 +1315,20 @@ static void configset_iter(struct config_set *cs, config_fn_t fn, void *data)
 	struct string_list *values;
 	struct config_set_element *entry;
 	struct configset_list *list = &cs->list;
-	struct key_value_info *kv_info;
 
 	for (i = 0; i < list->nr; i++) {
 		entry = list->items[i].e;
 		value_index = list->items[i].value_index;
 		values = &entry->value_list;
-		if (fn(entry->key, values->items[value_index].string, data) < 0) {
+
-			kv_info = values->items[value_index].util;
+		current_config_kvi = values->items[value_index].util;
-			git_die_config_linenr(entry->key, kv_info->filename, kv_info->linenr);
+
-		}
+		if (fn(entry->key, values->items[value_index].string, data) < 0)
+			git_die_config_linenr(entry->key,
+					      current_config_kvi->filename,
+					      current_config_kvi->linenr);
+
+		current_config_kvi = NULL;
 	}
 }
 
@@ -1356,14 +1385,19 @@ static int configset_add_value(struct config_set *cs, const char *key, const cha
 	l_item->e = e;
 	l_item->value_index = e->value_list.nr - 1;
 
-	if (cf) {
+	if (!cf)
+		die("BUG: configset_add_value has no source");
+	if (cf->name) {
 		kv_info->filename = strintern(cf->name);
 		kv_info->linenr = cf->linenr;
+		kv_info->origin_type = strintern(cf->origin_type);
 	} else {
 		/* for values read from `git_config_from_parameters()` */
 		kv_info->filename = NULL;
 		kv_info->linenr = -1;
+		kv_info->origin_type = NULL;
 	}
+	kv_info->scope = current_parsing_scope;
 	si->util = kv_info;
 
 	return 0;
@@ -2442,10 +2476,32 @@ int parse_config_key(const char *var,
 
 const char *current_config_origin_type(void)
 {
-	return cf && cf->origin_type ? cf->origin_type : "command line";
+	const char *type;
+	if (current_config_kvi)
+		type = current_config_kvi->origin_type;
+	else if(cf)
+		type = cf->origin_type;
+	else
+		die("BUG: current_config_origin_type called outside config callback");
+	return type ? type : "command line";
 }
 
 const char *current_config_name(void)
 {
-	return cf && cf->name ? cf->name : "";
+	const char *name;
+	if (current_config_kvi)
+		name = current_config_kvi->filename;
+	else if (cf)
+		name = cf->name;
+	else
+		die("BUG: current_config_name called outside config callback");
+	return name ? name : "";
+}
+
+enum config_scope current_config_scope(void)
+{
+	if (current_config_kvi)
+		return current_config_kvi->scope;
+	else
+		return current_parsing_scope;
 }

t/helper/test-config.c

@@ -25,6 +25,9 @@
  * 				ascending order of priority from a config_set
  * 				constructed from files entered as arguments.
  *
+ * iterate -> iterate over all values using git_config(), and print some
+ *            data for each
+ *
  * Examples:
  *
  * To print the value with highest priority for key "foo.bAr Baz.rock":
@@ -32,6 +35,36 @@
  *
  */
 
+static const char *scope_name(enum config_scope scope)
+{
+	switch (scope) {
+	case CONFIG_SCOPE_SYSTEM:
+		return "system";
+	case CONFIG_SCOPE_GLOBAL:
+		return "global";
+	case CONFIG_SCOPE_REPO:
+		return "repo";
+	case CONFIG_SCOPE_CMDLINE:
+		return "cmdline";
+	default:
+		return "unknown";
+	}
+}
+static int iterate_cb(const char *var, const char *value, void *data)
+{
+	static int nr;
+
+	if (nr++)
+		putchar('\n');
+
+	printf("key=%s\n", var);
+	printf("value=%s\n", value ? value : "(null)");
+	printf("origin=%s\n", current_config_origin_type());
+	printf("name=%s\n", current_config_name());
+	printf("scope=%s\n", scope_name(current_config_scope()));
+
+	return 0;
+}
 
 int main(int argc, char **argv)
 {
@@ -134,6 +167,9 @@ int main(int argc, char **argv)
 			printf("Value not found for \"%s\"\n", argv[2]);
 			goto exit1;
 		}
+	} else if (!strcmp(argv[1], "iterate")) {
+		git_config(iterate_cb, NULL);
+		goto exit0;
 	}
 
 	die("%s: Please check the syntax and the function name", argv[0]);

t/t1308-config-set.sh

@@ -229,4 +229,31 @@ test_expect_success 'error on modifying repo config without repo' '
 	)
 '
 
+cmdline_config="'foo.bar=from-cmdline'"
+test_expect_success 'iteration shows correct origins' '
+	echo "[foo]bar = from-repo" >.git/config &&
+	echo "[foo]bar = from-home" >.gitconfig &&
+	cat >expect <<-EOF &&
+	key=foo.bar
+	value=from-home
+	origin=file
+	name=$HOME/.gitconfig
+	scope=global
+
+	key=foo.bar
+	value=from-repo
+	origin=file
+	name=.git/config
+	scope=repo
+
+	key=foo.bar
+	value=from-cmdline
+	origin=command line
+	name=
+	scope=cmdline
+	EOF
+	GIT_CONFIG_PARAMETERS=$cmdline_config test-config iterate >actual &&
+	test_cmp expect actual
+'
+
 test_done

t/t5544-pack-objects-hook.sh

@@ -0,0 +1,62 @@
+#!/bin/sh
+
+test_description='test custom script in place of pack-objects'
+. ./test-lib.sh
+
+test_expect_success 'create some history to fetch' '
+	test_commit one &&
+	test_commit two
+'
+
+test_expect_success 'create debugging hook script' '
+	write_script .git/hook <<-\EOF
+		echo >&2 "hook running"
+		echo "$*" >hook.args
+		cat >hook.stdin
+		"$@" <hook.stdin >hook.stdout
+		cat hook.stdout
+	EOF
+'
+
+clear_hook_results () {
+	rm -rf .git/hook.* dst.git
+}
+
+test_expect_success 'hook runs via global config' '
+	clear_hook_results &&
+	test_config_global uploadpack.packObjectsHook ./hook &&
+	git clone --no-local . dst.git 2>stderr &&
+	grep "hook running" stderr
+'
+
+test_expect_success 'hook outputs are sane' '
+	# check that we recorded a usable pack
+	git index-pack --stdin <.git/hook.stdout &&
+
+	# check that we recorded args and stdin. We do not check
+	# the full argument list or the exact pack contents, as it would make
+	# the test brittle. So just sanity check that we could replay
+	# the packing procedure.
+	grep "^git" .git/hook.args &&
+	$(cat .git/hook.args) <.git/hook.stdin >replay
+'
+
+test_expect_success 'hook runs from -c config' '
+	clear_hook_results &&
+	git clone --no-local \
+	  -u "git -c uploadpack.packObjectsHook=./hook upload-pack" \
+	  . dst.git 2>stderr &&
+	grep "hook running" stderr
+'
+
+test_expect_success 'hook does not run from repo config' '
+	clear_hook_results &&
+	test_config uploadpack.packObjectsHook "./hook" &&
+	git clone --no-local . dst.git 2>stderr &&
+	! grep "hook running" stderr &&
+	test_path_is_missing .git/hook.args &&
+	test_path_is_missing .git/hook.stdin &&
+	test_path_is_missing .git/hook.stdout
+'
+
+test_done

upload-pack.c

@@ -56,6 +56,7 @@ static int keepalive = 5;
 static int use_sideband;
 static int advertise_refs;
 static int stateless_rpc;
+static const char *pack_objects_hook;
 
 static void reset_timeout(void)
 {
@@ -98,6 +99,14 @@ static void create_pack_file(void)
 	int i;
 	FILE *pipe_fd;
 
+	if (!pack_objects_hook)
+		pack_objects.git_cmd = 1;
+	else {
+		argv_array_push(&pack_objects.args, pack_objects_hook);
+		argv_array_push(&pack_objects.args, "git");
+		pack_objects.use_shell = 1;
+	}
+
 	if (shallow_nr) {
 		argv_array_push(&pack_objects.args, "--shallow-file");
 		argv_array_push(&pack_objects.args, "");
@@ -120,7 +129,6 @@ static void create_pack_file(void)
 	pack_objects.in = -1;
 	pack_objects.out = -1;
 	pack_objects.err = -1;
-	pack_objects.git_cmd = 1;
 
 	if (start_command(&pack_objects))
 		die("git upload-pack: unable to fork git-pack-objects");
@@ -813,6 +821,9 @@ static int upload_pack_config(const char *var, const char *value, void *unused)
 		keepalive = git_config_int(var, value);
 		if (!keepalive)
 			keepalive = -1;
+	} else if (current_config_scope() != CONFIG_SCOPE_REPO) {
+		if (!strcmp("uploadpack.packobjectshook", var))
+			return git_config_string(&pack_objects_hook, var, value);
 	}
 	return parse_hide_refs_config(var, value, "uploadpack");
 }