Re: [PATCH 2/2] builtin/push.c: make push_default a static variable

On Tue, Feb 17, 2015 at 09:45:07AM -0800, Junio C Hamano wrote:

Jeff King [off-list ref] writes:

quoted

If we wanted to implement "@{push}" (or "@{publish}") to mean "the
tracking ref of the remote ref you would push to if you ran git-push",
then this is a step in the wrong direction.

Is that because push_default variable needs to be looked at from
sha1_name.c when resolving "@{push}", optionally prefixed with the
name of the branch?

Yes, exactly.

I wonder if that codepath should know the gory details of which ref at
the remote the branch is pushed to and which remote-tracking ref we
use in the local repository to mirror that remote ref in the first
place?

I think that was one of the ugly bits from the series; that we had to
reimplement "where would we push" and "what would it be called if we
pushed and then fetched"? The former cares about push_default, and the
latter has to apply push and then fetch refspecs.

If you want to peek at it again, it's at:

  https://github.com/peff/git/commit/8859afb1af63cb3cb0bc4cc8c1719c2011f406c9

(but note that it should not be called @{publish}, as per earlier
discussions).

What do we do for the @{upstream} side of the things---it calls
branch_get() and when the branch structure is returned, the details
have been computed for us so get_upstream_branch() only needs to use
the information already computed.  The interesting parts of the
computation all happen inside remote.c, it seems.

So we probably would do something similar to @{push} side, which
would mean that push_default variable and the logic needs to be
visible to remote.c if we want to have the helper that is similar to
set_merge() that is used from branch_get() to support @{upstream}.

Sure, we could go that way. But I don't think it changes the issue for
_this_ patch series, which is that the variable needs visibility outside
of builtin/push.c (and we need to load the config for programs besides
git-push).

Hmmm, I have a feeling that "with default configuration, where does
'git push' send this branch to?" logic should be contained within
the source file whose name has "push" in it and exposed as a helper
function, instead of exposing just one of the lowest level knob
push_default to outside callers and have them figure things out.

Viewed from that angle, it might be the case that remote.c knows too
much about what happens during fetch and pull, but I dunno.

Yeah, it would be nice if there were a convenient lib-ified set of
functions for getting this information, and "fetch" and "push" commands
were built on top of it. I don't know how painful that would be, though.
The existing code has grown somewhat organically.

But even with that change, the lib-ified code needs to hook into
git_default_config (or do its own config lookup) so that we get the
proper value no matter who the caller is.

-Peff