From: Junio C Hamano <gitster@pobox.com>
Date: Mon, 9 Sep 2019 19:26:39 +0000 (-0700)
Subject: Merge branch 'mp/for-each-ref-missing-name-or-email'
X-Git-Url: https://git.lorimer.id.au/gitweb.git/diff_plain/a477abe9e4bdf1dd3627c20b69ed8b51f27c2f81?hp=-c

Merge branch 'mp/for-each-ref-missing-name-or-email'

"for-each-ref" and friends that shows refs did not protect themselves
against ancient tags that did not record tagger names when asked to
show "%(taggername)", which have been corrected.

* mp/for-each-ref-missing-name-or-email:
ref-filter: initialize empty name or email fields
---

a477abe9e4bdf1dd3627c20b69ed8b51f27c2f81
diff --combined ref-filter.c
index f27cfc8c3e,3116f74225..7338cfc671
--- a/ref-filter.c
+++ b/ref-filter.c
@@@ -20,9 -20,6 +20,9 @@@
  #include "commit-slab.h"
  #include "commit-graph.h"
  #include "commit-reach.h"
 +#include "worktree.h"
 +#include "hashmap.h"
 +#include "argv-array.h"
  
  static struct ref_msg {
  	const char *gone;
@@@ -78,27 -75,6 +78,27 @@@ static struct expand_data 
  	struct object_info info;
  } oi, oi_deref;
  
 +struct ref_to_worktree_entry {
 +	struct hashmap_entry ent; /* must be the first member! */
 +	struct worktree *wt; /* key is wt->head_ref */
 +};
 +
 +static int ref_to_worktree_map_cmpfnc(const void *unused_lookupdata,
 +				      const void *existing_hashmap_entry_to_test,
 +				      const void *key,
 +				      const void *keydata_aka_refname)
 +{
 +	const struct ref_to_worktree_entry *e = existing_hashmap_entry_to_test;
 +	const struct ref_to_worktree_entry *k = key;
 +	return strcmp(e->wt->head_ref,
 +		keydata_aka_refname ? keydata_aka_refname : k->wt->head_ref);
 +}
 +
 +static struct ref_to_worktree_map {
 +	struct hashmap map;
 +	struct worktree **worktrees;
 +} ref_to_worktree_map;
 +
  /*
   * An atom is a valid field atom listed below, possibly prefixed with
   * a "*" to denote deref_tag().
@@@ -254,31 -230,13 +254,31 @@@ static int objecttype_atom_parser(cons
  
  static int objectsize_atom_parser(const struct ref_format *format, struct used_atom *atom,
  				  const char *arg, struct strbuf *err)
 +{
 +	if (!arg) {
 +		if (*atom->name == '*')
 +			oi_deref.info.sizep = &oi_deref.size;
 +		else
 +			oi.info.sizep = &oi.size;
 +	} else if (!strcmp(arg, "disk")) {
 +		if (*atom->name == '*')
 +			oi_deref.info.disk_sizep = &oi_deref.disk_size;
 +		else
 +			oi.info.disk_sizep = &oi.disk_size;
 +	} else
 +		return strbuf_addf_ret(err, -1, _("unrecognized %%(objectsize) argument: %s"), arg);
 +	return 0;
 +}
 +
 +static int deltabase_atom_parser(const struct ref_format *format, struct used_atom *atom,
 +				 const char *arg, struct strbuf *err)
  {
  	if (arg)
 -		return strbuf_addf_ret(err, -1, _("%%(objectsize) does not take arguments"));
 +		return strbuf_addf_ret(err, -1, _("%%(deltabase) does not take arguments"));
  	if (*atom->name == '*')
 -		oi_deref.info.sizep = &oi_deref.size;
 +		oi_deref.info.delta_base_sha1 = oi_deref.delta_base_oid.hash;
  	else
 -		oi.info.sizep = &oi.size;
 +		oi.info.delta_base_sha1 = oi.delta_base_oid.hash;
  	return 0;
  }
  
@@@ -473,7 -431,6 +473,7 @@@ static struct 
  	{ "objecttype", SOURCE_OTHER, FIELD_STR, objecttype_atom_parser },
  	{ "objectsize", SOURCE_OTHER, FIELD_ULONG, objectsize_atom_parser },
  	{ "objectname", SOURCE_OTHER, FIELD_STR, objectname_atom_parser },
 +	{ "deltabase", SOURCE_OTHER, FIELD_STR, deltabase_atom_parser },
  	{ "tree", SOURCE_OBJ },
  	{ "parent", SOURCE_OBJ },
  	{ "numparent", SOURCE_OBJ, FIELD_ULONG },
@@@ -504,16 -461,11 +504,16 @@@
  	{ "flag", SOURCE_NONE },
  	{ "HEAD", SOURCE_NONE, FIELD_STR, head_atom_parser },
  	{ "color", SOURCE_NONE, FIELD_STR, color_atom_parser },
 +	{ "worktreepath", SOURCE_NONE },
  	{ "align", SOURCE_NONE, FIELD_STR, align_atom_parser },
  	{ "end", SOURCE_NONE },
  	{ "if", SOURCE_NONE, FIELD_STR, if_atom_parser },
  	{ "then", SOURCE_NONE },
  	{ "else", SOURCE_NONE },
 +	/*
 +	 * Please update $__git_ref_fieldlist in git-completion.bash
 +	 * when you add new atoms
 +	 */
  };
  
  #define REF_FORMATTING_STATE_INIT  { 0, NULL }
@@@ -928,21 -880,17 +928,21 @@@ static void grab_common_values(struct a
  			name++;
  		if (!strcmp(name, "objecttype"))
  			v->s = xstrdup(type_name(oi->type));
 -		else if (!strcmp(name, "objectsize")) {
 +		else if (!strcmp(name, "objectsize:disk")) {
 +			v->value = oi->disk_size;
 +			v->s = xstrfmt("%"PRIuMAX, (uintmax_t)oi->disk_size);
 +		} else if (!strcmp(name, "objectsize")) {
  			v->value = oi->size;
  			v->s = xstrfmt("%"PRIuMAX , (uintmax_t)oi->size);
 -		}
 +		} else if (!strcmp(name, "deltabase"))
 +			v->s = xstrdup(oid_to_hex(&oi->delta_base_oid));
  		else if (deref)
  			grab_objectname(name, &oi->oid, v, &used_atom[i]);
  	}
  }
  
  /* See grab_values */
 -static void grab_tag_values(struct atom_value *val, int deref, struct object *obj, void *buf, unsigned long sz)
 +static void grab_tag_values(struct atom_value *val, int deref, struct object *obj)
  {
  	int i;
  	struct tag *tag = (struct tag *) obj;
@@@ -964,7 -912,7 +964,7 @@@
  }
  
  /* See grab_values */
 -static void grab_commit_values(struct atom_value *val, int deref, struct object *obj, void *buf, unsigned long sz)
 +static void grab_commit_values(struct atom_value *val, int deref, struct object *obj)
  {
  	int i;
  	struct commit *commit = (struct commit *) obj;
@@@ -997,7 -945,7 +997,7 @@@
  	}
  }
  
 -static const char *find_wholine(const char *who, int wholen, const char *buf, unsigned long sz)
 +static const char *find_wholine(const char *who, int wholen, const char *buf)
  {
  	const char *eol;
  	while (*buf) {
@@@ -1028,7 -976,7 +1028,7 @@@ static const char *copy_name(const cha
  		if (!strncmp(cp, " <", 2))
  			return xmemdupz(buf, cp - buf);
  	}
- 	return "";
+ 	return xstrdup("");
  }
  
  static const char *copy_email(const char *buf)
@@@ -1036,10 -984,10 +1036,10 @@@
  	const char *email = strchr(buf, '<');
  	const char *eoemail;
  	if (!email)
- 		return "";
+ 		return xstrdup("");
  	eoemail = strchr(email, '>');
  	if (!eoemail)
- 		return "";
+ 		return xstrdup("");
  	return xmemdupz(email, eoemail + 1 - email);
  }
  
@@@ -1093,7 -1041,7 +1093,7 @@@ static void grab_date(const char *buf, 
  }
  
  /* See grab_values */
 -static void grab_person(const char *who, struct atom_value *val, int deref, struct object *obj, void *buf, unsigned long sz)
 +static void grab_person(const char *who, struct atom_value *val, int deref, void *buf)
  {
  	int i;
  	int wholen = strlen(who);
@@@ -1114,7 -1062,7 +1114,7 @@@
  		    !starts_with(name + wholen, "date"))
  			continue;
  		if (!wholine)
 -			wholine = find_wholine(who, wholen, buf, sz);
 +			wholine = find_wholine(who, wholen, buf);
  		if (!wholine)
  			return; /* no point looking for it */
  		if (name[wholen] == 0)
@@@ -1134,7 -1082,7 +1134,7 @@@
  	if (strcmp(who, "tagger") && strcmp(who, "committer"))
  		return; /* "author" for commit object is not wanted */
  	if (!wholine)
 -		wholine = find_wholine(who, wholen, buf, sz);
 +		wholine = find_wholine(who, wholen, buf);
  	if (!wholine)
  		return;
  	for (i = 0; i < used_atom_cnt; i++) {
@@@ -1152,7 -1100,7 +1152,7 @@@
  	}
  }
  
 -static void find_subpos(const char *buf, unsigned long sz,
 +static void find_subpos(const char *buf,
  			const char **sub, unsigned long *sublen,
  			const char **body, unsigned long *bodylen,
  			unsigned long *nonsiglen,
@@@ -1221,7 -1169,7 +1221,7 @@@ static void append_lines(struct strbuf 
  }
  
  /* See grab_values */
 -static void grab_sub_body_contents(struct atom_value *val, int deref, struct object *obj, void *buf, unsigned long sz)
 +static void grab_sub_body_contents(struct atom_value *val, int deref, void *buf)
  {
  	int i;
  	const char *subpos = NULL, *bodypos = NULL, *sigpos = NULL;
@@@ -1241,7 -1189,7 +1241,7 @@@
  		    !starts_with(name, "contents"))
  			continue;
  		if (!subpos)
 -			find_subpos(buf, sz,
 +			find_subpos(buf,
  				    &subpos, &sublen,
  				    &bodypos, &bodylen, &nonsiglen,
  				    &sigpos, &siglen);
@@@ -1294,19 -1242,19 +1294,19 @@@ static void fill_missing_values(struct 
   * pointed at by the ref itself; otherwise it is the object the
   * ref (which is a tag) refers to.
   */
 -static void grab_values(struct atom_value *val, int deref, struct object *obj, void *buf, unsigned long sz)
 +static void grab_values(struct atom_value *val, int deref, struct object *obj, void *buf)
  {
  	switch (obj->type) {
  	case OBJ_TAG:
 -		grab_tag_values(val, deref, obj, buf, sz);
 -		grab_sub_body_contents(val, deref, obj, buf, sz);
 -		grab_person("tagger", val, deref, obj, buf, sz);
 +		grab_tag_values(val, deref, obj);
 +		grab_sub_body_contents(val, deref, buf);
 +		grab_person("tagger", val, deref, buf);
  		break;
  	case OBJ_COMMIT:
 -		grab_commit_values(val, deref, obj, buf, sz);
 -		grab_sub_body_contents(val, deref, obj, buf, sz);
 -		grab_person("author", val, deref, obj, buf, sz);
 -		grab_person("committer", val, deref, obj, buf, sz);
 +		grab_commit_values(val, deref, obj);
 +		grab_sub_body_contents(val, deref, buf);
 +		grab_person("author", val, deref, buf);
 +		grab_person("committer", val, deref, buf);
  		break;
  	case OBJ_TREE:
  		/* grab_tree_values(val, deref, obj, buf, sz); */
@@@ -1417,8 -1365,7 +1417,8 @@@ static void fill_remote_ref_details(str
  		*s = show_ref(&atom->u.remote_ref.refname, refname);
  	else if (atom->u.remote_ref.option == RR_TRACK) {
  		if (stat_tracking_info(branch, &num_ours, &num_theirs,
 -				       NULL, AHEAD_BEHIND_FULL) < 0) {
 +				       NULL, atom->u.remote_ref.push,
 +				       AHEAD_BEHIND_FULL) < 0) {
  			*s = xstrdup(msgs.gone);
  		} else if (!num_ours && !num_theirs)
  			*s = xstrdup("");
@@@ -1436,8 -1383,7 +1436,8 @@@
  		}
  	} else if (atom->u.remote_ref.option == RR_TRACKSHORT) {
  		if (stat_tracking_info(branch, &num_ours, &num_theirs,
 -				       NULL, AHEAD_BEHIND_FULL) < 0) {
 +				       NULL, atom->u.remote_ref.push,
 +				       AHEAD_BEHIND_FULL) < 0) {
  			*s = xstrdup("");
  			return;
  		}
@@@ -1471,36 -1417,36 +1471,36 @@@ char *get_head_description(void
  	struct strbuf desc = STRBUF_INIT;
  	struct wt_status_state state;
  	memset(&state, 0, sizeof(state));
 -	wt_status_get_state(&state, 1);
 +	wt_status_get_state(the_repository, &state, 1);
 +
 +	/*
 +	 * The ( character must be hard-coded and not part of a localizable
 +	 * string, since the description is used as a sort key and compared
 +	 * with ref names.
 +	 */
 +	strbuf_addch(&desc, '(');
  	if (state.rebase_in_progress ||
  	    state.rebase_interactive_in_progress) {
  		if (state.branch)
 -			strbuf_addf(&desc, _("(no branch, rebasing %s)"),
 +			strbuf_addf(&desc, _("no branch, rebasing %s"),
  				    state.branch);
  		else
 -			strbuf_addf(&desc, _("(no branch, rebasing detached HEAD %s)"),
 +			strbuf_addf(&desc, _("no branch, rebasing detached HEAD %s"),
  				    state.detached_from);
  	} else if (state.bisect_in_progress)
 -		strbuf_addf(&desc, _("(no branch, bisect started on %s)"),
 +		strbuf_addf(&desc, _("no branch, bisect started on %s"),
  			    state.branch);
  	else if (state.detached_from) {
  		if (state.detached_at)
 -			/*
 -			 * TRANSLATORS: make sure this matches "HEAD
 -			 * detached at " in wt-status.c
 -			 */
 -			strbuf_addf(&desc, _("(HEAD detached at %s)"),
 -				state.detached_from);
 +			strbuf_addstr(&desc, HEAD_DETACHED_AT);
  		else
 -			/*
 -			 * TRANSLATORS: make sure this matches "HEAD
 -			 * detached from " in wt-status.c
 -			 */
 -			strbuf_addf(&desc, _("(HEAD detached from %s)"),
 -				state.detached_from);
 +			strbuf_addstr(&desc, HEAD_DETACHED_FROM);
 +		strbuf_addstr(&desc, state.detached_from);
  	}
  	else
 -		strbuf_addstr(&desc, _("(no branch)"));
 +		strbuf_addstr(&desc, _("no branch"));
 +	strbuf_addch(&desc, ')');
 +
  	free(state.branch);
  	free(state.onto);
  	free(state.detached_from);
@@@ -1536,8 -1482,6 +1536,8 @@@ static int get_object(struct ref_array_
  				     OBJECT_INFO_LOOKUP_REPLACE))
  		return strbuf_addf_ret(err, -1, _("missing object %s for %s"),
  				       oid_to_hex(&oi->oid), ref->refname);
 +	if (oi->info.disk_sizep && oi->disk_size < 0)
 +		BUG("Object size is less than zero.");
  
  	if (oi->info.contentp) {
  		*obj = parse_object_buffer(the_repository, &oi->oid, oi->type, oi->size, oi->content, &eaten);
@@@ -1547,7 -1491,7 +1547,7 @@@
  			return strbuf_addf_ret(err, -1, _("parse_object_buffer failed on %s for %s"),
  					       oid_to_hex(&oi->oid), ref->refname);
  		}
 -		grab_values(ref->value, deref, *obj, oi->content, oi->size);
 +		grab_values(ref->value, deref, *obj, oi->content);
  	}
  
  	grab_common_values(ref->value, deref, oi);
@@@ -1556,48 -1500,6 +1556,48 @@@
  	return 0;
  }
  
 +static void populate_worktree_map(struct hashmap *map, struct worktree **worktrees)
 +{
 +	int i;
 +
 +	for (i = 0; worktrees[i]; i++) {
 +		if (worktrees[i]->head_ref) {
 +			struct ref_to_worktree_entry *entry;
 +			entry = xmalloc(sizeof(*entry));
 +			entry->wt = worktrees[i];
 +			hashmap_entry_init(entry, strhash(worktrees[i]->head_ref));
 +
 +			hashmap_add(map, entry);
 +		}
 +	}
 +}
 +
 +static void lazy_init_worktree_map(void)
 +{
 +	if (ref_to_worktree_map.worktrees)
 +		return;
 +
 +	ref_to_worktree_map.worktrees = get_worktrees(0);
 +	hashmap_init(&(ref_to_worktree_map.map), ref_to_worktree_map_cmpfnc, NULL, 0);
 +	populate_worktree_map(&(ref_to_worktree_map.map), ref_to_worktree_map.worktrees);
 +}
 +
 +static char *get_worktree_path(const struct used_atom *atom, const struct ref_array_item *ref)
 +{
 +	struct hashmap_entry entry;
 +	struct ref_to_worktree_entry *lookup_result;
 +
 +	lazy_init_worktree_map();
 +
 +	hashmap_entry_init(&entry, strhash(ref->refname));
 +	lookup_result = hashmap_get(&(ref_to_worktree_map.map), &entry, ref->refname);
 +
 +	if (lookup_result)
 +		return xstrdup(lookup_result->wt->path);
 +	else
 +		return xstrdup("");
 +}
 +
  /*
   * Parse the object referred by ref, and grab needed value.
   */
@@@ -1635,13 -1537,6 +1635,13 @@@ static int populate_value(struct ref_ar
  
  		if (starts_with(name, "refname"))
  			refname = get_refname(atom, ref);
 +		else if (!strcmp(name, "worktreepath")) {
 +			if (ref->kind == FILTER_REFS_BRANCHES)
 +				v->s = get_worktree_path(atom, ref);
 +			else
 +				v->s = xstrdup("");
 +			continue;
 +		}
  		else if (starts_with(name, "symref"))
  			refname = get_symref(atom, ref);
  		else if (starts_with(name, "upstream")) {
@@@ -1864,62 -1759,21 +1864,62 @@@ static int filter_pattern_match(struct 
  	return match_pattern(filter, refname);
  }
  
 -/*
 - * Find the longest prefix of pattern we can pass to
 - * `for_each_fullref_in()`, namely the part of pattern preceding the
 - * first glob character. (Note that `for_each_fullref_in()` is
 - * perfectly happy working with a prefix that doesn't end at a
 - * pathname component boundary.)
 - */
 -static void find_longest_prefix(struct strbuf *out, const char *pattern)
 +static int qsort_strcmp(const void *va, const void *vb)
  {
 -	const char *p;
 +	const char *a = *(const char **)va;
 +	const char *b = *(const char **)vb;
  
 -	for (p = pattern; *p && !is_glob_special(*p); p++)
 -		;
 +	return strcmp(a, b);
 +}
  
 -	strbuf_add(out, pattern, p - pattern);
 +static void find_longest_prefixes_1(struct string_list *out,
 +				  struct strbuf *prefix,
 +				  const char **patterns, size_t nr)
 +{
 +	size_t i;
 +
 +	for (i = 0; i < nr; i++) {
 +		char c = patterns[i][prefix->len];
 +		if (!c || is_glob_special(c)) {
 +			string_list_append(out, prefix->buf);
 +			return;
 +		}
 +	}
 +
 +	i = 0;
 +	while (i < nr) {
 +		size_t end;
 +
 +		/*
 +		* Set "end" to the index of the element _after_ the last one
 +		* in our group.
 +		*/
 +		for (end = i + 1; end < nr; end++) {
 +			if (patterns[i][prefix->len] != patterns[end][prefix->len])
 +				break;
 +		}
 +
 +		strbuf_addch(prefix, patterns[i][prefix->len]);
 +		find_longest_prefixes_1(out, prefix, patterns + i, end - i);
 +		strbuf_setlen(prefix, prefix->len - 1);
 +
 +		i = end;
 +	}
 +}
 +
 +static void find_longest_prefixes(struct string_list *out,
 +				  const char **patterns)
 +{
 +	struct argv_array sorted = ARGV_ARRAY_INIT;
 +	struct strbuf prefix = STRBUF_INIT;
 +
 +	argv_array_pushv(&sorted, patterns);
 +	QSORT(sorted.argv, sorted.argc, qsort_strcmp);
 +
 +	find_longest_prefixes_1(out, &prefix, sorted.argv, sorted.argc);
 +
 +	argv_array_clear(&sorted);
 +	strbuf_release(&prefix);
  }
  
  /*
@@@ -1932,8 -1786,7 +1932,8 @@@ static int for_each_fullref_in_pattern(
  				       void *cb_data,
  				       int broken)
  {
 -	struct strbuf prefix = STRBUF_INIT;
 +	struct string_list prefixes = STRING_LIST_INIT_DUP;
 +	struct string_list_item *prefix;
  	int ret;
  
  	if (!filter->match_as_path) {
@@@ -1959,15 -1812,21 +1959,15 @@@
  		return for_each_fullref_in("", cb, cb_data, broken);
  	}
  
 -	if (filter->name_patterns[1]) {
 -		/*
 -		 * multiple patterns; in theory this could still work as long
 -		 * as the patterns are disjoint. We'd just make multiple calls
 -		 * to for_each_ref(). But if they're not disjoint, we'd end up
 -		 * reporting the same ref multiple times. So let's punt on that
 -		 * for now.
 -		 */
 -		return for_each_fullref_in("", cb, cb_data, broken);
 -	}
 +	find_longest_prefixes(&prefixes, filter->name_patterns);
  
 -	find_longest_prefix(&prefix, filter->name_patterns[0]);
 +	for_each_string_list_item(prefix, &prefixes) {
 +		ret = for_each_fullref_in(prefix->string, cb, cb_data, broken);
 +		if (ret)
 +			break;
 +	}
  
 -	ret = for_each_fullref_in(prefix.buf, cb, cb_data, broken);
 -	strbuf_release(&prefix);
 +	string_list_clear(&prefixes, 0);
  	return ret;
  }
  
@@@ -2142,9 -2001,7 +2142,9 @@@ static void free_array_item(struct ref_
  {
  	free((char *)item->symref);
  	if (item->value) {
 -		free((char *)item->value->s);
 +		int i;
 +		for (i = 0; i < used_atom_cnt; i++)
 +			free((char *)item->value[i].s);
  		free(item->value);
  	}
  	free(item);
@@@ -2155,21 -2012,14 +2155,21 @@@ void ref_array_clear(struct ref_array *
  {
  	int i;
  
 -	for (i = 0; i < used_atom_cnt; i++)
 -		free((char *)used_atom[i].name);
 -	FREE_AND_NULL(used_atom);
 -	used_atom_cnt = 0;
  	for (i = 0; i < array->nr; i++)
  		free_array_item(array->items[i]);
  	FREE_AND_NULL(array->items);
  	array->nr = array->alloc = 0;
 +
 +	for (i = 0; i < used_atom_cnt; i++)
 +		free((char *)used_atom[i].name);
 +	FREE_AND_NULL(used_atom);
 +	used_atom_cnt = 0;
 +
 +	if (ref_to_worktree_map.worktrees) {
 +		hashmap_free(&(ref_to_worktree_map.map), 1);
 +		free_worktrees(ref_to_worktree_map.worktrees);
 +		ref_to_worktree_map.worktrees = NULL;
 +	}
  }
  
  static void do_merge_filter(struct ref_filter_cbdata *ref_cbdata)
@@@ -2458,13 -2308,8 +2458,13 @@@ void parse_ref_sorting(struct ref_sorti
  
  int parse_opt_ref_sorting(const struct option *opt, const char *arg, int unset)
  {
 -	if (!arg) /* should --no-sort void the list ? */
 -		return -1;
 +	/*
 +	 * NEEDSWORK: We should probably clear the list in this case, but we've
 +	 * already munged the global used_atoms list, which would need to be
 +	 * undone.
 +	 */
 +	BUG_ON_OPT_NEG(unset);
 +
  	parse_ref_sorting(opt->value, arg);
  	return 0;
  }
@@@ -2479,11 -2324,9 +2479,11 @@@ int parse_opt_merge_filter(const struc
  
  	if (rf->merge) {
  		if (no_merged) {
 -			return opterror(opt, "is incompatible with --merged", 0);
 +			return error(_("option `%s' is incompatible with --merged"),
 +				     opt->long_name);
  		} else {
 -			return opterror(opt, "is incompatible with --no-merged", 0);
 +			return error(_("option `%s' is incompatible with --no-merged"),
 +				     opt->long_name);
  		}
  	}
  
@@@ -2497,7 -2340,7 +2497,7 @@@
  	rf->merge_commit = lookup_commit_reference_gently(the_repository,
  							  &oid, 0);
  	if (!rf->merge_commit)
 -		return opterror(opt, "must point to a commit", 0);
 +		return error(_("option `%s' must point to a commit"), opt->long_name);
  
  	return 0;
  }
diff --combined t/t6300-for-each-ref.sh
index ab69aa176d,dc1c2e3152..9c910ce746
--- a/t/t6300-for-each-ref.sh
+++ b/t/t6300-for-each-ref.sh
@@@ -83,8 -83,6 +83,8 @@@ test_atom head push:strip=1 remotes/myf
  test_atom head push:strip=-1 master
  test_atom head objecttype commit
  test_atom head objectsize 171
 +test_atom head objectsize:disk 138
 +test_atom head deltabase 0000000000000000000000000000000000000000
  test_atom head objectname $(git rev-parse refs/heads/master)
  test_atom head objectname:short $(git rev-parse --short refs/heads/master)
  test_atom head objectname:short=1 $(git rev-parse --short=1 refs/heads/master)
@@@ -126,10 -124,6 +126,10 @@@ test_atom tag upstream '
  test_atom tag push ''
  test_atom tag objecttype tag
  test_atom tag objectsize 154
 +test_atom tag objectsize:disk 138
 +test_atom tag '*objectsize:disk' 138
 +test_atom tag deltabase 0000000000000000000000000000000000000000
 +test_atom tag '*deltabase' 0000000000000000000000000000000000000000
  test_atom tag objectname $(git rev-parse refs/tags/testtag)
  test_atom tag objectname:short $(git rev-parse --short refs/tags/testtag)
  test_atom head objectname:short=1 $(git rev-parse --short=1 refs/heads/master)
@@@ -345,32 -339,6 +345,32 @@@ test_expect_success 'Verify descending 
  	test_cmp expected actual
  '
  
 +cat >expected <<\EOF
 +refs/tags/testtag
 +refs/tags/testtag-2
 +EOF
 +
 +test_expect_success 'exercise patterns with prefixes' '
 +	git tag testtag-2 &&
 +	test_when_finished "git tag -d testtag-2" &&
 +	git for-each-ref --format="%(refname)" \
 +		refs/tags/testtag refs/tags/testtag-2 >actual &&
 +	test_cmp expected actual
 +'
 +
 +cat >expected <<\EOF
 +refs/tags/testtag
 +refs/tags/testtag-2
 +EOF
 +
 +test_expect_success 'exercise glob patterns with prefixes' '
 +	git tag testtag-2 &&
 +	test_when_finished "git tag -d testtag-2" &&
 +	git for-each-ref --format="%(refname)" \
 +		refs/tags/testtag "refs/tags/testtag-*" >actual &&
 +	test_cmp expected actual
 +'
 +
  cat >expected <<\EOF
  'refs/heads/master'
  'refs/remotes/origin/master'
@@@ -418,15 -386,8 +418,15 @@@ test_atom head upstream:track '[ahead 1
  test_atom head upstream:trackshort '>'
  test_atom head upstream:track,nobracket 'ahead 1'
  test_atom head upstream:nobracket,track 'ahead 1'
 -test_atom head push:track '[ahead 1]'
 -test_atom head push:trackshort '>'
 +
 +test_expect_success 'setup for push:track[short]' '
 +	test_commit third &&
 +	git update-ref refs/remotes/myfork/master master &&
 +	git reset master~1
 +'
 +
 +test_atom head push:track '[behind 1]'
 +test_atom head push:trackshort '<'
  
  test_expect_success 'Check that :track[short] cannot be used with other atoms' '
  	test_must_fail git for-each-ref --format="%(refname:track)" 2>/dev/null &&
@@@ -453,10 -414,8 +453,10 @@@ test_expect_success 'Check for invalid 
  test_expect_success 'set up color tests' '
  	cat >expected.color <<-EOF &&
  	$(git rev-parse --short refs/heads/master) <GREEN>master<RESET>
 +	$(git rev-parse --short refs/remotes/myfork/master) <GREEN>myfork/master<RESET>
  	$(git rev-parse --short refs/remotes/origin/master) <GREEN>origin/master<RESET>
  	$(git rev-parse --short refs/tags/testtag) <GREEN>testtag<RESET>
 +	$(git rev-parse --short refs/tags/third) <GREEN>third<RESET>
  	$(git rev-parse --short refs/tags/two) <GREEN>two<RESET>
  	EOF
  	sed "s/<[^>]*>//g" <expected.color >expected.bare &&
@@@ -526,6 -485,25 +526,25 @@@ test_expect_success 'Check ambiguous he
  	test_cmp expected actual
  '
  
+ test_expect_success 'create tag without tagger' '
+ 	git tag -a -m "Broken tag" taggerless &&
+ 	git tag -f taggerless $(git cat-file tag taggerless |
+ 		sed -e "/^tagger /d" |
+ 		git hash-object --stdin -w -t tag)
+ '
+ 
+ test_atom refs/tags/taggerless type 'commit'
+ test_atom refs/tags/taggerless tag 'taggerless'
+ test_atom refs/tags/taggerless tagger ''
+ test_atom refs/tags/taggerless taggername ''
+ test_atom refs/tags/taggerless taggeremail ''
+ test_atom refs/tags/taggerless taggerdate ''
+ test_atom refs/tags/taggerless committer ''
+ test_atom refs/tags/taggerless committername ''
+ test_atom refs/tags/taggerless committeremail ''
+ test_atom refs/tags/taggerless committerdate ''
+ test_atom refs/tags/taggerless subject 'Broken tag'
+ 
  test_expect_success 'an unusual tag with an incomplete line' '
  
  	git tag -m "bogo" bogo &&