[PATCH] Make git-clone --use-separate-remote the default

[PATCH] Make git-clone --use-separate-remote the default

[Petr Baudis]

and --use-immingled-remote can be used to get the original behaviour;
it is also implied by --bare.

We get confused, frustrated and data-losing users *daily* on #git now
because git-clone still produces the crippled repositories having the
remote and local heads freely mixed together.

Signed-off-by: Petr Baudis <pasky@suse.cz>
---

 Documentation/git-clone.txt |   20 ++++++++++++++------
 git-clone.sh                |   14 +++++++-------
 2 files changed, 21 insertions(+), 13 deletions(-)

diff --git a/Documentation/git-clone.txt b/Documentation/git-clone.txt
index 8606047..b1ad79f 100644
--- a/Documentation/git-clone.txt
+++ b/Documentation/git-clone.txt
@@ -11,7 +11,8 @@ SYNOPSIS
 [verse]
 'git-clone' [--template=<template_directory>] [-l [-s]] [-q] [-n] [--bare]
 	  [-o <name>] [-u <upload-pack>] [--reference <repository>]
-	  [--use-separate-remote] <repository> [<directory>]
+	  [--use-separate-remote | --use-immingled-remote] <repository>
+	  [<directory>]
 
 DESCRIPTION
 -----------
@@ -71,9 +72,10 @@ OPTIONS
 	Make a 'bare' GIT repository.  That is, instead of
 	creating `<directory>` and placing the administrative
 	files in `<directory>/.git`, make the `<directory>`
-	itself the `$GIT_DIR`. This implies `-n` option.  When
-	this option is used, neither the `origin` branch nor the
-	default `remotes/origin` file is created.
+	itself the `$GIT_DIR`. This implies the `-n` and
+	`--use-immingled-remote' option.  When this option is used,
+	neither the `origin` branch nor the default `remotes/origin`
+	file is created.
 
 --origin <name>::
 -o <name>::
@@ -97,8 +99,14 @@ OPTIONS
 
 --use-separate-remote::
 	Save remotes heads under `$GIT_DIR/remotes/origin/` instead
-	of `$GIT_DIR/refs/heads/`.  Only the master branch is saved
-	in the latter.
+	of `$GIT_DIR/refs/heads/`.  Only the local master branch is
+	saved in the latter. This is the default.
+
+--use-immingled-remote::
+	Save remotes heads in the same namespace as the local heads,
+	`$GIT_DIR/refs/heads/'.  In regular repositories, this is
+	a legacy setup git-clone created by default in older Git
+	versions.  It is also still implied by `--bare'.
 
 <repository>::
 	The (possibly remote) repository to clone from.  It can
diff --git a/git-clone.sh b/git-clone.sh
index 3f006d1..9ed4135 100755
--- a/git-clone.sh
+++ b/git-clone.sh
@@ -14,7 +14,7 @@ die() {
 }
 
 usage() {
-	die "Usage: $0 [--template=<template_directory>] [--use-separate-remote] [--reference <reference-repo>] [--bare] [-l [-s]] [-q] [-u <upload-pack>] [--origin <name>] [-n] <repo> [<dir>]"
+	die "Usage: $0 [--template=<template_directory>] [--use-immingled-remote] [--reference <reference-repo>] [--bare] [-l [-s]] [-q] [-u <upload-pack>] [--origin <name>] [-n] <repo> [<dir>]"
 }
 
 get_repo_base() {
@@ -115,7 +115,7 @@ bare=
 reference=
 origin=
 origin_override=
-use_separate_remote=
+use_separate_remote=t
 while
 	case "$#,$1" in
 	0,*) break ;;
@@ -134,7 +134,10 @@ while
 	  template="$1" ;;
 	*,-q|*,--quiet) quiet=-q ;;
 	*,--use-separate-remote)
+		# default
 		use_separate_remote=t ;;
+	*,--use-immingled-remote)
+		use_separate_remote= ;;
 	1,--reference) usage ;;
 	*,--reference)
 		shift; reference="$1" ;;
@@ -169,18 +172,15 @@ repo="$1"
 test -n "$repo" ||
     die 'you must specify a repository to clone.'
 
-# --bare implies --no-checkout
+# --bare implies --no-checkout and --use-immingled-remote
 if test yes = "$bare"
 then
 	if test yes = "$origin_override"
 	then
 		die '--bare and --origin $origin options are incompatible.'
 	fi
-	if test t = "$use_separate_remote"
-	then
-		die '--bare and --use-separate-remote options are incompatible.'
-	fi
 	no_checkout=yes
+	use_separate_remote=
 fi
 

Re: [PATCH] Make git-clone --use-separate-remote the default

[Junio C Hamano]

Petr Baudis <pasky@suse.cz> writes:

> and --use-immingled-remote can be used to get the original behaviour;
> it is also implied by --bare.

What's immingled?

> We get confused, frustrated and data-losing users *daily* on #git now
> because git-clone still produces the crippled repositories having the
> remote and local heads freely mixed together.
>
> Signed-off-by: Petr Baudis <pasky@suse.cz>

Being strongly opinionated, not giving enough credit for the
evolutionary process behind the history and venting frustration
in the proposed commit log message is never a good strategy to
get the patch applied.

Even though I fully agree that use-separate-remotes should be
the default, to the point that I do not think we do not even
need a backward compatibility option.  People who want to use
traditional layout for simple one-remote-branch-only project
would not suffer anyway because 'origin' still means origin in
the new layout (refs/remotes/origin/HEAD).

We would need to update the tutorials to match this,though.  I
think it talks about the traditional layout and say 'See, now
you can run "ls .git/refs/heads/{master,origin}"' or something
like that.

Re: [PATCH] Make git-clone --use-separate-remote the default

[Andy Whitcroft]

Junio C Hamano wrote:
> Petr Baudis <pasky@suse.cz> writes:
> 
>> and --use-immingled-remote can be used to get the original behaviour;
>> it is also implied by --bare.
> 
> What's immingled?

Had me reaching for the dictionary too.  Seems to mean the same as
intermingled which would be my choice ...

Re: [PATCH] Make git-clone --use-separate-remote the default

[Petr Baudis]

On Fri, Nov 24, 2006 at 12:12:10AM CET, Junio C Hamano wrote:
> Petr Baudis <pasky@suse.cz> writes:
> 
> > and --use-immingled-remote can be used to get the original behaviour;
> > it is also implied by --bare.
> 
> What's immingled?

One dictionary says

   Immingle \Im*min"gle\, v. t.
      To mingle; to mix; to unite; to blend. [R.] --Thomson.

but perhaps it's too much an obscure word... better suggestions
welcomed.

> > We get confused, frustrated and data-losing users *daily* on #git now
> > because git-clone still produces the crippled repositories having the
> > remote and local heads freely mixed together.
> >
> > Signed-off-by: Petr Baudis <pasky@suse.cz>
> 
> Being strongly opinionated, not giving enough credit for the
> evolutionary process behind the history and venting frustration
> in the proposed commit log message is never a good strategy to
> get the patch applied.

Yes, sorry, the last days were a bit tiring to me.

I'm not sure what evolutionary process should I describe, though...

> Even though I fully agree that use-separate-remotes should be
> the default, to the point that I do not think we do not even
> need a backward compatibility option.  People who want to use
> traditional layout for simple one-remote-branch-only project
> would not suffer anyway because 'origin' still means origin in
> the new layout (refs/remotes/origin/HEAD).

I don't know, we still at least need to keep the functionality for
--bare.

> We would need to update the tutorials to match this,though.  I
> think it talks about the traditional layout and say 'See, now
> you can run "ls .git/refs/heads/{master,origin}"' or something
> like that.

Oops, yes. I can try to go through the tutorials during tomorrow or the
next week...

-- 
				Petr "Pasky" Baudis
Stuff: http://pasky.or.cz/
The meaning of Stonehenge in Traflamadorian, when viewed from above, is:
"Replacement part being rushed with all possible speed."

Re: [PATCH] Make git-clone --use-separate-remote the default

[J. Bruce Fields]

On Fri, Nov 24, 2006 at 12:42:03AM +0100, Petr Baudis wrote:
> On Fri, Nov 24, 2006 at 12:12:10AM CET, Junio C Hamano wrote:
> > Petr Baudis <pasky@suse.cz> writes:
> > 
> > > and --use-immingled-remote can be used to get the original behaviour;
> > > it is also implied by --bare.
> > 
> > What's immingled?
> 
> One dictionary says
> 
>    Immingle \Im*min"gle\, v. t.
>       To mingle; to mix; to unite; to blend. [R.] --Thomson.
> 
> but perhaps it's too much an obscure word... better suggestions
> welcomed.

commingled? legacy? flat?

Re: [PATCH] Make git-clone --use-separate-remote the default

[Junio C Hamano]

Petr Baudis <pasky@suse.cz> writes:

>> Even though I fully agree that use-separate-remotes should be
>> the default, to the point that I think we do not even
>> need a backward compatibility option.  People who want to use
>> traditional layout for simple one-remote-branch-only project
>> would not suffer anyway because 'origin' still means origin in
>> the new layout (refs/remotes/origin/HEAD).
>
> I don't know, we still at least need to keep the functionality for
> --bare.

I agree --bare should continue to be a "snapshot mirror"; I am
not advocating for the removal of the internal implementation
detail such as $use_separate_remote variable.

However, I think having one sane behaviour is the right thing to
do for a clone that prepares a repository with a working tree
(including the one made with -n option, which only means "do not
do the check-out immediately after cloning" for such a
repository).

The traditional layout is slightly simpler for a project with
the simplest needs (that is, a single upstream repository that
has a single 'master' branch), but I do think even that is not
an advantage anymore.

With the separate-remote layout, git-fetch would still fetch and
update the "origin" (although that is now remotes/origin/master
which is pointed at by remotes/origin/HEAD) and the user can
still refer to it with "origin".  Commands "git-pull origin",
"git-pull . origin", and "git-merge origin" all will continue to
work the same way as before for such a project as in the
traditional layout, and that is why I think we do not need
backward compatibility flag in this case.

Re: [PATCH] Make git-clone --use-separate-remote the default

[Junio C Hamano]

Junio C Hamano <junkio@cox.net> writes:

> I agree --bare should continue to be a "snapshot mirror"; I am
> not advocating for the removal of the internal implementation
> detail such as $use_separate_remote variable.
>
> However, I think having one sane behaviour is the right thing to
> do for a clone that prepares a repository with a working tree
> (including the one made with -n option, which only means "do not
> do the check-out immediately after cloning" for such a
> repository).

Just to let you know, I'll take the patch almost as is (even
with the --use-immingled-remote), except with a slight rewording
in the documentation to warn people that the backward
compatibility option will be removed before the next major
release.

However, this simple command fails:

	$ git push $URL master

if the target repository $URL is made with use-separate-remote.

This is because 'master' matches more than one on the remote
side (heads/master and remotes/origin/master) which triggers
"Hey, that's ambiguous, make yourself clear which one you mean!"
check.  This breaks t5400 test.  We could "fix" the test to make
it more explicit, but that is just a workaround.

I think the send-pack/receive-pack pair needs to be taught that
an unadorned branch name 'master' never matches anything under
refs/remotes. This means that it would require an explicit
refspec heads/master:remotes/origin/master in order to pudate
refs under refs/remotes on the remote side with a push.  I do
not think that is a big problem, because the normal patch-flow
for shared repository workflow is:

	remote			local

		      (fecth)
	heads/master	--->	remotes/origin/master ---.
							 | (merge)
	heads/master	<---	heads/master	      <--'

and pushing into remotes/origin/* is not a norm.

The function to fix is connect.c::match_explicit_refs() and I
_think_ making connect.c::count_refspec_match() not to consider
'foo' to match 'refs/remotes/origin/foo' (but still keeping it
to match 'refs/heads/foo' or 'refs/tags/foo') is enough to make
this happen.

This brings up two related issues.  Currently we automatically
prepare "Pull: refs/heads/$branch:refs/remotes/origin/$branch"
for all branches that exists at the remote site when a clone
happens.  Andy Parkins has a patch to allow a glob pattern to be
there, like this [*1*]:

	Pull: refs/heads/*:refs/remotes/origin/*

which makes sense, and we might want to have this as the default
after the clone [*2*].

Another is if we might want to add "Push: " entry in the default
after the clone.  I am a bit reluctant to make the default setup
too specific to CVS style "central shared repo" workflow, but
any stupid default would not suit people with truly distributed
workflow anyway, so it might be fine.

[Footnotes]

*1* I rewrote the patch because I wanted to deal with the
    fallout from recent packed-refs work at the same time.  So bugs
    in the counter-proposal patch is mine while the credit for the
    initiative and the idea goes to Andy.

*2* I think the fetch wildcarding has an issue with what remote
    head to merge when used with "git pull".  I think it should
    use the one that is pointed at by refs/remotes/origin/HEAD,
    but there is no code for that yet.  Hints, hints...

Re: [PATCH] Make git-clone --use-separate-remote the default

[Junio C Hamano]

Junio C Hamano <junkio@cox.net> writes:

> However, this simple command fails:
>
> 	$ git push $URL master
>
> if the target repository $URL is made with use-separate-remote.
>
> This is because 'master' matches more than one on the remote
> side (heads/master and remotes/origin/master) which triggers
> "Hey, that's ambiguous, make yourself clear which one you mean!"
> check.  This breaks t5400 test.  We could "fix" the test to make
> it more explicit, but that is just a workaround.
>
> I think the send-pack/receive-pack pair needs to be taught that
> an unadorned branch name 'master' never matches anything under
> refs/remotes. This means that it would require an explicit
> refspec heads/master:remotes/origin/master in order to pudate
> refs under refs/remotes on the remote side with a push.
> ...
> The function to fix is connect.c::match_explicit_refs() and I
> _think_ making connect.c::count_refspec_match() not to consider
> 'foo' to match 'refs/remotes/origin/foo' (but still keeping it
> to match 'refs/heads/foo' or 'refs/tags/foo') is enough to make
> this happen.

That is,...

-- >8 --
[PATCH] refs outside refs/{heads,tags} match less strongly.

This changes the refname matching logic used to decide which ref
is updated with git-send-pack.  We used to error out when
pushing 'master' when the other end has both 'master' branch and
a tracking branch 'remotes/$name/master' but with this, 'master'
matches only 'refs/heads/master' when both and no other 'master'
exist.

Pushing 'foo' when both heads/foo and tags/foo exist at the
remote end is still considered an error and you would need to
disambiguate between them by being more explicit.

When neither heads/foo nor tags/foo exists at the remote,
pushing 'foo' when there is only remotes/origin/foo is not
ambiguous, while it still is ambiguous when there are more than
one such weaker match (remotes/origin/foo and remotes/alt/foo,
for example).

Signed-off-by: Junio C Hamano <junkio@cox.net>

---

diff --git a/connect.c b/connect.c
index c55a20a..b9666cc 100644
--- a/connect.c
+++ b/connect.c
@@ -174,21 +174,58 @@ static int count_refspec_match(const cha
 			       struct ref *refs,
 			       struct ref **matched_ref)
 {
-	int match;
 	int patlen = strlen(pattern);
+	struct ref *matched_weak = NULL;
+	struct ref *matched = NULL;
+	int weak_match = 0;
+	int match = 0;
 
-	for (match = 0; refs; refs = refs->next) {
+	for (weak_match = match = 0; refs; refs = refs->next) {
 		char *name = refs->name;
 		int namelen = strlen(name);
+		int weak_match;
+
 		if (namelen < patlen ||
 		    memcmp(name + namelen - patlen, pattern, patlen))
 			continue;
 		if (namelen != patlen && name[namelen - patlen - 1] != '/')
 			continue;
-		match++;
-		*matched_ref = refs;
+
+		/* A match is "weak" if it is with refs outside
+		 * heads or tags, and did not specify the pattern
+		 * in full (e.g. "refs/remotes/origin/master") or at
+		 * least from the toplevel (e.g. "remotes/origin/master");
+		 * otherwise "git push $URL master" would result in
+		 * ambiguity between remotes/origin/master and heads/master
+		 * at the remote site.
+		 */
+		if (namelen != patlen &&
+		    patlen != namelen - 5 &&
+		    strncmp(name, "refs/heads/", 11) &&
+		    strncmp(name, "refs/tags/", 10)) {
+			/* We want to catch the case where only weak
+			 * matches are found and there are multiple
+			 * matches, and where more than one strong
+			 * matches are found, as ambiguous.  One
+			 * strong match with zero or more weak matches
+			 * are acceptable as a unique match.
+			 */
+			matched_weak = refs;
+			weak_match++;
+		}
+		else {
+			matched = refs;
+			match++;
+		}
+	}
+	if (!matched) {
+		*matched_ref = matched_weak;
+		return weak_match;
+	}
+	else {
+		*matched_ref = matched;
+		return match;
 	}
-	return match;
 }
 
 static void link_dst_tail(struct ref *ref, struct ref ***tail)

Re: [PATCH] Make git-clone --use-separate-remote the default

[Jakub Narebski]

Junio C Hamano wrote:

> Petr Baudis <pasky@suse.cz> writes:
> 
>>> Even though I fully agree that use-separate-remotes should be
>>> the default, to the point that I think we do not even
>>> need a backward compatibility option.  People who want to use
>>> traditional layout for simple one-remote-branch-only project
>>> would not suffer anyway because 'origin' still means origin in
>>> the new layout (refs/remotes/origin/HEAD).
>>
>> I don't know, we still at least need to keep the functionality for
>> --bare.

By the way, I think the backward compatibility option should be
simply named --dont-use-separate-remote, or --without-separate-remote,
or --no-separate-remote (the last is probably the best choice).

> I agree --bare should continue to be a "snapshot mirror"; I am
> not advocating for the removal of the internal implementation
> detail such as $use_separate_remote variable.
> 
> However, I think having one sane behaviour is the right thing to
> do for a clone that prepares a repository with a working tree
> (including the one made with -n option, which only means "do not
> do the check-out immediately after cloning" for such a
> repository).
> 
> The traditional layout is slightly simpler for a project with
> the simplest needs (that is, a single upstream repository that
> has a single 'master' branch), but I do think even that is not
> an advantage anymore.
> 
> With the separate-remote layout, git-fetch would still fetch and
> update the "origin" (although that is now remotes/origin/master
> which is pointed at by remotes/origin/HEAD) and the user can
> still refer to it with "origin".  Commands "git-pull origin",
> "git-pull . origin", and "git-merge origin" all will continue to
> work the same way as before for such a project as in the
> traditional layout, and that is why I think we do not need
> backward compatibility flag in this case.
 
The exception being that with --use-separate-remote you cannot checkout
tracking branches to see what it is there (at least for now, but IIRC we
want to relax this constraint; i.e. to forbid commiting to non-heads,
instead of forbidding checking out), you cannot use it as alternate
source (as alternate repo to check from) while still allowing to work
on it, and that gitweb doesn't show anything except heads and tags;
it doesn't show remotes.

By the way, does new "git peek-remote -a ." show anything except
refs/heads/, refs/tags/ and refs/remotes (e.g. StGit refs/bases/
and refs/patches/)?
-- 
Jakub Narebski
Warsaw, Poland
ShadeHawk on #git

Re: [PATCH] Make git-clone --use-separate-remote the default

[Salikh Zakirov]

Petr Baudis wrote:
> --- a/Documentation/git-clone.txt
> +++ b/Documentation/git-clone.txt
> ...
>  --use-separate-remote::
>  	Save remotes heads under `$GIT_DIR/remotes/origin/` instead
> -	of `$GIT_DIR/refs/heads/`.  Only the master branch is saved
> -	in the latter.

This description does not apply to repositories which do not have 'master' branch.
Maybe "only the HEAD branch of remote repository, where
HEAD is the branch designated as main branch in repository".

Re: [PATCH] Make git-clone --use-separate-remote the default

[Salikh Zakirov]

Junio C Hamano wrote:
> -- >8 --
> [PATCH] refs outside refs/{heads,tags} match less strongly.
> 
> Pushing 'foo' when both heads/foo and tags/foo exist at the
> remote end is still considered an error and you would need to
> disambiguate between them by being more explicit.
> 
> When neither heads/foo nor tags/foo exists at the remote,
> pushing 'foo' when there is only remotes/origin/foo is not
> ambiguous, while it still is ambiguous when there are more than
> one such weaker match (remotes/origin/foo and remotes/alt/foo,
> for example).

git-push.1 has following description:

    Some short-cut notations are also supported.

              o   tag <tag> means the same as refs/tags/<tag>:refs/tags/<tag>.

              o  A parameter <ref> without a colon is equivalent to
                 <ref>:<ref>, hence updates <ref>  in
                 the destination from <ref> in the source.

Maybe this is only my reading of manual page, but I understood
it like it does not leave the room for ambiguity, because it is using
_the same_ refspec as the local one.

That's why, when I do

   git-push repo x

and it results in

   git-push repo refs/heads/x:refs/remotes/origin/x

instead of expected

   git-push repo refs/heads/x:refs/heads/x

just because the remote repo did not have refs/heads/x, but happened
to have refs/remotes/origin/x, would be highly surprising to me.

The expected behaviour on 'git-push repo x' in my understanding is
1) git finds the exact reference for 'x' (i.e. either refs/heads/x or
refs/tags/x) according to local lookup rules
2) git uses the found reference _unambiguously_ to create or update exactly the
same reference in the remote repo.

Am I the only one to have this understanding?

Re: [PATCH] Make git-clone --use-separate-remote the default

[Junio C Hamano]

Salikh Zakirov <Salikh.Zakirov@Intel.com> writes:

> git-push.1 has following description:
>
>     Some short-cut notations are also supported.
>
>               o   tag <tag> means the same as refs/tags/<tag>:refs/tags/<tag>.
>
>               o  A parameter <ref> without a colon is equivalent to
>                  <ref>:<ref>, hence updates <ref>  in
>                  the destination from <ref> in the source.
>
> Maybe this is only my reading of manual page, but I understood
> it like it does not leave the room for ambiguity, because it is using
> _the same_ refspec as the local one.

If you write

	git push $remote tag $string

it is handled exactly as if you wrote:

	git push $remote refs/tags/${string}:refs/tags/${string}

and if you write

	git push $remote master

it is handled exactly as if you wrote:

	git push $remote master:master

The manual correctly describes the above, but the issue the fix
addresses is about what happens to that 'master' string that
follows the colon, and the 'master' string becomes ambiguous if
the remote end uses separate-remote layout.

The way this command:

	git push $remote $src:$dst

is handled is:

 (0) send-pack gets ls-remote equivalent from the remote.  This
     tells us the set of refs the remote has and the value of
     each of them.

 (1) $src can be a ref that is resolved locally the usual way.
     You could have any valid SHA-1 expression (e.g. HEAD~6).

 (2) $dst is compared with the list of refs that the remote
     has, and unique match is found.  So if the set of refs the
     remote side has:

	refs/heads/origin
        refs/heads/master
        refs/tags/v1.0.0

     and if $dst is 'master', refs/heads/master is what will be
     updated.

 (3) Then send-pack generates and sends the necessary pack to
     update the remote side with objects needed for $src, using
     the knowledge of what the remote has.  Also, send-pack
     instructs the remote to update which ref with what value.
     Continuing with the example, it tells the remote to update
     its refs/heads/master with the value of our 'master'.

That *matching* in step (2) is what the fix is about.

The matching code of send-pack from the beginning has been the
unique tail-match.  When the other end had a branch 'bugfix' and
a tag 'bugfix', then both of them would match because both
refs/heads/bugfix and refs/tags/bugfix ends with 'bugfix'
('gfix' does not match 'refs/heads/bugfix' -- we are not that
stupid ;-).

So you had to disambiguate this case by saying heads/bugfix if
you want to push the branch.  That was fine between branches and
tags, since having a branch and a tag with the same name is
usually not done in order to keep user's sanity.

However, separate-remote layout poses a more serious problem,
because most of the time you would expect to see similar names
under refs/heads/ and refs/remotes/origin/ directories.  If we
kept the original ref matching code, a cloned remote would have
both refs/heads/master and refs/remotes/origin/master almost
always, so somebody who is pushing 'master' to such a remote
would have had to disambiguate it by saying:

	git push heads/master

which is (as described in the part of the manual you quoted) a
shorthand for

	git push heads/master:heads/master

'heads/master' before the colon is used to find out which commit
in your local repository we are pushing, and 'heads/master'
after the colon is used to match against the list of refs from
the remote (which contains both 'refs/heads/master' and
'refs/remotes/origin/master'), and only because the user said
'heads/master' (not just 'master') this avoids ambiguity.

Even under separate-remote layout, we would want to be able to
say:

	git push master

to mean we want to push to remote's heads/master when the remote
has remotes/{origin,blech}/master.

And that is what the fix is about.

Re: [PATCH] Make git-clone --use-separate-remote the default

[Sergey Vlasov]

[-- Attachment #1: Type: text/plain, Size: 4264 bytes --]

On Fri, 24 Nov 2006 13:14:00 +0300 Salikh Zakirov wrote:

> git-push.1 has following description:
>
>     Some short-cut notations are also supported.
>
>               o   tag <tag> means the same as refs/tags/<tag>:refs/tags/<tag>.

BTW, this is broken (and was broken even in 1.4.3.x):

$ mkdir ~/tmp/test_repo
$ ( cd ~/tmp/test_repo; git-init-db )
defaulting to local storage area
$ git push ~/tmp/test_repo tag v1.4.4.1
error: src refspec tag does not match any.
error: dst refspec tag does not match any existing ref on the remote and does not start with refs/.
fatal: unexpected EOF

Omitting the "tag" word works:

$ git push ~/tmp/test_repo v1.4.4.1
updating 'refs/tags/v1.4.4.1'
  from 0000000000000000000000000000000000000000
  to   21dff5f4982333d694d105595a701540d4d0d1db
Generating pack...
Done counting 28130 objects.
Deltifying 28130 objects.
 100% (28130/28130) done
Writing 28130 objects.
 100% (28130/28130) done
Total 28130, written 28130 (delta 19344), reused 27628 (delta 18891)
refs/tags/v1.4.4.1: 0000000000000000000000000000000000000000 -> 21dff5f4982333d694d105595a701540d4d0d1db

Seems that nobody really uses the "tag NAME" syntax...

>               o  A parameter <ref> without a colon is equivalent to
>                  <ref>:<ref>, hence updates <ref>  in
>                  the destination from <ref> in the source.
>
> Maybe this is only my reading of manual page, but I understood
> it like it does not leave the room for ambiguity, because it is using
> _the same_ refspec as the local one.
>
> That's why, when I do
>
>    git-push repo x
>
> and it results in
>
>    git-push repo refs/heads/x:refs/remotes/origin/x
>
> instead of expected
>
>    git-push repo refs/heads/x:refs/heads/x
>
> just because the remote repo did not have refs/heads/x, but happened
> to have refs/remotes/origin/x, would be highly surprising to me.

Such interpretation would indeed be horrible, but I'm afraid this is
exactly the case now:

$ mkdir ~/tmp/test_repo
$ ( cd ~/tmp/test_repo; git-init-db )
defaulting to local storage area
$ git push ~/tmp/test_repo v1.4.0^0:refs/remotes/origin/master
updating 'refs/remotes/origin/master' using 'v1.4.0^0'
  from 0000000000000000000000000000000000000000
  to   41292ddd37202ff6dce34986c87a6000c5d3fbfa
Generating pack...
Done counting 19857 objects.
Deltifying 19857 objects.
 100% (19857/19857) done
Writing 19857 objects.
 100% (19857/19857) done
Total 19857, written 19857 (delta 13472), reused 19038 (delta 12884)
refs/remotes/origin/master: 0000000000000000000000000000000000000000 -> 41292ddd37202ff6dce34986c87a6000c5d3fbfa

$ git push ~/tmp/test_repo master
updating 'refs/remotes/origin/master' using 'refs/heads/master'
  from 41292ddd37202ff6dce34986c87a6000c5d3fbfa
  to   e945f95157c2c515e763ade874931fc1eb671a0b
Generating pack...
Done counting 8667 objects.
Result has 8278 objects.
Deltifying 8278 objects.
 100% (8278/8278) done
Writing 8278 objects.
 100% (8278/8278) done
Total 8278, written 8278 (delta 5924), reused 7396 (delta 5065)
refs/remotes/origin/master: 41292ddd37202ff6dce34986c87a6000c5d3fbfa -> e945f95157c2c515e763ade874931fc1eb671a0b

BTW, I cannot find the description of the matching algorithm used by
connect.c:count_refspec_match() anywhere in the git-push or git-fetch
man page, and I cannot understand why this algorithm is different from
the default search order ($name, refs/$name, refs/tags/$name,
refs/heads/$name, refs/remotes/$name, refs/remotes/$name/HEAD).

> The expected behaviour on 'git-push repo x' in my understanding is
> 1) git finds the exact reference for 'x' (i.e. either refs/heads/x or
> refs/tags/x) according to local lookup rules
> 2) git uses the found reference _unambiguously_ to create or update
> exactly the same reference in the remote repo.
>
> Am I the only one to have this understanding?

The problem is that "$x" and "$x:$x" would be not equivalent anymore,
unless we add a special case for "$x:$y" where $x == $y - hmm, but the
current code seems to have that special case:

			else if (!strcmp(rs[i].src, rs[i].dst) &&
				 matched_src) {
				/* pushing "master:master" when
				 * remote does not have master yet.
				 */

(but that code triggers only in case we did not find any matching ref in
the destination repo).

[-- Attachment #2: Type: application/pgp-signature, Size: 189 bytes --]

Re: [PATCH] Make git-clone --use-separate-remote the default

[Junio C Hamano]

Sergey Vlasov <vsu@altlinux.ru> writes:

> BTW, this is broken (and was broken even in 1.4.3.x):
>
> $ mkdir ~/tmp/test_repo
> $ ( cd ~/tmp/test_repo; git-init-db )
> defaulting to local storage area
> $ git push ~/tmp/test_repo tag v1.4.4.1
> error: src refspec tag does not match any.
> error: dst refspec tag does not match any existing ref on the remote and does not start with refs/.
> fatal: unexpected EOF
>
> Omitting the "tag" word works:

I think this was broken when git-push was made a built-in, and
the documentation was not updated.

I use only tags in vN.M.L.. format and Linus does so too, so
probably that was one of the reasons why this was not noticed
for quite some time.

Fixes welcome, preferably to the builtin-push.c not to the
documentation.

Re: [PATCH] Make git-clone --use-separate-remote the default

[Salikh Zakirov]

Junio C Hamano wrote:
> and if you write
> 
> 	git push $remote master
> 
> it is handled exactly as if you wrote:
> 
> 	git push $remote master:master
> 
> The manual correctly describes the above, but the issue the fix
> addresses is about what happens to that 'master' string that
> follows the colon, and the 'master' string becomes ambiguous if
> the remote end uses separate-remote layout.

Indeed, the manual describes it correctly.
My point is that this semantics fairly complex
and easy to understand incorrectly.

> Even under separate-remote layout, we would want to be able to
> say:
> 
> 	git push master
> 
> to mean we want to push to remote's heads/master when the remote
> has remotes/{origin,blech}/master.

I agree with your main point that 'git push master' should "just work"
for all existing and new repositories, however,
it is very confusing that 'git push master' can update something other than
refs/heads/master, depending on the refs existing in the remote repo.

Re: [PATCH] Make git-clone --use-separate-remote the default

[Salikh Zakirov]

Junio C Hamano wrote:
> The way this command:
> 
> 	git push $remote $src:$dst
> 
> is handled is:
> 
>  (0) send-pack gets ls-remote equivalent from the remote.  This
>      tells us the set of refs the remote has and the value of
>      each of them.
> 
>  (1) $src can be a ref that is resolved locally the usual way.
>      You could have any valid SHA-1 expression (e.g. HEAD~6).

>  (2) $dst is compared with the list of refs that the remote
>      has, and unique match is found.

I think that remote matching semantics is confusing, and the following change
would make understanding easier.

I was understanding the manual incorrectly for a long time until you've
explained its true meaning today (thanks!).

As a side effect, making 'git push repo master' unambiguously expanded
to 'git push repo refs/heads/master:refs/heads/master' will make
the syntax 'git push repo tag v1' unneeded at all, because it would be
exactly the same as 'git push repo v1'
(expanded to 'git push repo refs/tags/v1:refs/tags/v1').

--- connect.c
+++ connect.c
@@ -277,6 +277,16 @@ static int match_explicit_refs(struct re
                              rs[i].src);
                        break;
                }
+               if (!strcmp(rs[i].src,rs[i].dst)) {
+                       /* src refspec is the same as dst,
+                        * take the remote refpath exactly the same
+                        * as existing local reference
+                        */
+                       int len = strlen(matched_src->name) + 1;
+                       matched_dst = xcalloc(1, sizeof(*dst) + len);
+                       memcpy(matched_dst->name, matched_src->name, len);
+                       link_dst_tail(matched_dst, dst_tail);
+               } else
                switch (count_refspec_match(rs[i].dst, dst, &matched_dst)) {
                case 1:
                        break;

Re: [PATCH] Make git-clone --use-separate-remote the default

[Junio C Hamano]

Salikh Zakirov <salikh@gmail.com> writes:

> I think that remote matching semantics is confusing, and the following change
> would make understanding easier.

Hmm.  I think this is somewhat wrong.

Have you tested the patch with repositories with existing refs?

You do not seem to check if that fabricated matched_dst exists
on the other side, so matched_dst lacks "where was this ref
initially" information (aka old_sha1), if I am reading your
patch correctly.  Wouldn't that mean that you would confuse the
fast-forward check logic?

One setup I have that would be broken with this change is that
the remote end has refs/heads/up/obsd and no refs/heads/obsd,
and local end has refs/heads/obsd.  This is to work on
portability fix for OpenBSD.  With the current git-push, I think

	git push $remote_openbsd_box obsd

would correctly update the remote refs/heads/up/obsd with the
local tip of the obsd branch, so that then I can ssh into the
remote and say "git merge up/obsd" to continue on that OpenBSD
machine from where I left off on the local, non-OpenBSD machine.

I am not sure if people would mind breaking existing setups like
this.

By the way, there are other glitches with the current git-push
(rather, git-send-pack) that we need to tighten.  For example:

	git push $remote HEAD~6

does not error out as it should.  'HEAD~6' is expanded to
'HEAD~6:HEAD~6'; its left hand side is valid (6 revs before the
tip is used to update the remote side) but its right hand side
is not checked for validity ("HEAD~6" is not a valid refname)
and creates .git/HEAD~6 at the remote end which is completely
bogus.