[PATCH 4/5] Add --remote option to send-pack

[PATCH 4/5] Add --remote option to send-pack

[Daniel Barkalow]

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

diff --git a/send-pack.c b/send-pack.c
index d5b5162..7f76ef0 100644
--- a/send-pack.c
+++ b/send-pack.c
@@ -4,6 +4,7 @@
 #include "refs.h"
 #include "pkt-line.h"
 #include "run-command.h"
+#include "remote.h"
 
 static const char send_pack_usage[] =
 "git-send-pack [--all] [--force] [--receive-pack=<git-receive-pack>] [--verbose] [--thin] [<host>:]<directory> [<ref>...]\n"
@@ -176,7 +177,7 @@ static int receive_status(int in)
 	return ret;
 }
 
-static int send_pack(int in, int out, int nr_refspec, char **refspec)
+static int send_pack(int in, int out, struct remote *remote, int nr_refspec, char **refspec)
 {
 	struct ref *ref;
 	int new_refs;
@@ -213,18 +214,18 @@ static int send_pack(int in, int out, int nr_refspec, char **refspec)
 	new_refs = 0;
 	for (ref = remote_refs; ref; ref = ref->next) {
 		char old_hex[60], *new_hex;
-		int delete_ref;
+		int will_delete_ref;
 
 		if (!ref->peer_ref)
 			continue;
 
-		delete_ref = is_null_sha1(ref->peer_ref->new_sha1);
-		if (delete_ref && !allow_deleting_refs) {
+		will_delete_ref = is_null_sha1(ref->peer_ref->new_sha1);
+		if (will_delete_ref && !allow_deleting_refs) {
 			error("remote does not support deleting refs");
 			ret = -2;
 			continue;
 		}
-		if (!delete_ref &&
+		if (!will_delete_ref &&
 		    !hashcmp(ref->old_sha1, ref->peer_ref->new_sha1)) {
 			if (verbose)
 				fprintf(stderr, "'%s': up-to-date\n", ref->name);
@@ -251,7 +252,7 @@ static int send_pack(int in, int out, int nr_refspec, char **refspec)
 		 */
 
 		if (!force_update &&
-		    !delete_ref &&
+		    !will_delete_ref &&
 		    !is_null_sha1(ref->old_sha1) &&
 		    !ref->force) {
 			if (!has_sha1_file(ref->old_sha1) ||
@@ -275,7 +276,7 @@ static int send_pack(int in, int out, int nr_refspec, char **refspec)
 			}
 		}
 		hashcpy(ref->new_sha1, ref->peer_ref->new_sha1);
-		if (!delete_ref)
+		if (!will_delete_ref)
 			new_refs++;
 		strcpy(old_hex, sha1_to_hex(ref->old_sha1));
 		new_hex = sha1_to_hex(ref->new_sha1);
@@ -290,7 +291,7 @@ static int send_pack(int in, int out, int nr_refspec, char **refspec)
 		else
 			packet_write(out, "%s %s %s",
 				     old_hex, new_hex, ref->name);
-		if (delete_ref)
+		if (will_delete_ref)
 			fprintf(stderr, "deleting '%s'\n", ref->name);
 		else {
 			fprintf(stderr, "updating '%s'", ref->name);
@@ -300,6 +301,27 @@ static int send_pack(int in, int out, int nr_refspec, char **refspec)
 			fprintf(stderr, "\n  from %s\n  to   %s\n",
 				old_hex, new_hex);
 		}
+		if (remote) {
+			char *track =
+				remote_fetch_to(remote, ref->name);
+			if (track) {
+				struct ref_lock *lock;
+				fprintf(stderr, " Also local %s\n", track);
+				if (will_delete_ref) {
+					if (delete_ref(track, NULL)) {
+						error("Failed to delete");
+					}
+				} else {
+					lock = lock_any_ref_for_update(track, NULL);
+					if (!lock)
+						error("Failed to lock");
+					else
+						write_ref_sha1(lock, ref->new_sha1,
+							       "update by push");
+				}
+				free(track);
+			}
+		}
 	}
 
 	packet_flush(out);
@@ -344,6 +366,8 @@ int main(int argc, char **argv)
 	char **heads = NULL;
 	int fd[2], ret;
 	pid_t pid;
+	char *remote_name = NULL;
+	struct remote *remote = NULL;
 
 	setup_git_directory();
 	git_config(git_default_config);
@@ -361,6 +385,10 @@ int main(int argc, char **argv)
 				receivepack = arg + 7;
 				continue;
 			}
+			if (!prefixcmp(arg, "--remote=")) {
+				remote_name = arg + 9;
+				continue;
+			}
 			if (!strcmp(arg, "--all")) {
 				send_all = 1;
 				continue;
@@ -393,10 +421,18 @@ int main(int argc, char **argv)
 		usage(send_pack_usage);
 	verify_remote_names(nr_heads, heads);
 
+	if (remote_name) {
+		remote = remote_get(remote_name);
+		if (!remote_has_uri(remote, dest)) {
+			die("Destination %s is not a uri for %s",
+			    dest, remote_name);
+		}
+	}
+
 	pid = git_connect(fd, dest, receivepack);
 	if (pid < 0)
 		return 1;
-	ret = send_pack(fd[0], fd[1], nr_heads, heads);
+	ret = send_pack(fd[0], fd[1], remote, nr_heads, heads);
 	close(fd[0]);
 	close(fd[1]);
 	ret |= finish_connect(pid);
-- 
1.5.1.2.255.g6ead4-dirty

Re: [PATCH 4/5] Add --remote option to send-pack

[Junio C Hamano]

Daniel Barkalow <barkalow@iabervon.org> writes:

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

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.

Re: [PATCH 4/5] Add --remote option to send-pack

[Daniel Barkalow]

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

> Daniel Barkalow <barkalow@iabervon.org> writes:
> 
> > 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.
> 
> 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.

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*

Re: [PATCH 4/5] Add --remote option to send-pack

[Shawn O. Pearce]

Daniel Barkalow <barkalow@iabervon.org> wrote:
> 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.

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.

-- 
Shawn.

Re: [PATCH 4/5] Add --remote option to send-pack

[Junio C Hamano]

Daniel Barkalow <barkalow@iabervon.org> writes:

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

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.

Re: [PATCH 4/5] Add --remote option to send-pack

[Daniel Barkalow]

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

> Daniel Barkalow <barkalow@iabervon.org> writes:
> 
> > 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.
> >
> 
> 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 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*

Re: [PATCH 4/5] Add --remote option to send-pack

[Junio C Hamano]

Daniel Barkalow <barkalow@iabervon.org> writes:

> On Sat, 28 Apr 2007, Junio C Hamano wrote:
>
>> 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 you did effectively re-fetch instantaneously by doing an operation 
> that atomicly updates the ref and reports success.

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

Re: [PATCH 4/5] Add --remote option to send-pack

[Daniel Barkalow]

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

> Daniel Barkalow <barkalow@iabervon.org> writes:
> 
> > On Sat, 28 Apr 2007, Junio C Hamano wrote:
> >
> >> 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 you did effectively re-fetch instantaneously by doing an operation 
> > that atomicly updates the ref and reports success.
> 
> 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.

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

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

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*

Re: [PATCH 4/5] Add --remote option to send-pack

[Junio C Hamano]

Daniel Barkalow <barkalow@iabervon.org> writes:

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

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.

Re: [PATCH 4/5] Add --remote option to send-pack

[Daniel Barkalow]

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

> Daniel Barkalow <barkalow@iabervon.org> writes:
> 
> > 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.
> 
> 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.

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*

Re: [PATCH 4/5] Add --remote option to send-pack

[Junio C Hamano]

Daniel Barkalow <barkalow@iabervon.org> writes:

> Is this actually supported in config files? At least for pulls, the "tag 
> <tag>" notation is only available on the command line, afaict.

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.

> 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?

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.

Re: [PATCH 4/5] Add --remote option to send-pack

[Daniel Barkalow]

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

> Daniel Barkalow <barkalow@iabervon.org> writes:
> 
> > Is this actually supported in config files? At least for pulls, the "tag 
> > <tag>" notation is only available on the command line, afaict.
> 
> 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.

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

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

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*

Re: [PATCH 4/5] Add --remote option to send-pack

[Junio C Hamano]

Daniel Barkalow <barkalow@iabervon.org> writes:

> 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

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.

> Merging without tracking is fine, in general; the "without tracking" was 
> intentionally parenthetical. 

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.

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

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/*

Re: [PATCH 4/5] Add --remote option to send-pack

[Daniel Barkalow]

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

> Daniel Barkalow <barkalow@iabervon.org> writes:
> 
> > 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
> 
> 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.

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?

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

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.

> 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/*

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*

RE: [PATCH 4/5] Add --remote option to send-pack

[Loeliger Jon-LOELIGER]

> 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/*
>

Yes, please!

Jdl