Re: [PATCH 7/7] sha1_name: implement finding @{push}

2013-05-24 Thread Duy Nguyen
(I haven't caught up with git mails lately, but the @{special}
refactoring caught my eyes..)

On Thu, May 23, 2013 at 10:12 PM, Ramkumar Ramachandra
artag...@gmail.com wrote:
 Try this now: configure your current branch's pushremote to push to
 refs/heads/*:refs/heads/rr/*.  Now, type 'git show @{p}'.  Voila!

Voila what? Why not avoid guessing game and describe what the patch is for?

 +static void find_push_ref(struct branch *branch) {
 +   struct remote *remote = pushremote_get(NULL);
 +   const struct refspec *pat = NULL;
 +   char raw_ref[PATH_MAX];
 +   struct ref *this_ref;
 +   char *dst_name;
 +   int len;
 +
 +   sprintf(raw_ref, refs/heads/%s, branch-name);
 +   len = strlen(raw_ref) + 1;
 +   this_ref = xcalloc(1, sizeof(*this_ref) + len);
 +   memcpy(this_ref-name, raw_ref, len);
 +
 +   dst_name = get_ref_match(remote-push, remote-push_refspec_nr,
 +   this_ref, MATCH_REFS_ALL, 0, pat);
 +   printf(dst_name = %s\n, dst_name);
 +}
 +

Isn't this an abuse of extended sha-1 syntax? How can I combine this
with other @{}, ^, ~...?
--
Duy
--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: [PATCH 7/7] sha1_name: implement finding @{push}

2013-05-24 Thread Ramkumar Ramachandra
Duy Nguyen wrote:
 On Thu, May 23, 2013 at 10:12 PM, Ramkumar Ramachandra
 artag...@gmail.com wrote:
 Try this now: configure your current branch's pushremote to push to
 refs/heads/*:refs/heads/rr/*.  Now, type 'git show @{p}'.  Voila!

 Voila what? Why not avoid guessing game and describe what the patch is for?

If you're on branch master, it'll output refs/heads/rr/master.  The
topic is about having a @{push} corresponding to @{upstream}

 +static void find_push_ref(struct branch *branch) {
 +   struct remote *remote = pushremote_get(NULL);
 +   const struct refspec *pat = NULL;
 +   char raw_ref[PATH_MAX];
 +   struct ref *this_ref;
 +   char *dst_name;
 +   int len;
 +
 +   sprintf(raw_ref, refs/heads/%s, branch-name);
 +   len = strlen(raw_ref) + 1;
 +   this_ref = xcalloc(1, sizeof(*this_ref) + len);
 +   memcpy(this_ref-name, raw_ref, len);
 +
 +   dst_name = get_ref_match(remote-push, remote-push_refspec_nr,
 +   this_ref, MATCH_REFS_ALL, 0, pat);
 +   printf(dst_name = %s\n, dst_name);
 +}
 +

 Isn't this an abuse of extended sha-1 syntax? How can I combine this
 with other @{}, ^, ~...?

I'm unsure what you mean.  How can I be on branch master^1?  Did you
read the cover-letter?
--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: [PATCH 7/7] sha1_name: implement finding @{push}

2013-05-24 Thread Duy Nguyen
On Fri, May 24, 2013 at 11:15 PM, Ramkumar Ramachandra
artag...@gmail.com wrote:
 Duy Nguyen wrote:
 On Thu, May 23, 2013 at 10:12 PM, Ramkumar Ramachandra
 artag...@gmail.com wrote:
 Try this now: configure your current branch's pushremote to push to
 refs/heads/*:refs/heads/rr/*.  Now, type 'git show @{p}'.  Voila!

 Voila what? Why not avoid guessing game and describe what the patch is for?

 If you're on branch master, it'll output refs/heads/rr/master.  The
 topic is about having a @{push} corresponding to @{upstream}

Then show @{p} should show the tip commit of rr/master, not the ref
name. rev-parse (with an option, maybe) may be a better place for
this.

 +   dst_name = get_ref_match(remote-push, remote-push_refspec_nr,
 +   this_ref, MATCH_REFS_ALL, 0, pat);
 +   printf(dst_name = %s\n, dst_name);
 +}
 +

 Isn't this an abuse of extended sha-1 syntax? How can I combine this
 with other @{}, ^, ~...?

 I'm unsure what you mean.  How can I be on branch master^1?  Did you
 read the cover-letter?

I did not expect @{p} to printf(). If it's part of get_sha1(), how can
it return an sha-1? And the cover letter said 7/7 is the meat. Not
very informative.
--
Duy
--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: [PATCH 7/7] sha1_name: implement finding @{push}

2013-05-24 Thread Ramkumar Ramachandra
Duy Nguyen wrote:
 Then show @{p} should show the tip commit of rr/master, not the ref
 name.

Yes, that is correct.

 rev-parse (with an option, maybe) may be a better place for
 this.

Er, no.  I actually want things like diff @{p}..HEAD.  I want it to be
a first-class revision, just like @{u}.

 I did not expect @{p} to printf(). If it's part of get_sha1(), how can
 it return an sha-1? And the cover letter said 7/7 is the meat. Not
 very informative.

I also said sorry for the horrible mess ;)
Yes, it's obviously not supposed to print and die() with a Done!:
this was a development session, and I hit send-email as soon as I got
the right output.  It's supposed to behave exactly like @{u} (failing
to resolve occasionally).
--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: [PATCH 7/7] sha1_name: implement finding @{push}

2013-05-24 Thread Junio C Hamano
Ramkumar Ramachandra artag...@gmail.com writes:

 Duy Nguyen wrote:
 Then show @{p} should show the tip commit of rr/master, not the ref
 name.

 Yes, that is correct.

 rev-parse (with an option, maybe) may be a better place for
 this.

 Er, no.  I actually want things like diff @{p}..HEAD.  I want it to be
 a first-class revision, just like @{u}.

I think Duy's suggestion makes perfect sense; rev-parse already has
a mechanism to expand @{u} to the full name, so if you are hooking
into the same codepath as @{u} to implement the I publish there
information, which I think you should, you already should have it
for free.
--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: [PATCH 7/7] sha1_name: implement finding @{push}

2013-05-24 Thread Ramkumar Ramachandra
Junio C Hamano wrote:
 rev-parse (with an option, maybe) may be a better place for
 this.

 Er, no.  I actually want things like diff @{p}..HEAD.  I want it to be
 a first-class revision, just like @{u}.

 I think Duy's suggestion makes perfect sense; rev-parse already has
 a mechanism to expand @{u} to the full name, so if you are hooking
 into the same codepath as @{u} to implement the I publish there
 information, which I think you should, you already should have it
 for free.

*scratches head*

Okay, I'm not understanding this.  I've implemented @{push} as a
revision, so all callers who know how to get_sha1_basic() will be able
to resolve this (including rev-parse).  Are you talking about
--symbolic-full-name?  That just calls dwim_ref(), which calls
interpret_branch_name() anyway: except you get a symbolic name instead
of the sha1.  Why do I have to think about rev-parse specifically in
this patch series?  rev-parse has no special logic for @{u} either.

The codepath that resolves @{u} is in interpret_branch_name(): it's
just a matter of reading branch-merge[0]-dst; it's trivial to
determine because read_config() just reads the configuration file and
fills in these values when you get_branch().  How do I get I publish
there information for free?  Where is it contained?  In fact, it's so
complicated to get that information that I had to break my head to
even get this far (after mucking around in the transport layer);
that's what I'm trying to show in this series.  Unless I'm very badly
mistaken, it's _impossible_ for any codepath in git to determine where
a push will go, except the one activated by invoking the builtin push.

The long-term impact of this series is not just @{push}, but that
anyone else in git will be able to determine where a push is supposed
to go.  Ultimately, it can lead to very heavy optimizations of the
transport_push() codepath (which is currently super-convoluted unless
I'm missing something).
--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: [PATCH 7/7] sha1_name: implement finding @{push}

2013-05-24 Thread Junio C Hamano
Ramkumar Ramachandra artag...@gmail.com writes:

 Junio C Hamano wrote:
 rev-parse (with an option, maybe) may be a better place for
 this.

 Er, no.  I actually want things like diff @{p}..HEAD.  I want it to be
 a first-class revision, just like @{u}.

 I think Duy's suggestion makes perfect sense; rev-parse already has
 a mechanism to expand @{u} to the full name, so if you are hooking
 into the same codepath as @{u} to implement the I publish there
 information, which I think you should, you already should have it
 for free.

 *scratches head*

 Okay, I'm not understanding this.
 ...  How do I get I publish
 there information for free?

That is not what I said.

Let's step back a bit and think what it means that @{u} can be used
to name The latest, as best of my knowledge, of the possibly moving
ref that my work is based on.  You can do so by

 (1) having a ref that points at such commit and having a mechanism
 to keep the ref up to date;

 (2) having a mechansim to turn @{u} to such a ref; and

 (3) letting get_sha1() machinery to read from that ref when an
 object name is needed for @{u}.

None of the above is what rev-parse does, but because you have (2),
rev-parse --symbolic-full-name can just ask (2) for a refname.

That is what I meant by for free, and which I think you should
refers to all of these three things you would do to keep track of
The latest, as best of my knowledge, of the possibly moving ref
that points at the commit I pushed the last time for your @{p} in a
way similar to how @{u} works.

If I understand correctly the discussion so far, for a given branch
B, git push (no arguments to specify to which repository to push
nor what branch to update) would decide what the user did not say by
looking at:

- branch.B.pushremote (and using remote.pushdefault and
  branch.B.remote as fallbacks) to find what repository R to
  push to;

- branch.B.push (or mapping B through remote.R.push refspecs as
  a fallback logic) to find what destination branch D at R is
  updated by B.

Then after git push succeeds, it would look at remote.R.fetch
refspecs to pretend as if we fetched immediately from R, which is
how we keep track of the last pushed state, which is (1) of the
necessary three.

It follows that for B@{p} to be The last pushed for B, it has to
resolve to a remote tracking branch for D from R.  That (i.e. find R
and D and then map it through R's fetch refspec) is the logic we
need for (2).  That is very similar for B@{u}, which should be to
find the repository O it comes from by looking at branch.B.remote
(or 'origin'), and then where branch.B.merge from O is stored in our
ref namespace (e.g. refs/remotes/origin/topic) by mapping it through
the remote.O.fetch refspec.

Once you have (2), the implementation of (3) is quite obvious.  We
know where in the codeflow @{u} is turned into a ref; we can do the
same for @{p} by calling the logic (2) from the same place to turn
@{p} into a ref.

It is worth noting that @{p} has one error case that @{u} does not,
whose only error case is essentially that the branch you are asking
for @{u} does not have upstream configured.

For git push to work, you only need to be able to compute R and D
for B.  But R does not necessarily need to have fetch refspec for a
triangular pushing to work, hence we may not be recording what we
pushed the last time, violating (1), which in turn means (2) may not
have an answer.


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