patch, 7 partsLet's get that @{push}!

[7/7] is the meat. Sorry it's in such a messy state: I was having a field day tracing what push is actually doing. Anyway, I wanted to send out the series now to get early feedback.

In other news: why on earth is push doing _so_ much processing before pushing? Is it written very badly, or am I missing something?

Thanks.

(based on rr/die-on-missing-upstream)

Ramkumar Ramachandra (7):
  sha1_name: abstract upstream_mark() logic
  sha1_name: factor out die_no_upstream()
  sha1_name: remove upstream_mark()
  remote: expose parse_push_refspec()
  remote: expose get_ref_match()
  sha1_name: prepare to introduce AT_KIND_PUSH
  sha1_name: implement finding @{push}

 remote.c    |   4 +--
 remote.h    |   4 +++
 sha1_name.c | 111 ++++++++++++++++++++++++++++++++++++++++++++----------------
 3 files changed, 88 insertions(+), 31 deletions(-)

Currently, the only non-numerical @{...} expression we support is @{u[pstream]}. Since we're slowly growing git to support triangular workflows, it might make sense to have a @{p[ush]} and various other special @{...} expressions in the future. To prepare for this, abstract out the upstream_mark() logic to accept any suffix, while preserving the upstream_mark() interface.

Signed-off-by: Ramkumar Ramachandra <artagnon@gmail.com>
---
 sha1_name.c | 40 +++++++++++++++++++++++++++++++---------
 1 file changed, 31 insertions(+), 9 deletions(-)

Currently interpret_branch_name() tries to parse various things, and finally falls back to parsing <branch>@{u[pstream]}. It dies if the input string contained an "@{u[pstream]}" but an upstream could not be found. The logic can be generalized to check for any branch property after branch_get(). In preparation for introducing more special @{...} forms, factor out die_no_upstream().

Signed-off-by: Ramkumar Ramachandra <artagnon@gmail.com>
---
 sha1_name.c | 40 ++++++++++++++++++++++------------------
 1 file changed, 22 insertions(+), 18 deletions(-)

The first caller get_sha1_basic() just wants to make sure that no non-numerical @{...} form was matched, so that it can proceed with processing numerical @{...} forms. Since we're going to introduce more non-numerical @{...} forms, replace this upstream_mark() call with a call to at_mark() passing NULL as the last argument; we don't care what the kind is: all we need to know is if the return value is zero (parse failure).

The second caller interpret_branch_name() will be expanded in the future to handle all possible AT_KIND_* values. So, replace the upstream_mark() call with an upstream_mark() call capturing at_kind and using it in a switch statement to perform the appropriate action.

Signed-off-by: Ramkumar Ramachandra <artagnon@gmail.com>
---
 sha1_name.c | 26 ++++++++++++--------------
 1 file changed, 12 insertions(+), 14 deletions(-)

parse_fetch_refspec() is already available to other callers via remote.h. There's no reason why parse_push_refspec() shouldn't be.

Signed-off-by: Ramkumar Ramachandra <artagnon@gmail.com>
---
 remote.c | 2 +-
 remote.h | 1 +
 2 files changed, 2 insertions(+), 1 deletion(-)

We need it.

Signed-off-by: Ramkumar Ramachandra <artagnon@gmail.com>
---
 remote.c | 2 +-
 remote.h | 3 +++
 2 files changed, 4 insertions(+), 1 deletion(-)

Introduce an AT_KIND_PUSH to be represented as "@{p[ush]}". Determine it using branch.remote.push_refspec.

Signed-off-by: Ramkumar Ramachandra <artagnon@gmail.com>
---
 sha1_name.c | 13 +++++++++++--
 1 file changed, 11 insertions(+), 2 deletions(-)

On Thu, May 23, 2013 at 10:12 AM, Ramkumar Ramachandra <artagnon@gmail.com> wrote:

I think the semantics of this don't make any sense.

  git push branch@{upstream}

Is very clear: push upstream of branch.

  git push branch@{push}

Is not clear at all: push push of branch?

Is it a noun, or a verb?

I would expect branch@{X} to be 'the X of branch', and "the push of branch" doesn't make sense. Whatever X ends up being, it should be a noun.

Felipe Contreras wrote:

We can pick the name later. I had to pick a name to write code, and that happens to be @{push}.

Try this now: configure your current branch's pushremote to push to "refs/heads/*:refs/heads/rr/*". Now, type 'git show @{p}'. Voila!

It currently only works when:

1. remote.<name>.push is explicitly specified.
2. There is a pattern to match (*).

Proof-of-concept only.

Signed-off-by: Ramkumar Ramachandra <artagnon@gmail.com>
---
 sha1_name.c | 20 ++++++++++++++++++++
 1 file changed, 20 insertions(+)

(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 <artagnon@gmail.com> wrote:

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

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

Duy Nguyen wrote:

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

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

On Fri, May 24, 2013 at 11:15 PM, Ramkumar Ramachandra <artagnon@gmail.com> wrote:

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.

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

Duy Nguyen wrote:

Yes, that is correct.

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

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).

Ramkumar Ramachandra <artagnon@gmail.com> writes:

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.

Junio C Hamano wrote:

*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).

Ramkumar Ramachandra <artagnon@gmail.com> writes:

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.