threads / patch / 5920

patchclone: the given repository dir should be relative to $PWD

Subject: [PATCH] clone: the given repository dir should be relative to $PWD

## tl;dr

3 messages between Oct 14, 2006 and Oct 15, 2006. Diffs are folded; open one to read it.

replies: 2people: 2as markdown or json

Yasushi SHOJI· Oct 14, 2006, 12:02 UTC · lore

the repository argument for git-clone should be relative to $PWD instead of the given target directory. The old behavior gave us surprising success and you need a few minute to know why it worked.

GIT_DIR is already exported so no need to cd into $D. And this makes $PWD for git-fetch-pack, which is the actual command to take the given repository dir, the same as git-clone.

Signed-off-by: Yasushi SHOJI <yashi@atmark-techno.com>
---

While I'm not sure this is a feature we rely on or not, and I don't want to change the way people work, IMHO the old behaviour isn't appropriate for such higher level porcelain.

The patch should be for post 1.4.3.
 git-clone.sh                  |    2 +-
 t/t5600-clone-fail-cleanup.sh |    6 ++++++
 2 files changed, 7 insertions(+), 1 deletions(-)
Show changes to 2 files +7 −1

git-clone.sh, t/t5600-clone-fail-cleanup.sh

diff --git a/git-clone.sh b/git-clone.sh
index 3998c55..bf54a11 100755
--- a/git-clone.sh
+++ b/git-clone.sh
@@ -312,7 +312,7 @@ yes,yes)
 		fi
 		;;
 	*)
-		cd "$D" && case "$upload_pack" in
+		case "$upload_pack" in
 		'') git-fetch-pack --all -k $quiet "$repo" ;;
 		*) git-fetch-pack --all -k $quiet "$upload_pack" "$repo" ;;
 		esac >"$GIT_DIR/CLONE_HEAD" || {
diff --git a/t/t5600-clone-fail-cleanup.sh b/t/t5600-clone-fail-cleanup.sh
index 0c6a363..041be04 100755
--- a/t/t5600-clone-fail-cleanup.sh
+++ b/t/t5600-clone-fail-cleanup.sh
@@ -25,6 +25,12 @@ test_create_repo foo
 # clone doesn't like it if there is no HEAD. Is that a bug?
 (cd foo && touch file && git add file && git commit -m 'add file' >/dev/null 2>&1)
 
+# source repository given to git-clone should be relative to the
+# current path not to the target dir
+test_expect_failure \
+    'clone of non-existent (relative to $PWD) source should fail' \
+    'git-clone ../foo baz'
+
 test_expect_success \
     'clone should work now that source exists' \
     'git-clone foo bar'
-- 
1.4.2.3
Junio C Hamano· Oct 15, 2006, 01:16 UTC · re: Yasushi SHOJI · lore

Re: [PATCH] clone: the given repository dir should be relative to $PWD

Yasushi SHOJI <yashi@atmark-techno.com> writes:
Show 16 quoted lines
> the repository argument for git-clone should be relative to $PWD
> instead of the given target directory.  The old behavior gave us
> surprising success and you need a few minute to know why it worked.
>
> GIT_DIR is already exported so no need to cd into $D. And this makes
> $PWD for git-fetch-pack, which is the actual command to take the given
> repository dir, the same as git-clone.
>
> Signed-off-by: Yasushi SHOJI <yashi@atmark-techno.com>
> ---
>
> While I'm not sure this is a feature we rely on or not, and I don't
> want to change the way people work, IMHO the old behaviour isn't
> appropriate for such higher level porcelain.
>
> The patch should be for post 1.4.3.

Well spotted. I am fairly sure that this "clone from repository relative to the target" is not intended behaviour. I'd say we should fix this before 1.4.3.

... or are there any valid reason to keep the current behaviour that I missed?

Yasushi SHOJI· Oct 15, 2006, 03:09 UTC · re: Junio C Hamano · lore

Re: [PATCH] clone: the given repository dir should be relative to $PWD

At Sat, 14 Oct 2006 18:16:33 -0700, Junio C Hamano wrote:

Show 23 quoted lines
> 
> Yasushi SHOJI <yashi@atmark-techno.com> writes:
> 
> > the repository argument for git-clone should be relative to $PWD
> > instead of the given target directory.  The old behavior gave us
> > surprising success and you need a few minute to know why it worked.
> >
> > GIT_DIR is already exported so no need to cd into $D. And this makes
> > $PWD for git-fetch-pack, which is the actual command to take the given
> > repository dir, the same as git-clone.
> >
> > Signed-off-by: Yasushi SHOJI <yashi@atmark-techno.com>
> > ---
> >
> > While I'm not sure this is a feature we rely on or not, and I don't
> > want to change the way people work, IMHO the old behaviour isn't
> > appropriate for such higher level porcelain.
> >
> > The patch should be for post 1.4.3.
> 
> Well spotted.  I am fairly sure that this "clone from repository
> relative to the target" is not intended behaviour.  I'd say we
> should fix this before 1.4.3.

OK. if the behavior isn't intended and there ain't much user for it, I don't have any reason not to. my last sentence was more like a question to you rather than my statement.

let's fix it before 1.4.3.
> ... or are there any valid reason to keep the current behaviour
> that I missed?

I don't think so. I personally consider the behavior a bug. I just thought that we don't want to have user saying "hey, v1.4.3 doesn't work any more!" report, given that we are already in -rc2.

-- 
          yashi

← back to recent threads