Re: [PATCH v2] Introduce <branch>@{upstream} as shortcut to the tracked branch

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



On Thu, Sep 10, 2009 at 05:25:57PM +0200, Johannes Schindelin wrote:

> 	Changes since v1:
> 	- changed to @{upstream} (and @{u})

Hmm. After applying v2, I accidentally tried "git rev-parse @{t}" and
was surprised to find that it worked! The problem, of course, is that we
saw that it was not one of our keywords and therefore dumped it into
approxidate, which happily converted it into the current time, and
provided me with HEAD@{now}.

Which is sad, because it means "HEAD@{usptream}" will silently produce
incorrect results.

I don't see a way around it, though, short of tightening approxidate's
parsing. Maybe it could keep a flag for "I noticed _anything_ of value
in this string", and we could barf if it isn't set. That would still
allow arbitrary stuff like:

  the 6th of july

and keep Linus' favorite

  I ate 6 hot dogs in July.

but disallow the most obviously wrong dates like:

  t
  usptream
  total bogosity

> +	ret = tracked_suffix(*string, *len);
> +	if (ret) {
> +		char *ref = xstrndup(*string, *len - ret);
> +		struct branch *tracking = branch_get(*ref ? ref : NULL);
> +
> +		free(ref);
> +		if (!tracking)
> +			die ("No tracking branch found for '%s'", ref);
> +		if (tracking->merge && tracking->merge[0]->dst) {
> +			*string = xstrdup(tracking->merge[0]->dst);
> +			*len = strlen(*string);
> +			return (char *)*string;
> +		}
> +	}
> +

I don't think it is a good idea to die for !tracking, but not for
!tracking->merge. That leads to inconsistent user-visible results:

  $ git checkout HEAD^0
  $ git rev-parse HEAD@{u}
  fatal: No tracking branch found for 'HEAD'
  $ git rev-parse bogus@{u}
  bogus@{u}
  fatal: ambiguous argument 'bogus@{u}': unknown revision or path not in the working tree.
  Use '--' to separate paths from revisions

Shouldn't both cases say the same thing?

Also, your die message has two problems:

 1. It looks at ref immediately after it is free'd, spewing junk.

 2. Ref can be the empty string, which gives you the ugly:

       fatal: No tracking branch found for ''

    Should we munge that into HEAD (or "the current branch") for the
    user?

-Peff
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at  http://vger.kernel.org/majordomo-info.html

[Index of Archives]     [Linux Kernel Development]     [Gcc Help]     [IETF Annouce]     [DCCP]     [Netdev]     [Networking]     [Security]     [V4L]     [Bugtraq]     [Yosemite]     [MIPS Linux]     [ARM Linux]     [Linux Security]     [Linux RAID]     [Linux SCSI]     [Fedora Users]