threads / patch / 8464

patchFix the remote note the fetch-tool prints after storing a fetched reference

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

## tl;dr

9 messages between Jun 6, 2007 and Jun 6, 2007. Diffs are folded; open one to read it.

replies: 8people: 2as markdown or json

Alex Riesen· Jun 6, 2007, 07:45 UTC · lore
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(-)
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(-)
Show changes to builtin-fetch--tool.c +6 −6
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
Junio C Hamano· Jun 6, 2007, 08:14 UTC · re: Alex Riesen · lore

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

"Alex Riesen" <raa.lkml@gmail.com> writes:
Show 9 quoted lines
> 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.

Alex Riesen· Jun 6, 2007, 08:42 UTC · re: Junio C Hamano · lore

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

On 6/6/07, Junio C Hamano <gitster@pobox.com> wrote:
Show 13 quoted lines
> "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".

J Junio C Hamano· Jun 6, 2007, 08:50 UTC · re: Alex Riesen · lore

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

"Alex Riesen" <raa.lkml@gmail.com> writes:
Show 10 quoted lines
> 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.

Alex Riesen· Jun 6, 2007, 11:27 UTC · re: J Junio C Hamano · lore

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

On 6/6/07, J Junio C Hamano <gitster@pobox.com> wrote:
Show 9 quoted lines
> > 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!

Show 7 quoted lines
> > 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?
Junio C Hamano· Jun 6, 2007, 19:28 UTC · re: Alex Riesen · lore

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

"Alex Riesen" <raa.lkml@gmail.com> writes:
Show 9 quoted lines
>> > 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.

Alex Riesen· Jun 6, 2007, 15:52 UTC · re: Alex Riesen · lore

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(-)
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(-)
Show changes to builtin-fetch--tool.c +15 −6
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
Junio C Hamano· Jun 6, 2007, 20:07 UTC · re: Alex Riesen · lore

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

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

Alex Riesen· Jun 6, 2007, 22:48 UTC · re: Junio C Hamano · lore

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

Junio C Hamano, Wed, Jun 06, 2007 22:07:24 +0200:
Show 18 quoted lines
> "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.

Show 5 quoted lines
> 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:
Show changes to git-clone.sh +1 −1
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

← back to recent threads