patch, 5 partsAdd --remote option to send-pack

With the --remote option, send-pack will look at the remote configuration for the specified remote (which must match the destination URL), and update the local tracking refs to match changes it causes in the remote heads they track.

The previous values of local tracking heads are ignored.

Signed-off-by: Daniel Barkalow <barkalow@iabervon.org>
---
 send-pack.c |   54 +++++++++++++++++++++++++++++++++++++++++++++---------
 1 files changed, 45 insertions(+), 9 deletions(-)

Daniel Barkalow <barkalow@iabervon.org> writes:

I didn't actually look at the code of this patch, as I am rejecting the initial round of [PATCH 2/5] this depends on, but this is one thing Cogito did that we don't, and I think it makes sort of sense. We pretend as if we fetched back immediately after we pushed without giving anybody a chance to further update the refs we updated at the remote. I generally do not like "pretend that we did something" when talking about reliable operations (and SCM _is_ all about reliable operation), and this behaviour is exactly it, but I think this one is excusable.

On Sat, 28 Apr 2007, Junio C Hamano wrote:

I'll redo the series soon, but I wanted to respond to the commentary on the general concept.

We're not pretending anything; remote has confirmed that the head that the ref tracks has a particular new value (which we provided), so we should be able to update the tracking ref to that value. I don't think it's particularly important that we came by this information in the course of an exchange that wasn't a fetch.

	-Daniel
*This .sig left intentionally blank*

Daniel Barkalow <barkalow@iabervon.org> wrote:

s/remote has confirmed/remote has claimed/

The only way to know the remote really did remember that refs/heads/foo is now at 3c1718... is to perform some sort of operation against it that makes it dump its refs back. Trying to push again, or trying to fetch does that.

There is also the possibility that a post-update or post-receive hook will actually modify the ref *after* the push is "complete". But that's like so crazy that I really don't think anyone has a workflow that does that. They might have such a hook that creates a new ref however, or updates a totally unrelated ref (e.g. compile the code and then update a "last-built" ref).

So in this particular case I have to agree with Daniel, its probably OK to do what Cogito does and update the tracking branch after the push was claimed to be successful by the remote.

Daniel Barkalow <barkalow@iabervon.org> writes:

We are indeed pretending.  Consider:

 (1) You push, and push succeeds.

 (2) Somebody fetches your result, works on it and pushes back;
     this might happen in post-receive hook.

 (3) You fetch.  You should see somebody else's commit at the
     tip, not what you pushed in (1).

By not fetching but instead of storing what you pushed, you are pretending that you re-fetched so fast that you gave no chance to anybody to perform (2) quickly enough.

But my conclusion is that this does not matter in practice, and I am agreeing with what you tried to do. That does not change the fact that we are pretending.

On Sat, 28 Apr 2007, Junio C Hamano wrote:

But you did effectively re-fetch instantaneously by doing an operation that atomicly updates the ref and reports success. If you consider the operation that updates tracking refs not to be necessarily "git-fetch" but rather "having a connection that discusses refs", this sequence isn't any different from (1) being a fetch instead of a push; there's always the chance that your interaction with the server is shortly before the ref values you update to being replaced, whether the operation you're doing is getting the remote's value or setting the remote's value.

Even if a post-update hook further changes the ref you push to, it had better not change it to a non-descendant of the value you pushed, since there would be a race with other people fetching your value with the post-update hook is running and seeing history roll back. So, at worst, you could have the situation where, effectively, you *always* miss out on the latest thing, but it's already the case that you would *sometimes* miss out on the latest thing, if the post-update hook is running more slowly than your next fetch.

The fact that the config option is "fetch" isn't really an argument that the way you have to get the knowledge of the value of the ref at the instant you remember locally has to be "git-fetch"; any way of getting knowledge of that value at some instant should suffice.

	-Daniel
*This .sig left intentionally blank*

Daniel Barkalow <barkalow@iabervon.org> writes:

I do not think there is much point arguing over this; I am not fundamentally opposed to keeping a copy of what we just pushed to the other side.

But I think it needs to be documented that hooks on the remote side could do funny things, and probably we should strongly discourage people from doing such.

You do need to take care of the case where we are _not_ tracking the remote side, though (i.e. lack of colon in the fetch refspecs).

On Wed, 2 May 2007, Junio C Hamano wrote:

I don't see what's any *more* odd about this situation with my change than otherwise (and I'm afraid I'm missing something, which is why I'm still discussing it).

Indeed. Is there documentation on all the possibilities for refspecs somewhere all together? I could only find it for particular programs, and didn't see a no-colon option permitted anywhere.

	-Daniel
*This .sig left intentionally blank*

Daniel Barkalow <barkalow@iabervon.org> writes:

Documentation/pull-fetch-param.txt which is included by various manpages would be the best place to start.

    <refspec>::
            The canonical format of a <refspec> parameter is
            `+?<src>:<dst>`; that is, an optional plus `+`, followed
            by the source ref, followed by a colon `:`, followed by
            the destination ref.
    +
    The remote ref that matches <src>
    is fetched, and if <dst> is not empty string, the local
    ref that matches it is fast forwarded using <src>.
    Again, if the optional plus `+` is used, the local ref
    is updated even if it does not result in a fast forward
    update.

Hmph. So <dst> could be empty, and in such a case there is no tracking. But this does not say missing colon is allowed (but that was intentionally left out as we clarify it later). Let's read on. After a few [Notes], we find this.

    Some short-cut notations are also supported.
    +
    * `tag <tag>` means the same as `refs/tags/<tag>:refs/tags/<tag>`; 
      it requests fetching everything up to the given tag.
    * A parameter <ref> without a colon is equivalent to
      <ref>: when pulling/fetching, so it merges <ref> into the current
      branch without storing the remote branch anywhere locally

Ahh, so a refspec that does not have a colon, and ends with a colon (hence an empty <dst>) are equivalent, and does not result in remote tracking.

On Wed, 2 May 2007, Junio C Hamano wrote:

Is this actually supported in config files? At least for pulls, the "tag <tag>" notation is only available on the command line, afaict. For that matter, I think that having "<ref>:" or "<ref>" in a config file fetch line would be really bad; these refs would always be merged into any current head (without any remote tracking) when the remote is fetched from?

	-Daniel
*This .sig left intentionally blank*

Daniel Barkalow <barkalow@iabervon.org> writes:

That is the only difference as far as I remember. But you are right in that the set of refspecs allowed on the command line does not have to coincide with the ones in remotes file or the config (the latter two should match, though), and the latter may not be documented (the intention was to match all of them so the quoted documentation was enough, but having "tag <tag>" in config does not make sense). Documentation/config.txt could use a bit of updates. Hint, hint.

It's not bad at all. Why people are so eager to _force_ tracking on other people, I do not understand.

I had a moral equivalent of this in .git/remotes/origin (back then .git/config did not exist) when I was a contributor:

	[remote "origin"]
        	url = git.kernel.org:/pub/scm/git/git.git/
                fetch = refs/heads/master

If the user does not want to track, he does not have to.

On Fri, 4 May 2007, Junio C Hamano wrote:

Once I've figured out what the intended behavior is, and have a canonical implementation so that it clearly does that, I'll document it. Until then, it's a bit tricky. :)

Wouldn't it be better to to this as:

[remote "origin"]
	url = git.kernel.org:/pub/scm/git/git.git/
[branch "master"]
	remote = origin
	merge = refs/heads/master

If you're not tracking, the only thing you do with remote refs is merge them (after getting an anonymous temporary copy, of course), which should be the "merge" line, not the "fetch" line. Furthermore, you certainly don't want to merge refs/heads/master if you fetch (or pull) when you have todo (or a branch derived from it) checked out.

Merging without tracking is fine, in general; the "without tracking" was intentionally parenthetical. I just don't think always merging a particular remote head into the current branch, regardless of what branch is current, is a good idea.

	-Daniel
*This .sig left intentionally blank*

Daniel Barkalow <barkalow@iabervon.org> writes:

As I am likely to pull from other people, and I happen to expect the old fashioned "git pull gfi" without explicit refspec on the command line to get the first Pull: line from remotes file to tell which one to merge from, I am in favor of _not_ having that [branch "master"] stuff in my repository.

remote.*.fetch is about what remote branches are fetched; tracking may or may not happen as a side effect. What I do with the resulting .git/FETCH_HEAD is my business and branch.*.merge should _not_ be the only way to access it.

Actually, fetching with or without tracking are both valid options, and it does not make much sense to say which one is norm and which one is exception.

Exactly. That's why I do not think [branch "master"] for toplevel maintainer usage is not very useful.

There are a few improvements we probably would want.

One is the interaction between the config "fetch =..." refspecs and the command line ones. Currently the rule is:

 - If you do not say refspecs on the command line, fetch config
   (or Pull: lines from remotes/* file) are used.

 - If you do have refspecs on the command line, fetch config are
   not used (they are ignored).

Which is fine when we talk about "git fetch", but if you _are_ using remote tracking, and if you are running "git fetch" as an implementation detail of running "git pull" (IOW, you said "git pull origin next"), it is less than optimum. Instead of "only fetch 'next' without tracking and then merge it", we would certainly want "fetch to track everything as usual, and then merge 'next' instead of what is usually merged".

Another thing is that which branch is merged into the current branch should not be a function of the current branch, as the current configuration mechanism suggests. The current way maps the current branch to "which repository's which branch". In addition to that, I think it should be a function of ("current branch", "remote repository") pair. IOW, allow you to say "If I pull from this remote without saying which one to merge, merge that branch. If I pull from this other remote, merge that other branch". Something like:

        [branch "master"]
                ; Use this when "git pull" did not say which remote
                remote = origin

                ; Use this when "git pull origin" did not say which
                ; branch(es) to merge
                merge = refs/heads/master
                ; the above should be a synonym for
		; merge = refs/heads/master for origin

                ; "git pull fast-import" while on my "master"
                ; would merge 'for-junio' branch there.
                merge = refs/heads/for-junio for fast-import

                ; similarly, but use subtree strategy.
                merge = refs/heads/for-junio with subtree for git-gui

        [remote "origin"]
                url = git://git.kernel.org/pub/scm/git/git.git/
                fetch = refs/heads/*:remotes/origin/*
        [remote "fast-import"]
                url = git://repo.or.cz/git/fastimport.git/
                fetch = refs/heads/*:remotes/gfi/*
        [remote "git-gui"]
                url = git://repo.or.cz/git-gui.git/
                fetch = refs/heads/*:refs/remotes/git-gui/*

On Sat, 5 May 2007, Junio C Hamano wrote:

Ah, okay. So FETCH_HEAD gets: (1) if something is specified on the command line, that; (2) if there is a branch.*.merge, that; (3) the first refspec (without a *?) listed in remote.*.fetch. Right?

I think that might be confusing; I'd expect that the tracking for next would be updated, but the tracking for other heads wouldn't. I'm not sure I'd ever actually complain about origin/master being more up-to-date than can be intuitively explained, but I don't think a "git pull origin next" would make me expect origin/master to be up-to-date right after. So I'd say, "only fetch 'next'; if you track 'next' normally, update that."

On the other hand, I've been doing a lot of "git diff origin all-my-work" and removing hunks I don't want to send, and this makes the implicit assumption that origin == merge_base(origin, all-my-work), which is true only because I merge origin whenever I fetch it.

I think this is a good feature, but maybe the syntax should be:

 merge = fast-import refs/heads/for-junio

since the remote name is user-chosen, arbitrary, and has to be typed on the command line, and therefore people won't put in spaces, and "git fetch fast-import refs/heads/for-junio" is what you'd type on the command line that this abbreviates, so it's intuitive.

	-Daniel
*This .sig left intentionally blank*

Yes, please!

Jdl