Git development
 help / color / mirror / Atom feed
* Updates to the latest {diff,merge}tool series
From: David Aguilar @ 2009-04-07  8:21 UTC (permalink / raw)
  To: gitster; +Cc: git, charles, markus.heidelberg

Here they are.

07/14: Updated usage synopsis
09/14: Updated diff.tool config.txt documentation
10/14: The original broke the test suite on Mac OS...
       Testing {diff,merge}tool.<tool>.path is tricky.
11/14: This depends on 07/14, so the rebase requires a resend
14/14: Reworked logic as discussed.

^ permalink raw reply

* Re: [PATCH 5/5] branch: show upstream branch when double verbose
From: Paolo Ciarrocchi @ 2009-04-07  8:12 UTC (permalink / raw)
  To: Jeff King; +Cc: git
In-Reply-To: <20090407071656.GE2924@coredump.intra.peff.net>

On Tue, Apr 7, 2009 at 9:16 AM, Jeff King <peff@peff.net> wrote:
> This information is easily accessible when we are
> calculating the relationship. The only reason not to print
> it all the time is that it consumes a fair bit of screen
> space, and may not be of interest to the user.
>
> Signed-off-by: Jeff King <peff@peff.net>
> ---
> This one is very RFC. Should this information be part of the regular
> "-v"? Should it be part of "git branch" with regular verbosity?
>
> Should the format be different? I wonder if
>
>  master 1234abcd [origin/master: ahead 5, behind 6] whatever
>
> will be interpreted as "origin/master is ahead 5, behind 6" when it is
> really the reverse. Maybe "[ahead 5, behind 6 from origin/master]" would
> be better?

Yes I think so.
Thanks a lot for your work!

-Paolo

^ permalink raw reply

* Re: Performance issue: initial git clone causes massive repack
From: Björn Steinbrink @ 2009-04-07  8:10 UTC (permalink / raw)
  To: Nicolas Pitre
  Cc: Sverre Rabbelier, david, Junio C Hamano, Nicolas Sebrecht,
	Robin H. Johnson, Git Mailing List
In-Reply-To: <alpine.LFD.2.00.0904052315210.6741@xanadu.home>

On 2009.04.05 23:24:27 -0400, Nicolas Pitre wrote:
> On Sun, 5 Apr 2009, Sverre Rabbelier wrote:
> 
> > Heya,
> > 
> > On Sun, Apr 5, 2009 at 23:28,  <david@lang.hm> wrote:
> > > Guys, back off a little on telling the gentoo people to change.
> > 
> > I agree here, we should either say "look, we don't really support big
> > repositories because [explanation here], unless you [workarounds
> > here]" OR we should work to improve the support we do have. Of course,
> > the latter option does not magically create developer time to work on
> > that, but if we do go that way we should at least tell people that we
> > are aware of the problems and that it's on the global TODO list (not
> > necessarily on anyone's personal TODO list though).
> 
> For the record... I at least am aware of the problem and it is indeed on 
> my personal git todo list.  Not that I have a clear solution yet (I've 
> been pondering on some git packing issues for almost 4 years now).
> 
> Still, in this particular case, the problem appears to be unclear to me, 
> like "this shouldn't be so bad".

It's not primarily pack-objects, I think. It's the rev-list that's run
by upload-pack.  Running "git rev-list --objects --all" on that repo
eats about 2G RSS, easily killing the system's cache on a small box,
leading to swapping and a painful time reading the packfile contents
afterwards to send them to the client.

Björn

^ permalink raw reply

* Re: [PATCH] git-gui: fix use of undeclared variable diff_empty_count
From: Joerg Bornemann @ 2009-04-07  8:07 UTC (permalink / raw)
  To: git
In-Reply-To: <49DA5F20.4030800@web.de>

This bug occurs with msysgit under mysterious circumstances. We have at
least two Windows machines where "git diff" works correctly but "git 
gui" is unable to get the actual diffs. Before commit 584fa9cc, git gui 
favoured the user with an infinite message box loop. This is what 
584fa9cc should've fixed: display a nice message to the user. Instead I 
see the Tcl error message box, complaining about the missing variable 
diff_empty_count.

^ permalink raw reply

* Re: [PATCH 5/5] branch: show upstream branch when double verbose
From: Michael J Gruber @ 2009-04-07  8:02 UTC (permalink / raw)
  To: Jeff King; +Cc: git, Paolo Ciarrocchi
In-Reply-To: <20090407071656.GE2924@coredump.intra.peff.net>

Jeff King venit, vidit, dixit 07.04.2009 09:16:
> This information is easily accessible when we are
> calculating the relationship. The only reason not to print
> it all the time is that it consumes a fair bit of screen
> space, and may not be of interest to the user.
> 
> Signed-off-by: Jeff King <peff@peff.net>
> ---
> This one is very RFC. Should this information be part of the regular
> "-v"? Should it be part of "git branch" with regular verbosity?
> 
> Should the format be different? I wonder if
> 
>   master 1234abcd [origin/master: ahead 5, behind 6] whatever
> 
> will be interpreted as "origin/master is ahead 5, behind 6" when it is
> really the reverse. Maybe "[ahead 5, behind 6 from origin/master]" would
> be better?

Maybe [origin/master +5 -6]? That should be short enough for sticking it
into -v. We could even use [origin/master +0 -0] for an up-to-date
branch then.

In any case, I think often one is interested in one branch only. I would
expect "git branch -v foo" to give me the -v info just for branch foo.
Currently it does not. But that would be an independent patch on top.

>  Documentation/git-branch.txt |    4 +++-
>  builtin-branch.c             |   23 +++++++++++++++++------
>  2 files changed, 20 insertions(+), 7 deletions(-)
> 
> diff --git a/Documentation/git-branch.txt b/Documentation/git-branch.txt
> index 31ba7f2..ba3dea6 100644
> --- a/Documentation/git-branch.txt
> +++ b/Documentation/git-branch.txt
> @@ -100,7 +100,9 @@ OPTIONS
>  
>  -v::
>  --verbose::
> -	Show sha1 and commit subject line for each head.
> +	Show sha1 and commit subject line for each head, along with
> +	relationship to upstream branch (if any). If given twice, print
> +	the name of the upstream branch, as well.
>  
>  --abbrev=<length>::
>  	Alter the sha1's minimum display length in the output listing.
> diff --git a/builtin-branch.c b/builtin-branch.c
> index ca81d72..3275821 100644
> --- a/builtin-branch.c
> +++ b/builtin-branch.c
> @@ -301,19 +301,30 @@ static int ref_cmp(const void *r1, const void *r2)
>  	return strcmp(c1->name, c2->name);
>  }
>  
> -static void fill_tracking_info(struct strbuf *stat, const char *branch_name)
> +static void fill_tracking_info(struct strbuf *stat, const char *branch_name,
> +		int show_upstream_ref)
>  {
>  	int ours, theirs;
>  	struct branch *branch = branch_get(branch_name);
>  
> -	if (!stat_tracking_info(branch, &ours, &theirs) || (!ours && !theirs))
> +	if (!stat_tracking_info(branch, &ours, &theirs)) {
> +		if (branch && branch->merge && branch->merge[0]->dst &&
> +		    show_upstream_ref)
> +			strbuf_addf(stat, "[%s] ",
> +			    shorten_unambiguous_ref(branch->merge[0]->dst));
>  		return;
> +	}
> +
> +	strbuf_addch(stat, '[');
> +	if (show_upstream_ref)
> +		strbuf_addf(stat, "%s: ",
> +			shorten_unambiguous_ref(branch->merge[0]->dst));
>  	if (!ours)
> -		strbuf_addf(stat, "[behind %d] ", theirs);
> +		strbuf_addf(stat, "behind %d] ", theirs);
>  	else if (!theirs)
> -		strbuf_addf(stat, "[ahead %d] ", ours);
> +		strbuf_addf(stat, "ahead %d] ", ours);
>  	else
> -		strbuf_addf(stat, "[ahead %d, behind %d] ", ours, theirs);
> +		strbuf_addf(stat, "ahead %d, behind %d] ", ours, theirs);
>  }
>  
>  static int matches_merge_filter(struct commit *commit)
> @@ -379,7 +390,7 @@ static void print_ref_item(struct ref_item *item, int maxwidth, int verbose,
>  		}
>  
>  		if (item->kind == REF_LOCAL_BRANCH)
> -			fill_tracking_info(&stat, item->name);
> +			fill_tracking_info(&stat, item->name, verbose > 1);
>  
>  		strbuf_addf(&out, " %s %s%s",
>  			find_unique_abbrev(item->commit->object.sha1, abbrev),

^ permalink raw reply

* Re: [question] how can i verify whether a local branch is tracking a remote branch?
From: Michael J Gruber @ 2009-04-07  7:59 UTC (permalink / raw)
  To: Jeff King; +Cc: Paolo Ciarrocchi, git, Junio C Hamano
In-Reply-To: <20090406212935.GB882@coredump.intra.peff.net>

Jeff King venit, vidit, dixit 06.04.2009 23:29:
> On Mon, Apr 06, 2009 at 02:00:34PM +0200, Michael J Gruber wrote:
> 
>>> +		if (!strcmp(name, "tracking")) {
>>> +			struct branch *branch;
>>> +			if (prefixcmp(ref->refname, "refs/heads/"))
>>> +				continue;
>>> +			branch = branch_get(ref->refname + 11);
>>> +			if (branch && branch->merge && branch->merge[0] &&
>>> +			    branch->merge[0]->dst)
>>> +				v->s = branch->merge[0]->dst;
>>
>> Isn't that missing out on those cases where you --track (i.e. follow) a
>> local (upstream) branch? See
>> 5e6e2b4 (Make local branches behave like remote branches when --tracked,
>> 2009-04-01)
> 
> I thought the logic was in branch_get to handle it. And indeed:
> 
>   $ git checkout --track -b new master
>   Branch new set up to track local branch master.
>   Switched to a new branch "new"
>   $ git for-each-ref --format='%(refname) %(tracking)'
>   refs/heads/master
>   refs/heads/new refs/heads/master
> 
> So it will point either to something in refs/remotes or in refs/heads,
> as applicable.

Uhm, yes, sorry for the noise. It was actually me who fixed branch_get()...

> 
>> If we hook it up into git-branch there would be to useful directions:
> 
> The difference being that git-branch is porcelain and git-for-each-ref
> is plumbing. So they really serve different purposes.
> 
>> - "git branch --follows foo" could list all branches which follow foo,
>> analogous to --contains. It gives you all your feature work on top of
>> foo, all branches affected by rebasing foo etc.
> 
> Sure, that would probably be useful.
> 
>> - "git branch --whatever foo" could list the branch whoch foo follows.
>>
>> I just notices that "git branch -v foo" does not give me the "-v" output
>> for foo... Improving that would open up the possibility to go for -vv foo.
> 
> See the "-vv" patch I just posted elsewhere in the thread.

I'll comment there ;)

Michael

^ permalink raw reply

* Re: [PATCH 4/5] make get_short_ref a public function
From: Michael J Gruber @ 2009-04-07  7:57 UTC (permalink / raw)
  To: Jeff King; +Cc: git, Paolo Ciarrocchi
In-Reply-To: <20090407071420.GD2924@coredump.intra.peff.net>

Jeff King venit, vidit, dixit 07.04.2009 09:14:
> Often we want to shorten a full ref name to something "prettier"
> to show a user. For example, "refs/heads/master" is often shown
> simply as "master", or "refs/remotes/origin/master" is shown as
> "origin/master".
> 
> Many places in the code use a very simple formula: skip common
> prefixes like refs/heads, refs/remotes, etc. This is codified in
> the prettify_ref function.
> 
> for-each-ref has a more correct (but more expensive) approach:
> consider the ref lookup rules, and try shortening as much as
> possible while remaining unambiguous.
> 
> This patch makes the latter strategy globally available as
> shorten_unambiguous_ref.
> 
> Signed-off-by: Jeff King <peff@peff.net>
> ---
> Actually, I am not quite sure that this function is "more correct". It
> looks at the rev-parsing rules as a hierarchy, so if you have
> "refs/remotes/foo" and "refs/heads/foo", then it will abbreviate the
> first to "remotes/foo" (as expected) and the latter to just "foo".
> 
> This is technically correct, as "refs/heads/foo" will be selected by
> "foo", but it will warn about ambiguity. Should we actually try to avoid
> reporting refs which would be ambiguous?
> 
> Should this simply replace prettify_ref (and other places which should
> be using prettify_ref but aren't)? It is definitely more expensive, as
> it has to resolve refs to look for ambiguities, but I don't know if we
> care in most code paths.

I would think that as long as the default is to warn about ambiguous
refs we should not generate ambiguous refs...

Other than that it's very nice, it can be used in many places.

> 
>  builtin-for-each-ref.c |  105 +-----------------------------------------------
>  refs.c                 |   99 +++++++++++++++++++++++++++++++++++++++++++++
>  refs.h                 |    1 +
>  3 files changed, 101 insertions(+), 104 deletions(-)
> 
> diff --git a/builtin-for-each-ref.c b/builtin-for-each-ref.c
> index 277d1fb..8c82484 100644
> --- a/builtin-for-each-ref.c
> +++ b/builtin-for-each-ref.c
> @@ -546,109 +546,6 @@ static void grab_values(struct atom_value *val, int deref, struct object *obj, v
>  }
>  
>  /*
> - * generate a format suitable for scanf from a ref_rev_parse_rules
> - * rule, that is replace the "%.*s" spec with a "%s" spec
> - */
> -static void gen_scanf_fmt(char *scanf_fmt, const char *rule)
> -{
> -	char *spec;
> -
> -	spec = strstr(rule, "%.*s");
> -	if (!spec || strstr(spec + 4, "%.*s"))
> -		die("invalid rule in ref_rev_parse_rules: %s", rule);
> -
> -	/* copy all until spec */
> -	strncpy(scanf_fmt, rule, spec - rule);
> -	scanf_fmt[spec - rule] = '\0';
> -	/* copy new spec */
> -	strcat(scanf_fmt, "%s");
> -	/* copy remaining rule */
> -	strcat(scanf_fmt, spec + 4);
> -
> -	return;
> -}
> -
> -/*
> - * Shorten the refname to an non-ambiguous form
> - */
> -static char *get_short_ref(const char *ref)
> -{
> -	int i;
> -	static char **scanf_fmts;
> -	static int nr_rules;
> -	char *short_name;
> -
> -	/* pre generate scanf formats from ref_rev_parse_rules[] */
> -	if (!nr_rules) {
> -		size_t total_len = 0;
> -
> -		/* the rule list is NULL terminated, count them first */
> -		for (; ref_rev_parse_rules[nr_rules]; nr_rules++)
> -			/* no +1 because strlen("%s") < strlen("%.*s") */
> -			total_len += strlen(ref_rev_parse_rules[nr_rules]);
> -
> -		scanf_fmts = xmalloc(nr_rules * sizeof(char *) + total_len);
> -
> -		total_len = 0;
> -		for (i = 0; i < nr_rules; i++) {
> -			scanf_fmts[i] = (char *)&scanf_fmts[nr_rules]
> -					+ total_len;
> -			gen_scanf_fmt(scanf_fmts[i], ref_rev_parse_rules[i]);
> -			total_len += strlen(ref_rev_parse_rules[i]);
> -		}
> -	}
> -
> -	/* bail out if there are no rules */
> -	if (!nr_rules)
> -		return xstrdup(ref);
> -
> -	/* buffer for scanf result, at most ref must fit */
> -	short_name = xstrdup(ref);
> -
> -	/* skip first rule, it will always match */
> -	for (i = nr_rules - 1; i > 0 ; --i) {
> -		int j;
> -		int short_name_len;
> -
> -		if (1 != sscanf(ref, scanf_fmts[i], short_name))
> -			continue;
> -
> -		short_name_len = strlen(short_name);
> -
> -		/*
> -		 * check if the short name resolves to a valid ref,
> -		 * but use only rules prior to the matched one
> -		 */
> -		for (j = 0; j < i; j++) {
> -			const char *rule = ref_rev_parse_rules[j];
> -			unsigned char short_objectname[20];
> -			char refname[PATH_MAX];
> -
> -			/*
> -			 * the short name is ambiguous, if it resolves
> -			 * (with this previous rule) to a valid ref
> -			 * read_ref() returns 0 on success
> -			 */
> -			mksnpath(refname, sizeof(refname),
> -				 rule, short_name_len, short_name);
> -			if (!read_ref(refname, short_objectname))
> -				break;
> -		}
> -
> -		/*
> -		 * short name is non-ambiguous if all previous rules
> -		 * haven't resolved to a valid ref
> -		 */
> -		if (j == i)
> -			return short_name;
> -	}
> -
> -	free(short_name);
> -	return xstrdup(ref);
> -}
> -
> -
> -/*
>   * Parse the object referred by ref, and grab needed value.
>   */
>  static void populate_value(struct refinfo *ref)
> @@ -704,7 +601,7 @@ static void populate_value(struct refinfo *ref)
>  		if (formatp) {
>  			formatp++;
>  			if (!strcmp(formatp, "short"))
> -				refname = get_short_ref(refname);
> +				refname = shorten_unambiguous_ref(refname);
>  			else
>  				die("unknown %.*s format %s",
>  					formatp - name, name, formatp);
> diff --git a/refs.c b/refs.c
> index 59c373f..1e5e7b4 100644
> --- a/refs.c
> +++ b/refs.c
> @@ -1652,3 +1652,102 @@ struct ref *find_ref_by_name(const struct ref *list, const char *name)
>  			return (struct ref *)list;
>  	return NULL;
>  }
> +
> +/*
> + * generate a format suitable for scanf from a ref_rev_parse_rules
> + * rule, that is replace the "%.*s" spec with a "%s" spec
> + */
> +static void gen_scanf_fmt(char *scanf_fmt, const char *rule)
> +{
> +	char *spec;
> +
> +	spec = strstr(rule, "%.*s");
> +	if (!spec || strstr(spec + 4, "%.*s"))
> +		die("invalid rule in ref_rev_parse_rules: %s", rule);
> +
> +	/* copy all until spec */
> +	strncpy(scanf_fmt, rule, spec - rule);
> +	scanf_fmt[spec - rule] = '\0';
> +	/* copy new spec */
> +	strcat(scanf_fmt, "%s");
> +	/* copy remaining rule */
> +	strcat(scanf_fmt, spec + 4);
> +
> +	return;
> +}
> +
> +char *shorten_unambiguous_ref(const char *ref)
> +{
> +	int i;
> +	static char **scanf_fmts;
> +	static int nr_rules;
> +	char *short_name;
> +
> +	/* pre generate scanf formats from ref_rev_parse_rules[] */
> +	if (!nr_rules) {
> +		size_t total_len = 0;
> +
> +		/* the rule list is NULL terminated, count them first */
> +		for (; ref_rev_parse_rules[nr_rules]; nr_rules++)
> +			/* no +1 because strlen("%s") < strlen("%.*s") */
> +			total_len += strlen(ref_rev_parse_rules[nr_rules]);
> +
> +		scanf_fmts = xmalloc(nr_rules * sizeof(char *) + total_len);
> +
> +		total_len = 0;
> +		for (i = 0; i < nr_rules; i++) {
> +			scanf_fmts[i] = (char *)&scanf_fmts[nr_rules]
> +					+ total_len;
> +			gen_scanf_fmt(scanf_fmts[i], ref_rev_parse_rules[i]);
> +			total_len += strlen(ref_rev_parse_rules[i]);
> +		}
> +	}
> +
> +	/* bail out if there are no rules */
> +	if (!nr_rules)
> +		return xstrdup(ref);
> +
> +	/* buffer for scanf result, at most ref must fit */
> +	short_name = xstrdup(ref);
> +
> +	/* skip first rule, it will always match */
> +	for (i = nr_rules - 1; i > 0 ; --i) {
> +		int j;
> +		int short_name_len;
> +
> +		if (1 != sscanf(ref, scanf_fmts[i], short_name))
> +			continue;
> +
> +		short_name_len = strlen(short_name);
> +
> +		/*
> +		 * check if the short name resolves to a valid ref,
> +		 * but use only rules prior to the matched one
> +		 */
> +		for (j = 0; j < i; j++) {
> +			const char *rule = ref_rev_parse_rules[j];
> +			unsigned char short_objectname[20];
> +			char refname[PATH_MAX];
> +
> +			/*
> +			 * the short name is ambiguous, if it resolves
> +			 * (with this previous rule) to a valid ref
> +			 * read_ref() returns 0 on success
> +			 */
> +			mksnpath(refname, sizeof(refname),
> +				 rule, short_name_len, short_name);
> +			if (!read_ref(refname, short_objectname))
> +				break;
> +		}
> +
> +		/*
> +		 * short name is non-ambiguous if all previous rules
> +		 * haven't resolved to a valid ref
> +		 */
> +		if (j == i)
> +			return short_name;
> +	}
> +
> +	free(short_name);
> +	return xstrdup(ref);
> +}
> diff --git a/refs.h b/refs.h
> index 68c2d16..2d0f961 100644
> --- a/refs.h
> +++ b/refs.h
> @@ -80,6 +80,7 @@ extern int for_each_reflog(each_ref_fn, void *);
>  extern int check_ref_format(const char *target);
>  
>  extern const char *prettify_ref(const struct ref *ref);
> +extern char *shorten_unambiguous_ref(const char *ref);
>  
>  /** rename ref, return 0 on success **/
>  extern int rename_ref(const char *oldref, const char *newref, const char *logmsg);

^ permalink raw reply

* Re: [PATCH] for-each-ref: remove multiple xstrdup() in  get_short_ref()
From: Bert Wesarg @ 2009-04-07  7:54 UTC (permalink / raw)
  To: Jeff King; +Cc: git, Paolo Ciarrocchi
In-Reply-To: <20090407074435.GB7327@coredump.intra.peff.net>

On Tue, Apr 7, 2009 at 09:44, Jeff King <peff@peff.net> wrote:
> On Tue, Apr 07, 2009 at 09:33:19AM +0200, Bert Wesarg wrote:
>
>> Now that get_short_ref() always return an malloced string, consolidate to
>> one xstrcpy() call.
>
> Makes sense to squash in on top of what I have. But I think it actually
> is pretty easy to always return a pointer into the existing string
> (patch based on current master):
Yes, thats probably a good idea. The caller can always do a
xstrdup(get_short_ref(ref)).

> @@ -637,12 +637,14 @@ static char *get_short_ref(struct refinfo *ref)
>                 * short name is non-ambiguous if all previous rules
>                 * haven't resolved to a valid ref
>                 */
> -               if (j == i)
> -                       return short_name;
> +               if (j == i) {
> +                       ref += strlen(ref) - strlen(short_name);
we have strlen(short_name) in short_name_len already.

Bert

^ permalink raw reply

* Re: [RFC/PATCH 0/2] New 'stage' command
From: Octavio Alvarez @ 2009-04-07  7:38 UTC (permalink / raw)
  To: Junio C Hamano, David Kågedal
  Cc: git, David Aguilar, Sverre Rabbelier, markus.heidelberg,
	Felipe Contreras
In-Reply-To: <7vy6ud4otd.fsf@gitster.siamese.dyndns.org>

On Mon, 06 Apr 2009 11:30:54 -0700, Junio C Hamano <gitster@pobox.com> wrote:
>    - when you want to work with the index, you say --cached;
>
>    - when you want to work with both the index and the work tree at the
>      same time, you say --index.

This is where I also think the options are messed up.

If I want to work with the "index", shouldn't I use... uhh... "--index"?

If I want to work with both, shouldn't I use something like "--both"?

"And if --index works with the workcopy and index, will --cache use both,
the workcopy and the cache?"

To a new user like me, these options don't look like they refer to different
actions, but to different existing things. Those options say that there is
an index, and there is a cache.

(Now I learned all of this through reading the docs which is not hard, but
reading this from the man pages is not intuitive at all.)

^ permalink raw reply

* Re: [PATCH] for-each-ref: remove multiple xstrdup() in  get_short_ref()
From: Bert Wesarg @ 2009-04-07  7:44 UTC (permalink / raw)
  To: Jeff King; +Cc: git, Paolo Ciarrocchi, Bert Wesarg
In-Reply-To: <1239089599-24760-1-git-send-email-bert.wesarg@googlemail.com>

On Tue, Apr 7, 2009 at 09:33, Bert Wesarg <bert.wesarg@googlemail.com> wrote:
> Now that get_short_ref() always return an malloced string, consolidate to
> one xstrcpy() call.
>
> Signed-off-by: Bert Wesarg <bert.wesarg@googlemail.com>
>
> ---
> Also an
>
> Acked-by: Bert Wesarg <bert.wesarg@googlemail.com>
Sorry, wrong Message-ID for In-Reply-To, this Ack is for:

[PATCH 1/5] for-each-ref: refactor get_short_ref function

with Message-ID <20090407070501.GA2924@coredump.intra.peff.net>

Bert

^ permalink raw reply

* Re: [PATCH] for-each-ref: remove multiple xstrdup() in get_short_ref()
From: Jeff King @ 2009-04-07  7:44 UTC (permalink / raw)
  To: Bert Wesarg; +Cc: git, Paolo Ciarrocchi
In-Reply-To: <1239089599-24760-1-git-send-email-bert.wesarg@googlemail.com>

On Tue, Apr 07, 2009 at 09:33:19AM +0200, Bert Wesarg wrote:

> Now that get_short_ref() always return an malloced string, consolidate to
> one xstrcpy() call.

Makes sense to squash in on top of what I have. But I think it actually
is pretty easy to always return a pointer into the existing string
(patch based on current master):

---
diff --git a/builtin-for-each-ref.c b/builtin-for-each-ref.c
index 5cbb4b0..8b24a4a 100644
--- a/builtin-for-each-ref.c
+++ b/builtin-for-each-ref.c
@@ -569,7 +569,7 @@ static void gen_scanf_fmt(char *scanf_fmt, const char *rule)
 /*
  * Shorten the refname to an non-ambiguous form
  */
-static char *get_short_ref(struct refinfo *ref)
+static const char *get_short_ref(const char *ref)
 {
 	int i;
 	static char **scanf_fmts;
@@ -598,17 +598,17 @@ static char *get_short_ref(struct refinfo *ref)
 
 	/* bail out if there are no rules */
 	if (!nr_rules)
-		return ref->refname;
+		return ref;
 
 	/* buffer for scanf result, at most ref->refname must fit */
-	short_name = xstrdup(ref->refname);
+	short_name = xstrdup(ref);
 
 	/* skip first rule, it will always match */
 	for (i = nr_rules - 1; i > 0 ; --i) {
 		int j;
 		int short_name_len;
 
-		if (1 != sscanf(ref->refname, scanf_fmts[i], short_name))
+		if (1 != sscanf(ref, scanf_fmts[i], short_name))
 			continue;
 
 		short_name_len = strlen(short_name);
@@ -637,12 +637,14 @@ static char *get_short_ref(struct refinfo *ref)
 		 * short name is non-ambiguous if all previous rules
 		 * haven't resolved to a valid ref
 		 */
-		if (j == i)
-			return short_name;
+		if (j == i) {
+			ref += strlen(ref) - strlen(short_name);
+			break;
+		}
 	}
 
 	free(short_name);
-	return ref->refname;
+	return ref;
 }
 
 
@@ -684,7 +686,7 @@ static void populate_value(struct refinfo *ref)
 			if (formatp) {
 				formatp++;
 				if (!strcmp(formatp, "short"))
-					refname = get_short_ref(ref);
+					refname = get_short_ref(ref->refname);
 				else
 					die("unknown refname format %s",
 					    formatp);

^ permalink raw reply related

* Re: git over http not re-authenticating after 301 redirect?
From: Martin Storsjö @ 2009-04-07  7:42 UTC (permalink / raw)
  To: Paul Vincent Craven; +Cc: git
In-Reply-To: <5591393c0904061914y5ea26812kcfc0d14b52ed4300@mail.gmail.com>

Hi Paul,

On Mon, 6 Apr 2009, Paul Vincent Craven wrote:

> Note that after the 301 redirect, I don't seem to have a username sent
> anymore. I'm not sure this is the issue, but is seems like a
> possibility.
> 
> I have the same issue on other computers that have cloned the repository.
> 
> The /my_repo.git/info/ directory does exist.
> 
> I thought I had this working the first day I set it up, but now it no
> longer does. If someone could point me in the correct direction, I'd
> appreciate it.

It's a long shot, but try see if you can upgrade to a newer version of 
libcurl; I think I saw some issue like this when I was experimenting with 
different http auth setups last week, that was solved by using newer 
versions...

// Martin

^ permalink raw reply

* Re: [PATCH 4/5] make get_short_ref a public function
From: Bert Wesarg @ 2009-04-07  7:39 UTC (permalink / raw)
  To: Jeff King; +Cc: git, Paolo Ciarrocchi
In-Reply-To: <20090407071420.GD2924@coredump.intra.peff.net>

On Tue, Apr 7, 2009 at 09:14, Jeff King <peff@peff.net> wrote:
> Often we want to shorten a full ref name to something "prettier"
> to show a user. For example, "refs/heads/master" is often shown
> simply as "master", or "refs/remotes/origin/master" is shown as
> "origin/master".
>
> Many places in the code use a very simple formula: skip common
> prefixes like refs/heads, refs/remotes, etc. This is codified in
> the prettify_ref function.
>
> for-each-ref has a more correct (but more expensive) approach:
> consider the ref lookup rules, and try shortening as much as
> possible while remaining unambiguous.
>
> This patch makes the latter strategy globally available as
> shorten_unambiguous_ref.
>
> Signed-off-by: Jeff King <peff@peff.net>
> ---
> Actually, I am not quite sure that this function is "more correct". It
> looks at the rev-parsing rules as a hierarchy, so if you have
> "refs/remotes/foo" and "refs/heads/foo", then it will abbreviate the
> first to "remotes/foo" (as expected) and the latter to just "foo".
>
> This is technically correct, as "refs/heads/foo" will be selected by
> "foo", but it will warn about ambiguity. Should we actually try to avoid
> reporting refs which would be ambiguous?
Back than, there was the idea that the core.warnAmbiguousRefs config
could be used for this.

Anyway

Acked-by: Bert Wesarg <bert.wesarg@googlemail.com>

^ permalink raw reply

* [PATCH] for-each-ref: remove multiple xstrdup() in get_short_ref()
From: Bert Wesarg @ 2009-04-07  7:33 UTC (permalink / raw)
  To: Jeff King; +Cc: git, Paolo Ciarrocchi, Bert Wesarg
In-Reply-To: <20090407070254.GA2870@coredump.intra.peff.net>

Now that get_short_ref() always return an malloced string, consolidate to
one xstrcpy() call.

Signed-off-by: Bert Wesarg <bert.wesarg@googlemail.com>

---
Also an 

Acked-by: Bert Wesarg <bert.wesarg@googlemail.com>

for Jeffs patch.

 builtin-for-each-ref.c |   11 +++++------
 1 files changed, 5 insertions(+), 6 deletions(-)

diff --git a/builtin-for-each-ref.c b/builtin-for-each-ref.c
index 4aaf75c..108c128 100644
--- a/builtin-for-each-ref.c
+++ b/builtin-for-each-ref.c
@@ -596,13 +596,13 @@ static char *get_short_ref(const char *ref)
 		}
 	}
 
-	/* bail out if there are no rules */
-	if (!nr_rules)
-		return xstrdup(ref);
-
 	/* buffer for scanf result, at most ref must fit */
 	short_name = xstrdup(ref);
 
+	/* bail out if there are no rules */
+	if (!nr_rules)
+		return short_name;
+
 	/* skip first rule, it will always match */
 	for (i = nr_rules - 1; i > 0 ; --i) {
 		int j;
@@ -641,8 +641,7 @@ static char *get_short_ref(const char *ref)
 			return short_name;
 	}
 
-	free(short_name);
-	return xstrdup(ref);
+	return strcpy(short_name, ref);
 }
 
 
-- 
tg: (e9786d7..) bw/ammend-jk/refactor-get_short_ref (depends on: jk/refactor-get_short_ref)

^ permalink raw reply related

* Re: [PATCH 0/4] NO_PERL support
From: Jeff King @ 2009-04-07  7:31 UTC (permalink / raw)
  To: Junio C Hamano; +Cc: Robin H. Johnson, Git Mailing List
In-Reply-To: <20090403192700.GA14965@coredump.intra.peff.net>

On Fri, Apr 03, 2009 at 03:27:01PM -0400, Jeff King wrote:

> OK, here is a series based on Robin's patch that I think is suitable for
> inclusion in mainstream git. The first two are related cleanups, the
> third is a rebase of what I sent earlier today, and the fourth covers
> the matching tests.
> 
>   1/4: commit: abort commit if interactive add failed
>   2/4: tests: remove exit after test_done call
>   3/4: Makefile: allow building without perl
>   4/4: tests: skip perl tests if NO_PERL is defined

And here is a 5/4 that I missed when looking at Robin's patch.

-- >8 --
Subject: [PATCH] Makefile: disable perl entirely for empty PERL_PATH

This matches the TCLTK_PATH behavior.

Signed-off-by: Jeff King <peff@peff.net>
---
 Makefile |    4 ++++
 1 files changed, 4 insertions(+), 0 deletions(-)

diff --git a/Makefile b/Makefile
index 584a757..6e0fe24 100644
--- a/Makefile
+++ b/Makefile
@@ -1109,6 +1109,10 @@ ifeq ($(TCLTK_PATH),)
 NO_TCLTK=NoThanks
 endif
 
+ifeq ($(PERL_PATH),)
+NO_PERL=NoThanks
+endif
+
 QUIET_SUBDIR0  = +$(MAKE) -C # space to separate -C and subdir
 QUIET_SUBDIR1  =
 
-- 
1.6.2.2.450.gd6aa9.dirty

^ permalink raw reply related

* Re: [PATCH 3/4] Makefile: allow building without perl
From: Jeff King @ 2009-04-07  7:27 UTC (permalink / raw)
  To: Robin H. Johnson; +Cc: Git Mailing List
In-Reply-To: <20090405T000318Z@curie.orbis-terrarum.net>

On Sat, Apr 04, 2009 at 05:06:09PM -0700, Robin H. Johnson wrote:

> At a glance, PERL_PATH in configure.ac isn't actually used at the
> moment, the definition in the Makefile overrides it.

It is. The configure variables go into config.mak.autogen which is
included after PERL_PATH is set to a default value.

> Ok, drop the export then, but do keep at least the if(is empty
> PERL_PATH) then { set NO_PERL }, even if you won't keep the existence
> check.

Will do.

-Peff

^ permalink raw reply

* Re: [PATCH v2] Add configuration variable for sign-off to format-patch
From: Jeff King @ 2009-04-07  7:23 UTC (permalink / raw)
  To: Andreas Ericsson; +Cc: Junio C Hamano, Heiko Voigt, git
In-Reply-To: <49D9E040.40007@op5.se>

On Mon, Apr 06, 2009 at 12:58:08PM +0200, Andreas Ericsson wrote:

> How about:
> "Some projects (notably the Linux kernel and git itself) put special
> meaning in a 'Signed-off-by' line while other's dont. Please refer
> to your project's documentation for appropriate behaviour."

I think that is fine, with some typo fixups:

  s/other's/others/
  s/dont/don't/

And of course being American, I would spell it behavior. :)

> Optionally with the following amendment:
>
> "In general, you should refrain from signing off on a patch containing
> material that you're not sure can be legally spread under the project's
> license."

I don't think that makes sense. That is what signing-off is about for
the kernel and for git, but I don't think there is any reason another
project might not want to use the convenient signoff options to mean
something totally unrelated (like we use Acked-by).

-Peff

^ permalink raw reply

* [PATCH 5/5] branch: show upstream branch when double verbose
From: Jeff King @ 2009-04-07  7:16 UTC (permalink / raw)
  To: git; +Cc: Paolo Ciarrocchi
In-Reply-To: <20090407070254.GA2870@coredump.intra.peff.net>

This information is easily accessible when we are
calculating the relationship. The only reason not to print
it all the time is that it consumes a fair bit of screen
space, and may not be of interest to the user.

Signed-off-by: Jeff King <peff@peff.net>
---
This one is very RFC. Should this information be part of the regular
"-v"? Should it be part of "git branch" with regular verbosity?

Should the format be different? I wonder if

  master 1234abcd [origin/master: ahead 5, behind 6] whatever

will be interpreted as "origin/master is ahead 5, behind 6" when it is
really the reverse. Maybe "[ahead 5, behind 6 from origin/master]" would
be better?

 Documentation/git-branch.txt |    4 +++-
 builtin-branch.c             |   23 +++++++++++++++++------
 2 files changed, 20 insertions(+), 7 deletions(-)

diff --git a/Documentation/git-branch.txt b/Documentation/git-branch.txt
index 31ba7f2..ba3dea6 100644
--- a/Documentation/git-branch.txt
+++ b/Documentation/git-branch.txt
@@ -100,7 +100,9 @@ OPTIONS
 
 -v::
 --verbose::
-	Show sha1 and commit subject line for each head.
+	Show sha1 and commit subject line for each head, along with
+	relationship to upstream branch (if any). If given twice, print
+	the name of the upstream branch, as well.
 
 --abbrev=<length>::
 	Alter the sha1's minimum display length in the output listing.
diff --git a/builtin-branch.c b/builtin-branch.c
index ca81d72..3275821 100644
--- a/builtin-branch.c
+++ b/builtin-branch.c
@@ -301,19 +301,30 @@ static int ref_cmp(const void *r1, const void *r2)
 	return strcmp(c1->name, c2->name);
 }
 
-static void fill_tracking_info(struct strbuf *stat, const char *branch_name)
+static void fill_tracking_info(struct strbuf *stat, const char *branch_name,
+		int show_upstream_ref)
 {
 	int ours, theirs;
 	struct branch *branch = branch_get(branch_name);
 
-	if (!stat_tracking_info(branch, &ours, &theirs) || (!ours && !theirs))
+	if (!stat_tracking_info(branch, &ours, &theirs)) {
+		if (branch && branch->merge && branch->merge[0]->dst &&
+		    show_upstream_ref)
+			strbuf_addf(stat, "[%s] ",
+			    shorten_unambiguous_ref(branch->merge[0]->dst));
 		return;
+	}
+
+	strbuf_addch(stat, '[');
+	if (show_upstream_ref)
+		strbuf_addf(stat, "%s: ",
+			shorten_unambiguous_ref(branch->merge[0]->dst));
 	if (!ours)
-		strbuf_addf(stat, "[behind %d] ", theirs);
+		strbuf_addf(stat, "behind %d] ", theirs);
 	else if (!theirs)
-		strbuf_addf(stat, "[ahead %d] ", ours);
+		strbuf_addf(stat, "ahead %d] ", ours);
 	else
-		strbuf_addf(stat, "[ahead %d, behind %d] ", ours, theirs);
+		strbuf_addf(stat, "ahead %d, behind %d] ", ours, theirs);
 }
 
 static int matches_merge_filter(struct commit *commit)
@@ -379,7 +390,7 @@ static void print_ref_item(struct ref_item *item, int maxwidth, int verbose,
 		}
 
 		if (item->kind == REF_LOCAL_BRANCH)
-			fill_tracking_info(&stat, item->name);
+			fill_tracking_info(&stat, item->name, verbose > 1);
 
 		strbuf_addf(&out, " %s %s%s",
 			find_unique_abbrev(item->commit->object.sha1, abbrev),
-- 
1.6.2.2.450.gd6aa9.dirty

^ permalink raw reply related

* [PATCH 4/5] make get_short_ref a public function
From: Jeff King @ 2009-04-07  7:14 UTC (permalink / raw)
  To: git; +Cc: Paolo Ciarrocchi
In-Reply-To: <20090407070254.GA2870@coredump.intra.peff.net>

Often we want to shorten a full ref name to something "prettier"
to show a user. For example, "refs/heads/master" is often shown
simply as "master", or "refs/remotes/origin/master" is shown as
"origin/master".

Many places in the code use a very simple formula: skip common
prefixes like refs/heads, refs/remotes, etc. This is codified in
the prettify_ref function.

for-each-ref has a more correct (but more expensive) approach:
consider the ref lookup rules, and try shortening as much as
possible while remaining unambiguous.

This patch makes the latter strategy globally available as
shorten_unambiguous_ref.

Signed-off-by: Jeff King <peff@peff.net>
---
Actually, I am not quite sure that this function is "more correct". It
looks at the rev-parsing rules as a hierarchy, so if you have
"refs/remotes/foo" and "refs/heads/foo", then it will abbreviate the
first to "remotes/foo" (as expected) and the latter to just "foo".

This is technically correct, as "refs/heads/foo" will be selected by
"foo", but it will warn about ambiguity. Should we actually try to avoid
reporting refs which would be ambiguous?

Should this simply replace prettify_ref (and other places which should
be using prettify_ref but aren't)? It is definitely more expensive, as
it has to resolve refs to look for ambiguities, but I don't know if we
care in most code paths.

 builtin-for-each-ref.c |  105 +-----------------------------------------------
 refs.c                 |   99 +++++++++++++++++++++++++++++++++++++++++++++
 refs.h                 |    1 +
 3 files changed, 101 insertions(+), 104 deletions(-)

diff --git a/builtin-for-each-ref.c b/builtin-for-each-ref.c
index 277d1fb..8c82484 100644
--- a/builtin-for-each-ref.c
+++ b/builtin-for-each-ref.c
@@ -546,109 +546,6 @@ static void grab_values(struct atom_value *val, int deref, struct object *obj, v
 }
 
 /*
- * generate a format suitable for scanf from a ref_rev_parse_rules
- * rule, that is replace the "%.*s" spec with a "%s" spec
- */
-static void gen_scanf_fmt(char *scanf_fmt, const char *rule)
-{
-	char *spec;
-
-	spec = strstr(rule, "%.*s");
-	if (!spec || strstr(spec + 4, "%.*s"))
-		die("invalid rule in ref_rev_parse_rules: %s", rule);
-
-	/* copy all until spec */
-	strncpy(scanf_fmt, rule, spec - rule);
-	scanf_fmt[spec - rule] = '\0';
-	/* copy new spec */
-	strcat(scanf_fmt, "%s");
-	/* copy remaining rule */
-	strcat(scanf_fmt, spec + 4);
-
-	return;
-}
-
-/*
- * Shorten the refname to an non-ambiguous form
- */
-static char *get_short_ref(const char *ref)
-{
-	int i;
-	static char **scanf_fmts;
-	static int nr_rules;
-	char *short_name;
-
-	/* pre generate scanf formats from ref_rev_parse_rules[] */
-	if (!nr_rules) {
-		size_t total_len = 0;
-
-		/* the rule list is NULL terminated, count them first */
-		for (; ref_rev_parse_rules[nr_rules]; nr_rules++)
-			/* no +1 because strlen("%s") < strlen("%.*s") */
-			total_len += strlen(ref_rev_parse_rules[nr_rules]);
-
-		scanf_fmts = xmalloc(nr_rules * sizeof(char *) + total_len);
-
-		total_len = 0;
-		for (i = 0; i < nr_rules; i++) {
-			scanf_fmts[i] = (char *)&scanf_fmts[nr_rules]
-					+ total_len;
-			gen_scanf_fmt(scanf_fmts[i], ref_rev_parse_rules[i]);
-			total_len += strlen(ref_rev_parse_rules[i]);
-		}
-	}
-
-	/* bail out if there are no rules */
-	if (!nr_rules)
-		return xstrdup(ref);
-
-	/* buffer for scanf result, at most ref must fit */
-	short_name = xstrdup(ref);
-
-	/* skip first rule, it will always match */
-	for (i = nr_rules - 1; i > 0 ; --i) {
-		int j;
-		int short_name_len;
-
-		if (1 != sscanf(ref, scanf_fmts[i], short_name))
-			continue;
-
-		short_name_len = strlen(short_name);
-
-		/*
-		 * check if the short name resolves to a valid ref,
-		 * but use only rules prior to the matched one
-		 */
-		for (j = 0; j < i; j++) {
-			const char *rule = ref_rev_parse_rules[j];
-			unsigned char short_objectname[20];
-			char refname[PATH_MAX];
-
-			/*
-			 * the short name is ambiguous, if it resolves
-			 * (with this previous rule) to a valid ref
-			 * read_ref() returns 0 on success
-			 */
-			mksnpath(refname, sizeof(refname),
-				 rule, short_name_len, short_name);
-			if (!read_ref(refname, short_objectname))
-				break;
-		}
-
-		/*
-		 * short name is non-ambiguous if all previous rules
-		 * haven't resolved to a valid ref
-		 */
-		if (j == i)
-			return short_name;
-	}
-
-	free(short_name);
-	return xstrdup(ref);
-}
-
-
-/*
  * Parse the object referred by ref, and grab needed value.
  */
 static void populate_value(struct refinfo *ref)
@@ -704,7 +601,7 @@ static void populate_value(struct refinfo *ref)
 		if (formatp) {
 			formatp++;
 			if (!strcmp(formatp, "short"))
-				refname = get_short_ref(refname);
+				refname = shorten_unambiguous_ref(refname);
 			else
 				die("unknown %.*s format %s",
 					formatp - name, name, formatp);
diff --git a/refs.c b/refs.c
index 59c373f..1e5e7b4 100644
--- a/refs.c
+++ b/refs.c
@@ -1652,3 +1652,102 @@ struct ref *find_ref_by_name(const struct ref *list, const char *name)
 			return (struct ref *)list;
 	return NULL;
 }
+
+/*
+ * generate a format suitable for scanf from a ref_rev_parse_rules
+ * rule, that is replace the "%.*s" spec with a "%s" spec
+ */
+static void gen_scanf_fmt(char *scanf_fmt, const char *rule)
+{
+	char *spec;
+
+	spec = strstr(rule, "%.*s");
+	if (!spec || strstr(spec + 4, "%.*s"))
+		die("invalid rule in ref_rev_parse_rules: %s", rule);
+
+	/* copy all until spec */
+	strncpy(scanf_fmt, rule, spec - rule);
+	scanf_fmt[spec - rule] = '\0';
+	/* copy new spec */
+	strcat(scanf_fmt, "%s");
+	/* copy remaining rule */
+	strcat(scanf_fmt, spec + 4);
+
+	return;
+}
+
+char *shorten_unambiguous_ref(const char *ref)
+{
+	int i;
+	static char **scanf_fmts;
+	static int nr_rules;
+	char *short_name;
+
+	/* pre generate scanf formats from ref_rev_parse_rules[] */
+	if (!nr_rules) {
+		size_t total_len = 0;
+
+		/* the rule list is NULL terminated, count them first */
+		for (; ref_rev_parse_rules[nr_rules]; nr_rules++)
+			/* no +1 because strlen("%s") < strlen("%.*s") */
+			total_len += strlen(ref_rev_parse_rules[nr_rules]);
+
+		scanf_fmts = xmalloc(nr_rules * sizeof(char *) + total_len);
+
+		total_len = 0;
+		for (i = 0; i < nr_rules; i++) {
+			scanf_fmts[i] = (char *)&scanf_fmts[nr_rules]
+					+ total_len;
+			gen_scanf_fmt(scanf_fmts[i], ref_rev_parse_rules[i]);
+			total_len += strlen(ref_rev_parse_rules[i]);
+		}
+	}
+
+	/* bail out if there are no rules */
+	if (!nr_rules)
+		return xstrdup(ref);
+
+	/* buffer for scanf result, at most ref must fit */
+	short_name = xstrdup(ref);
+
+	/* skip first rule, it will always match */
+	for (i = nr_rules - 1; i > 0 ; --i) {
+		int j;
+		int short_name_len;
+
+		if (1 != sscanf(ref, scanf_fmts[i], short_name))
+			continue;
+
+		short_name_len = strlen(short_name);
+
+		/*
+		 * check if the short name resolves to a valid ref,
+		 * but use only rules prior to the matched one
+		 */
+		for (j = 0; j < i; j++) {
+			const char *rule = ref_rev_parse_rules[j];
+			unsigned char short_objectname[20];
+			char refname[PATH_MAX];
+
+			/*
+			 * the short name is ambiguous, if it resolves
+			 * (with this previous rule) to a valid ref
+			 * read_ref() returns 0 on success
+			 */
+			mksnpath(refname, sizeof(refname),
+				 rule, short_name_len, short_name);
+			if (!read_ref(refname, short_objectname))
+				break;
+		}
+
+		/*
+		 * short name is non-ambiguous if all previous rules
+		 * haven't resolved to a valid ref
+		 */
+		if (j == i)
+			return short_name;
+	}
+
+	free(short_name);
+	return xstrdup(ref);
+}
diff --git a/refs.h b/refs.h
index 68c2d16..2d0f961 100644
--- a/refs.h
+++ b/refs.h
@@ -80,6 +80,7 @@ extern int for_each_reflog(each_ref_fn, void *);
 extern int check_ref_format(const char *target);
 
 extern const char *prettify_ref(const struct ref *ref);
+extern char *shorten_unambiguous_ref(const char *ref);
 
 /** rename ref, return 0 on success **/
 extern int rename_ref(const char *oldref, const char *newref, const char *logmsg);
-- 
1.6.2.2.450.gd6aa9.dirty

^ permalink raw reply related

* [PATCH 3/5] for-each-ref: add "upstream" format field
From: Jeff King @ 2009-04-07  7:09 UTC (permalink / raw)
  To: git; +Cc: Paolo Ciarrocchi, Santi Béjar
In-Reply-To: <20090407070254.GA2870@coredump.intra.peff.net>

The logic for determining the upstream ref of a branch is
somewhat complex to perform in a shell script. This patch
provides a plumbing mechanism for scripts to access the C
logic used internally by git-status, git-branch, etc.

For example:

  $ git for-each-ref \
       --format='%(refname:short) %(upstream:short)' \
       refs/heads/
  master origin/master

Signed-off-by: Jeff King <peff@peff.net>
---
This is a cleaned-up version of what I sent previously. Mainly just code
cleanups, and it no longer frees the branch struct, which seems to be
allocated from semi-permanent storage during branch_get.

Should the documentation explain the concept of "upstream" more fully? I
noticed Santi sent a glossary patch earlier today, so maybe that is
enough.

 Documentation/git-for-each-ref.txt |    5 +++++
 builtin-for-each-ref.c             |   14 ++++++++++++++
 t/t6300-for-each-ref.sh            |   22 ++++++++++++++++++++++
 3 files changed, 41 insertions(+), 0 deletions(-)

diff --git a/Documentation/git-for-each-ref.txt b/Documentation/git-for-each-ref.txt
index 5061d3e..b362e9e 100644
--- a/Documentation/git-for-each-ref.txt
+++ b/Documentation/git-for-each-ref.txt
@@ -85,6 +85,11 @@ objectsize::
 objectname::
 	The object name (aka SHA-1).
 
+upstream::
+	The name of a local ref which can be considered ``upstream''
+	from the displayed ref. Respects `:short` in the same way as
+	`refname` above.
+
 In addition to the above, for commit and tag objects, the header
 field names (`tree`, `parent`, `object`, `type`, and `tag`) can
 be used to specify the value in the header field.
diff --git a/builtin-for-each-ref.c b/builtin-for-each-ref.c
index b50c93b..277d1fb 100644
--- a/builtin-for-each-ref.c
+++ b/builtin-for-each-ref.c
@@ -8,6 +8,7 @@
 #include "blob.h"
 #include "quote.h"
 #include "parse-options.h"
+#include "remote.h"
 
 /* Quoting styles */
 #define QUOTE_NONE 0
@@ -66,6 +67,7 @@ static struct {
 	{ "subject" },
 	{ "body" },
 	{ "contents" },
+	{ "upstream" },
 };
 
 /*
@@ -682,6 +684,18 @@ static void populate_value(struct refinfo *ref)
 
 		if (!prefixcmp(name, "refname"))
 			refname = ref->refname;
+		else if(!prefixcmp(name, "upstream")) {
+			struct branch *branch;
+			/* only local branches may have an upstream */
+			if (prefixcmp(ref->refname, "refs/heads/"))
+				continue;
+			branch = branch_get(ref->refname + 11);
+
+			if (!branch || !branch->merge || !branch->merge[0] ||
+			    !branch->merge[0]->dst)
+				continue;
+			refname = branch->merge[0]->dst;
+		}
 		else
 			continue;
 
diff --git a/t/t6300-for-each-ref.sh b/t/t6300-for-each-ref.sh
index 8bfae44..daf02d5 100755
--- a/t/t6300-for-each-ref.sh
+++ b/t/t6300-for-each-ref.sh
@@ -26,6 +26,13 @@ test_expect_success 'Create sample commit with known timestamp' '
 	git tag -a -m "Tagging at $datestamp" testtag
 '
 
+test_expect_success 'Create upstream config' '
+	git update-ref refs/remotes/origin/master master &&
+	git remote add origin nowhere &&
+	git config branch.master.remote origin &&
+	git config branch.master.merge refs/heads/master
+'
+
 test_atom() {
 	case "$1" in
 		head) ref=refs/heads/master ;;
@@ -39,6 +46,7 @@ test_atom() {
 }
 
 test_atom head refname refs/heads/master
+test_atom head upstream refs/remotes/origin/master
 test_atom head objecttype commit
 test_atom head objectsize 171
 test_atom head objectname 67a36f10722846e891fbada1ba48ed035de75581
@@ -68,6 +76,7 @@ test_atom head contents 'Initial
 '
 
 test_atom tag refname refs/tags/testtag
+test_atom tag upstream ''
 test_atom tag objecttype tag
 test_atom tag objectsize 154
 test_atom tag objectname 98b46b1d36e5b07909de1b3886224e3e81e87322
@@ -203,6 +212,7 @@ test_expect_success 'Check format "rfc2822" date fields output' '
 
 cat >expected <<\EOF
 refs/heads/master
+refs/remotes/origin/master
 refs/tags/testtag
 EOF
 
@@ -214,6 +224,7 @@ test_expect_success 'Verify ascending sort' '
 
 cat >expected <<\EOF
 refs/tags/testtag
+refs/remotes/origin/master
 refs/heads/master
 EOF
 
@@ -224,6 +235,7 @@ test_expect_success 'Verify descending sort' '
 
 cat >expected <<\EOF
 'refs/heads/master'
+'refs/remotes/origin/master'
 'refs/tags/testtag'
 EOF
 
@@ -244,6 +256,7 @@ test_expect_success 'Quoting style: python' '
 
 cat >expected <<\EOF
 "refs/heads/master"
+"refs/remotes/origin/master"
 "refs/tags/testtag"
 EOF
 
@@ -273,6 +286,15 @@ test_expect_success 'Check short refname format' '
 	test_cmp expected actual
 '
 
+cat >expected <<EOF
+origin/master
+EOF
+
+test_expect_success 'Check short upstream format' '
+	git for-each-ref --format="%(upstream:short)" refs/heads >actual &&
+	test_cmp expected actual
+'
+
 test_expect_success 'Check for invalid refname format' '
 	test_must_fail git for-each-ref --format="%(refname:INVALID)"
 '
-- 
1.6.2.2.450.gd6aa9.dirty

^ permalink raw reply related

* Re: [PATCH v2 14/14] difftool/mergetool: refactor commands to use git-mergetool--lib
From: David Aguilar @ 2009-04-07  7:07 UTC (permalink / raw)
  To: Markus Heidelberg; +Cc: gitster, git, benji, charles
In-Reply-To: <200904070137.17716.markus.heidelberg@web.de>

On  0, Markus Heidelberg <markus.heidelberg@web.de> wrote:
> David Aguilar, 06.04.2009:
> > diff --git a/git-mergetool--lib.sh b/git-mergetool--lib.sh
> 
> > +run_merge_tool () {
> > +	base_present="$2"
> > +	if diff_mode; then
> > +		base_present="false"
> > +	fi
> > +	if test -z "$base_present"; then
> > +		base_present="true"
> > +	fi
> 
> The second if is never true, so isn't necessary. run_merge_tool() is
> called with $2 = true or false in mergetool and $2 = "" in difftool.
> 
> But I wonder, if it would be better to change the proceeding in the
> case-esac in the next hunk below:
> 
> Currently it is:
>     if $base_present
>         mergetool with base
>     else
>         if $merge_mode
>             mergetool without base
>         else
>             difftool
>         fi
>     fi
> 
> Maybe better:
>     if $merge_mode
>         if $base_present
>             mergetool with base
>         else
>             mergetool without base
>         fi
>     else
>         difftool
>     fi
> 
> Then the first if can vanish as well and $base_present doesn't have to
> be set to false in diff_mode.
> 
> And check_unchanged() doesn't have to be called in diff_mode any more,
> $status could be set to 0 by default and doesn't have to be touched when
> in diff_mode. Only in merge_mode git-mergetool has to know, whether the
> merge went fine.
> 
> Then it will be:
>     if $merge_mode
>         touch $BACKUP
>         if $base_present
>             mergetool with base
>         else
>             mergetool without base
>         fi
> 	check_unchanged
>     else
>         difftool
>     fi
> 
> or:
>     if $merge_mode
>         if $base_present
>             mergetool with base
>         else
>             mergetool without base
>         fi
> 	status=$?
>     else
>         difftool
>     fi
> 
> Sorry for coming so late with this.
> 

Nah, I like your suggestion much better.


> > +	if echo "${VISUAL:-$EDITOR}" | grep emacs > /dev/null 2>&1; then
> > +		# $EDITOR is emacs so add emerge as a candidate
> > +		tools="$tools emerge opendiff vimdiff"
> > +	elif echo "${VISUAL:-$EDITOR}" | grep vim > /dev/null 2>&1; then
> > +		# $EDITOR is vim so add vimdiff as a candidate
> > +		tools="$tools vimdiff opendiff emerge"
> > +	else
> > +		tools="$tools opendiff emerge vimdiff"
> > +	fi
> 
> Why is opendiff here? I thought the graphical tools should go above.
> Doesn't have Mac OS $DISPLAY set?

Good catch..
Ahh.. shoot I broke the foo.<tool>.path test on Mac OS
(no tkdiff to override there).
I'll have to redo that to use test-tool.


-- 

	David

^ permalink raw reply

* [PATCH 2/5] for-each-ref: refactor refname handling
From: Jeff King @ 2009-04-07  7:06 UTC (permalink / raw)
  To: git; +Cc: Paolo Ciarrocchi
In-Reply-To: <20090407070254.GA2870@coredump.intra.peff.net>

This code handles some special magic like *-deref and the
:short formatting specifier. The next patch will add another
field which outputs a ref and wants to use the same code.

This patch splits the "which ref are we outputting" from the
actual formatting. There should be no behavioral change.

Signed-off-by: Jeff King <peff@peff.net>
---
The diff is scary, but it is mostly reindentation.

 builtin-for-each-ref.c |   47 ++++++++++++++++++++++++++---------------------
 1 files changed, 26 insertions(+), 21 deletions(-)

diff --git a/builtin-for-each-ref.c b/builtin-for-each-ref.c
index 4aaf75c..b50c93b 100644
--- a/builtin-for-each-ref.c
+++ b/builtin-for-each-ref.c
@@ -672,32 +672,37 @@ static void populate_value(struct refinfo *ref)
 		const char *name = used_atom[i];
 		struct atom_value *v = &ref->value[i];
 		int deref = 0;
+		const char *refname;
+		const char *formatp;
+
 		if (*name == '*') {
 			deref = 1;
 			name++;
 		}
-		if (!prefixcmp(name, "refname")) {
-			const char *formatp = strchr(name, ':');
-			const char *refname = ref->refname;
-
-			/* look for "short" refname format */
-			if (formatp) {
-				formatp++;
-				if (!strcmp(formatp, "short"))
-					refname = get_short_ref(ref->refname);
-				else
-					die("unknown refname format %s",
-					    formatp);
-			}
 
-			if (!deref)
-				v->s = refname;
-			else {
-				int len = strlen(refname);
-				char *s = xmalloc(len + 4);
-				sprintf(s, "%s^{}", refname);
-				v->s = s;
-			}
+		if (!prefixcmp(name, "refname"))
+			refname = ref->refname;
+		else
+			continue;
+
+		formatp = strchr(name, ':');
+		/* look for "short" refname format */
+		if (formatp) {
+			formatp++;
+			if (!strcmp(formatp, "short"))
+				refname = get_short_ref(refname);
+			else
+				die("unknown %.*s format %s",
+					formatp - name, name, formatp);
+		}
+
+		if (!deref)
+			v->s = refname;
+		else {
+			int len = strlen(refname);
+			char *s = xmalloc(len + 4);
+			sprintf(s, "%s^{}", refname);
+			v->s = s;
 		}
 	}
 
-- 
1.6.2.2.450.gd6aa9.dirty

^ permalink raw reply related

* [PATCH 1/5] for-each-ref: refactor get_short_ref function
From: Jeff King @ 2009-04-07  7:05 UTC (permalink / raw)
  To: git; +Cc: Paolo Ciarrocchi
In-Reply-To: <20090407070254.GA2870@coredump.intra.peff.net>

This function took a "refinfo" object which is unnecessarily
restrictive; it only ever looked at the refname field. This
patch refactors it to take just the ref name as a string.

While we're touching the relevant lines, let's give it
consistent memory semantics. Previously, some code paths
would return an allocated string and some would return the
original string; now it will always return a malloc'd
string.

This doesn't actually fix a bug or a leak, because
for-each-ref doesn't clean up its memory, but it makes the
function a lot less surprising for reuse (which will
happen in a later patch).

Signed-off-by: Jeff King <peff@peff.net>
---
Actually, as the modified version is always prefix-shortened,
it should be possible to rewrite this in a way that never allocates
memory. I picked the least-invasive and most lazy approach.

 builtin-for-each-ref.c |   14 +++++++-------
 1 files changed, 7 insertions(+), 7 deletions(-)

diff --git a/builtin-for-each-ref.c b/builtin-for-each-ref.c
index 5cbb4b0..4aaf75c 100644
--- a/builtin-for-each-ref.c
+++ b/builtin-for-each-ref.c
@@ -569,7 +569,7 @@ static void gen_scanf_fmt(char *scanf_fmt, const char *rule)
 /*
  * Shorten the refname to an non-ambiguous form
  */
-static char *get_short_ref(struct refinfo *ref)
+static char *get_short_ref(const char *ref)
 {
 	int i;
 	static char **scanf_fmts;
@@ -598,17 +598,17 @@ static char *get_short_ref(struct refinfo *ref)
 
 	/* bail out if there are no rules */
 	if (!nr_rules)
-		return ref->refname;
+		return xstrdup(ref);
 
-	/* buffer for scanf result, at most ref->refname must fit */
-	short_name = xstrdup(ref->refname);
+	/* buffer for scanf result, at most ref must fit */
+	short_name = xstrdup(ref);
 
 	/* skip first rule, it will always match */
 	for (i = nr_rules - 1; i > 0 ; --i) {
 		int j;
 		int short_name_len;
 
-		if (1 != sscanf(ref->refname, scanf_fmts[i], short_name))
+		if (1 != sscanf(ref, scanf_fmts[i], short_name))
 			continue;
 
 		short_name_len = strlen(short_name);
@@ -642,7 +642,7 @@ static char *get_short_ref(struct refinfo *ref)
 	}
 
 	free(short_name);
-	return ref->refname;
+	return xstrdup(ref);
 }
 
 
@@ -684,7 +684,7 @@ static void populate_value(struct refinfo *ref)
 			if (formatp) {
 				formatp++;
 				if (!strcmp(formatp, "short"))
-					refname = get_short_ref(ref);
+					refname = get_short_ref(ref->refname);
 				else
 					die("unknown refname format %s",
 					    formatp);
-- 
1.6.2.2.450.gd6aa9.dirty

^ permalink raw reply related

* [RFC/PATCH 0/5] making upstream branch information accessible
From: Jeff King @ 2009-04-07  7:02 UTC (permalink / raw)
  To: git; +Cc: Paolo Ciarrocchi

Here are slightly more cleaned-up versions of patches I've thrown out in
the last few days. The aim is to make the information on
upstream/tracking relationships more accessible both to scripts and to
users.

  1/5: for-each-ref: refactor get_short_ref function
  2/5: for-each-ref: refactor refname handling

    Cleanup for 3/5.

  3/5: for-each-ref: add "upstream" format field

    Plumbing support.

  4/5: make get_short_ref a public function

    Clean up for 4/5. Builds on 1/5.

  5/5: branch: show upstream branch when double verbose

-Peff

^ permalink raw reply

* Re: [PATCH] mailmap: resurrect lower-casing of email addresses
From: Marius Storm-Olsen @ 2009-04-07  5:52 UTC (permalink / raw)
  To: Johannes Schindelin; +Cc: git, gitster
In-Reply-To: <f182fb1700e8dea15459fd02ced2a6e5797bec99.1238458535u.git.johannes.schindelin@gmx.de>

[-- Attachment #1: Type: text/plain, Size: 687 bytes --]

Johannes Schindelin said the following on 31.03.2009 02:18:
> Commit 0925ce4(Add map_user() and clear_mailmap() to mailmap) broke the
> lower-casing of email addresses.  This mostly did not matter if your
> .mailmap has only lower-case email addresses;  However, we did not
> require .mailmap to contain lowercase-only email addresses.
> 
> Signed-off-by: Johannes Schindelin <johannes.schindelin@gmx.de>

Ouch! Sorry for missing that part! IMO, it would be correct to simply 
restore previous behavior to lowercase the whole email, so:

Ack-by: Marius Storm-Olsen <marius@trolltech.com>

-- 
.marius [@trolltech.com]
'if you know what you're doing, it's not research'


[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 187 bytes --]

^ permalink raw reply


This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox