[PATCH] Fix the remote note the fetch-tool prints after storing a fetched reference

[PATCH] Fix the remote note the fetch-tool prints after storing a fetched reference

[Alex Riesen]

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

Otherwise ".git" is removed from every remote name which has it:

  $ git fetch -v
    * refs/heads/origin: same as branch 'master' of /home/user/linux
      commit: 5ecd310
  $ ls /home/user/linux
  ls: /home/user/linux: No such file or directory
  $ ls /home/user/linux.git
  HEAD  objects  packed-refs  ...

Signed-off-by: Alex Riesen <raa.lkml@gmail.com>
---
 builtin-fetch--tool.c |   12 ++++++------
 1 files changed, 6 insertions(+), 6 deletions(-)

[-- Attachment #2: 0001-Fix-the-remote-note-the-fetch-tool-prints-after-stor.txt --]
[-- Type: text/plain, Size: 1536 bytes --]

From 100742821619fdd83e75aa8dcb88489aa4b60648 Mon Sep 17 00:00:00 2001
From: Alex Riesen <raa@limbo.localdomain>
Date: Wed, 6 Jun 2007 00:16:14 +0200
Subject: [PATCH] Fix the remote note the fetch-tool prints after storing a fetched reference

Otherwise ".git" is removed from every remote name which has it:

  $ git fetch -v
    * refs/heads/origin: same as branch 'master' of /home/user/linux
      commit: 5ecd310
  $ ls /home/user/linux
  ls: /home/user/linux: No such file or directory
  $ ls /home/user/linux.git
  HEAD  objects  packed-refs  ...

Signed-off-by: Alex Riesen <raa@limbo.localdomain>
---
 builtin-fetch--tool.c |   12 ++++++------
 1 files changed, 6 insertions(+), 6 deletions(-)

diff --git a/builtin-fetch--tool.c b/builtin-fetch--tool.c
index ed4d5de..22ca125 100644
--- a/builtin-fetch--tool.c
+++ b/builtin-fetch--tool.c
@@ -140,7 +140,7 @@ static int append_fetch_head(FILE *fp,
 			     int verbose, int force)
 {
 	struct commit *commit;
-	int remote_len, i, note_len;
+	int remote_len, note_len;
 	unsigned char sha1[20];
 	char note[1024];
 	const char *what, *kind;
@@ -173,11 +173,11 @@ static int append_fetch_head(FILE *fp,
 	}
 
 	remote_len = strlen(remote);
-	for (i = remote_len - 1; remote[i] == '/' && 0 <= i; i--)
-		;
-	remote_len = i + 1;
-	if (4 < i && !strncmp(".git", remote + i - 3, 4))
-		remote_len = i - 3;
+	if (remote_len > 5) {
+		char *p = strrchr(remote, '/');
+		if (p && !strcmp(p, "/.git"))
+		    remote_len -= 4;
+	}
 
 	note_len = 0;
 	if (*what) {
-- 
1.5.2.1.134.g352b

Re: [PATCH] Fix the remote note the fetch-tool prints after storing a fetched reference

[Junio C Hamano]

"Alex Riesen" <raa.lkml@gmail.com> writes:

> Otherwise ".git" is removed from every remote name which has it:
>
>  $ git fetch -v
>    * refs/heads/origin: same as branch 'master' of /home/user/linux
>      commit: 5ecd310
>  $ ls /home/user/linux
>  ls: /home/user/linux: No such file or directory
>  $ ls /home/user/linux.git
>  HEAD  objects  packed-refs  ...

I suspect the above misses the point.

The test "ls /home/user/linux" is not relevant.  Ability to say
"git fetch /home/user/linux" is.

Re: [PATCH] Fix the remote note the fetch-tool prints after storing a fetched reference

[Alex Riesen]

On 6/6/07, Junio C Hamano <gitster@pobox.com> wrote:
> "Alex Riesen" <raa.lkml@gmail.com> writes:
>
> > Otherwise ".git" is removed from every remote name which has it:
> >
> >  $ git fetch -v
> >    * refs/heads/origin: same as branch 'master' of /home/user/linux
> >      commit: 5ecd310
> >  $ ls /home/user/linux
> >  ls: /home/user/linux: No such file or directory
> >  $ ls /home/user/linux.git
> >  HEAD  objects  packed-refs  ...
>
> I suspect the above misses the point.

Depends on the point. I mean to say: the path in the output
of the command does not exist anywhere.

> The test "ls /home/user/linux" is not relevant.  Ability to say
> "git fetch /home/user/linux" is.

This is still ambiguous:

$ git fetch -v /home/user/a
* fetched /home/user/a
  commit: b85c140
$ git fetch -v /home/user/a.git
* fetched /home/user/a
  commit: b85c140

Which one was fetched when? /home/user/a or /home/user/a.git?
No imagine you don't see the command (as if git fetch is called
in some frontend). Would you be able to distinguish the sources?

Besides, I just noticed git-clone is broken WRT the .git
as well: I can clone a "a.git" into "b" (and it ignores -l and -s!),
but I can't fetch the "a" (aka "origin") being in "b". And of
course, "origin" in "b" is setup as "/path/a", not "/path/a.git".

Re: [PATCH] Fix the remote note the fetch-tool prints after storing a fetched reference

[J Junio C Hamano]

"Alex Riesen" <raa.lkml@gmail.com> writes:

> On 6/6/07, Junio C Hamano <gitster@pobox.com> wrote:
>> I suspect the above misses the point.
>
> Depends on the point. I mean to say: the path in the output
> of the command does not exist anywhere.
>
>> The test "ls /home/user/linux" is not relevant.  Ability to say
>> "git fetch /home/user/linux" is.
>
> This is still ambiguous:

Yup, there is an ambiguity.  Always has been.

> Which one was fetched when? /home/user/a or /home/user/a.git?

I am half tempted to say that this is very close to "doctor, it
confuses me when I have both of them".  Imagine the case where
the source were a remote repository _and_ there was no way other
than interacting with it via git protocol.  You cannot really
tell (well, you can tell half, by trying both and if a worked
but a.git didn't you can tell that a.git does not exist) nor you
do not really care.

> Besides, I just noticed git-clone is broken WRT the .git
> as well: I can clone a "a.git" into "b" (and it ignores -l and -s!),
> but I can't fetch the "a" (aka "origin") being in "b". And of
> course, "origin" in "b" is setup as "/path/a", not "/path/a.git".

This probably is worth fixing, independent from what the message
says before or after your patch.

Re: [PATCH] Fix the remote note the fetch-tool prints after storing a fetched reference

[Alex Riesen]

On 6/6/07, J Junio C Hamano <gitster@pobox.com> wrote:
> > Which one was fetched when? /home/user/a or /home/user/a.git?
>
> I am half tempted to say that this is very close to "doctor, it
> confuses me when I have both of them".  Imagine the case where
> the source were a remote repository _and_ there was no way other
> than interacting with it via git protocol.  You cannot really
> tell (well, you can tell half, by trying both and if a worked
> but a.git didn't you can tell that a.git does not exist) nor you
> do not really care.

Of course you can! Just print the path to the remote EXACTLY
how the user entered it!

> > Besides, I just noticed git-clone is broken WRT the .git
> > as well: I can clone a "a.git" into "b" (and it ignores -l and -s!),
> > but I can't fetch the "a" (aka "origin") being in "b". And of
> > course, "origin" in "b" is setup as "/path/a", not "/path/a.git".
>
> This probably is worth fixing, independent from what the message
> says before or after your patch.

And "probably" means: it is broken, but this is ok?

Re: [PATCH] Fix the remote note the fetch-tool prints after storing a fetched reference

[Junio C Hamano]

"Alex Riesen" <raa.lkml@gmail.com> writes:

>> > Besides, I just noticed git-clone is broken WRT the .git
>> > as well: I can clone a "a.git" into "b" (and it ignores -l and -s!),
>> > but I can't fetch the "a" (aka "origin") being in "b". And of
>> > course, "origin" in "b" is setup as "/path/a", not "/path/a.git".
>>
>> This probably is worth fixing, independent from what the message
>> says before or after your patch.
>
> And "probably" means: it is broken, but this is ok?

No; it means "patches welcome".  Regardless of the patch you are
promoting, that would be a real fix.

[PATCH] Fix the remote note the fetch-tool prints after storing a fetched reference

[Alex Riesen]

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

Otherwise ".git" is removed from every remote path which has it,
including "/some/path/somename.git".

Signed-off-by: Alex Riesen <raa.lkml@gmail.com>
---

Fixed truncation of "/.git/" (with a trailing slash).
The old code broke t5515, because it has left "../.git/"
unchanged, but should have removed ".git/".

 builtin-fetch--tool.c |   21 +++++++++++++++------
 1 files changed, 15 insertions(+), 6 deletions(-)

[-- Attachment #2: 0001-Fix-the-remote-note-the-fetch-tool-prints-after-stori.txt --]
[-- Type: text/plain, Size: 1830 bytes --]

From c767dcc2000c72909013b9e97b18403686b44498 Mon Sep 17 00:00:00 2001
From: Alex Riesen <raa@limbo.localdomain>
Date: Wed, 6 Jun 2007 00:16:14 +0200
Subject: [PATCH] Fix the remote note the fetch-tool prints after storing a fetched reference

Otherwise ".git" is removed from every remote name which has it:

  $ git fetch -v
    * refs/heads/origin: same as branch 'master' of /home/user/linux
      commit: 5ecd310
  $ ls /home/user/linux
  ls: /home/user/linux: No such file or directory
  $ ls /home/user/linux.git
  HEAD  objects  packed-refs  ...

Signed-off-by: Alex Riesen <raa.lkml@gmail.com>
---
 builtin-fetch--tool.c |   21 +++++++++++++++------
 1 files changed, 15 insertions(+), 6 deletions(-)

diff --git a/builtin-fetch--tool.c b/builtin-fetch--tool.c
index ed4d5de..ec4a8c3 100644
--- a/builtin-fetch--tool.c
+++ b/builtin-fetch--tool.c
@@ -140,7 +140,7 @@ static int append_fetch_head(FILE *fp,
 			     int verbose, int force)
 {
 	struct commit *commit;
-	int remote_len, i, note_len;
+	int remote_len, note_len;
 	unsigned char sha1[20];
 	char note[1024];
 	const char *what, *kind;
@@ -173,11 +173,20 @@ static int append_fetch_head(FILE *fp,
 	}
 
 	remote_len = strlen(remote);
-	for (i = remote_len - 1; remote[i] == '/' && 0 <= i; i--)
-		;
-	remote_len = i + 1;
-	if (4 < i && !strncmp(".git", remote + i - 3, 4))
-		remote_len = i - 3;
+	if (remote_len > 5) {
+		static const char ending[] = "/.git";
+		const char *pe = ending + 4;
+		const char *pr = remote + remote_len - 1;
+		/* ignore trailing slashes */
+		while (pr > remote && *pr == '/')
+			--pr;
+		for (; pr > remote && pe > ending; --pr, --pe)
+			if (*pr != *pe)
+				break;
+		if (pe == ending)
+			/* "+ 1" to leave the / before .git */
+			remote_len = pr - remote + 1;
+	}
 
 	note_len = 0;
 	if (*what) {
-- 
1.5.2.1.868.gb85c1

Re: [PATCH] Fix the remote note the fetch-tool prints after storing a fetched reference

[Junio C Hamano]

"Alex Riesen" <raa.lkml@gmail.com> writes:

> Otherwise ".git" is removed from every remote path which has it,
> including "/some/path/somename.git".

I was very tempted to apply this patch, but after re-reading it
once more, I decided to drop it.

I do not mind keeping the extra .git/ suffix in the -v output in
your example, as the message is just "verbose information":

  $ git fetch -v
    * refs/heads/origin: same as branch 'master' of /home/user/linux
      commit: 5ecd310

But the same output is used by log messages for the merge
commits; indeed, the primary purpose of this output is to
prepare messages for merge commits.  From the beginning we
deliberately omitted .git suffix to keep the resulting log
message short and sweet.  Keeping .git suffix goes against it.

A repository "git://$site/repo.git" can be accessed with
"git://$site/repo".  Repository owners _can_ confuse their users
by having both repo and repo.git, but I honestly do not see a
reason to encourage that.

Earlier you mentioned you know of a case where "$URL/repo.git"
exists and "$URL/repo" does not, and the current clone fails to
set up the target repository after cloning from "$URL/repo"
correctly?  I admit I haven't reproduced nor debugged it, but if
that is the case that needs to be fixed.

Re: [PATCH] Fix the remote note the fetch-tool prints after storing a fetched reference

[Alex Riesen]

Junio C Hamano, Wed, Jun 06, 2007 22:07:24 +0200:
> "Alex Riesen" <raa.lkml@gmail.com> writes:
> 
> > Otherwise ".git" is removed from every remote path which has it,
> > including "/some/path/somename.git".
> 
> I was very tempted to apply this patch, but after re-reading it
> once more, I decided to drop it.
> 
> I do not mind keeping the extra .git/ suffix in the -v output in
> your example, as the message is just "verbose information":
> 
>   $ git fetch -v
>     * refs/heads/origin: same as branch 'master' of /home/user/linux
>       commit: 5ecd310
> 
> But the same output is used by log messages for the merge
> commits; indeed, the primary purpose of this output is to
> prepare messages for merge commits.

BTW, this is hard to believe. Were does git-merge call fetch--tool?
Nor have I found its use for logging purposes. Or, if you refer to the
logs a user can create by redirecting output of git fetch someplace,
than it is highly suspicious for tham to contain something short,
sweet and not existing.

> From the beginning we deliberately omitted .git suffix to keep the
> resulting log message short and sweet.  Keeping .git suffix goes
> against it.

But this is how the repo is actually named! If you want to keep the
_message_ short and sweet than cut the suffix in the message
generation code.

> A repository "git://$site/repo.git" can be accessed with
> "git://$site/repo".  Repository owners _can_ confuse their users
> by having both repo and repo.git, but I honestly do not see a
> reason to encourage that.

People will never know whether they are encouraged or discouraged.
They just wont notice it is _wrong_ until they copy-paste the path
from git-fetch output and do something nasty to it.

> Earlier you mentioned you know of a case where "$URL/repo.git"
> exists and "$URL/repo" does not, and the current clone fails to
> set up the target repository after cloning from "$URL/repo"
> correctly?  I admit I haven't reproduced nor debugged it, but if
> that is the case that needs to be fixed.

    mkdir a.git && cd a.git && git init && :>file && \
    git add . && git commit -m1 && mv .git/* . && rmdir .git && \
    cd .. && git clone -l -s a b && cd b && git fetch

The cloned repo "b" gets "a" verbatim, instead of something more
useful (like "../../a.git" or "/full/path/a.git". It is not even
"/full/path/a").

git clone works correctly (origin's url is an absolute path) if a.git
is given:

    mkdir a.git && cd a.git && git init && :>file && \
    git add . && git commit -m1 && mv .git/* . && rmdir .git && \
    cd .. && git clone -l -s a.git b && cd b && git fetch

Fix, needed because of all this "detachable suffix" confusion:

diff --git a/git-clone.sh b/git-clone.sh
index fdd354f..d45618d 100755
--- a/git-clone.sh
+++ b/git-clone.sh
@@ -20,7 +20,7 @@ usage() {
 get_repo_base() {
 	(
 		cd "`/bin/pwd`" &&
-		cd "$1" &&
+		cd "$1" || cd "$1.git" &&
 		{
 			cd .git
 			pwd