Re: git commit/push can fail silently when clone omits ".git"
Hi, On Thu, Nov 08, 2012 at 01:56:43PM -0500, Jeff King wrote: > Unfortunately, the patch below which does that seems to make t7407 very > unhappy. It looks like the submodule test uses "git clone ." and > "git-submodule add" expects the "/." to still be at the end of the > configured URL when processing relative submodule paths. I'm not sure if > that is important, or an unnecessary brittleness in the submodule code. > > Jens, Heiko? After some analysis it seems to me that the test deviates from the expected behavior. For relative urls we have documented that if we have a remote in the superproject a relative submodule path is relative to that remotes url. In the test super has been cloned from ".". So the tests root directory should be the directory the submodule path is relative to. That would be ./submodule (since submodule is also in the root directory) and not ../submodule. Before your patch a "/." was added to the origin of super and "/." is currently counted as a path component. So we have another corner case here: When your superproject was cloned from "." the urls you currently have to specify with submodule add are wrong (one ".." to much). Since this is a change in behaviour I would like to further think about the implications this brings if we fix this. Not sure how many people clone from ".". The correct behavior (as documented) is the one you introduce with your patch. If we decide to fix this we should also correct the path calculation in git-submodule.sh. Cheers Heiko -- To unsubscribe from this list: send the line "unsubscribe git" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: git commit/push can fail silently when clone omits ".git"
On Sun, Nov 04, 2012 at 12:50:58PM -0700, Jeffrey S. Haemer wrote:
> I got bitten by what follows. Yes, it's an edge case. Yes I now understand
> why it does what it does. Yes the right answer is "Don't do that, Jeff." :-)
>
> Still, it took me a little time to figure out what I'd done wrong because
> the failure is silent, so I thought I'd document it. Perhaps there's even
> some way to issue an error message for cases like this.
>
> The attached test script shows the issue in detail, but here's the basic
> failure:
>
> $ ls
> hello.git
> $ git clone hello # *Mistake!* Succeeds, but should have cloned "hello.git"
> or into something else.
It does clone hello.git into "hello", but it sets remote.origin.url in
the cloned repository to "/path/to/hello". I.e., to itself, rather than
the correct hello.git.
The reason is that "clone" sets the config from the repo name you gave
it, not the path it finds on disk. The name you gave was not ambiguous
at the time of clone, but it became so during the clone. I am tempted to
say that we should set the config to the path we found on disk, not what
the user gave us. That includes the ugly "/.git" for non-bare repos, but
we should be able to safely strip that off without adding any ambiguity
(i.e, it is only "foo" versus "foo.git" that is ambiguous).
Unfortunately, the patch below which does that seems to make t7407 very
unhappy. It looks like the submodule test uses "git clone ." and
"git-submodule add" expects the "/." to still be at the end of the
configured URL when processing relative submodule paths. I'm not sure if
that is important, or an unnecessary brittleness in the submodule code.
Jens, Heiko?
---
diff --git a/builtin/clone.c b/builtin/clone.c
index 0d663e3..687d5c0 100644
--- a/builtin/clone.c
+++ b/builtin/clone.c
@@ -713,8 +713,12 @@ int cmd_clone(int argc, const char **argv, const char
*prefix)
repo_name = argv[0];
path = get_repo_path(repo_name, &is_bundle);
- if (path)
- repo = xstrdup(absolute_path(repo_name));
+ if (path) {
+ if (!suffixcmp(path, "/.git"))
+ repo = xstrndup(path, strlen(path) - 5);
+ else
+ repo = xstrdup(path);
+ }
else if (!strchr(repo_name, ':'))
die(_("repository '%s' does not exist"), repo_name);
else
diff --git a/t/t5601-clone.sh b/t/t5601-clone.sh
index 67869b4..0eeeb2d 100755
--- a/t/t5601-clone.sh
+++ b/t/t5601-clone.sh
@@ -280,4 +280,20 @@ test_expect_success 'clone checking out a tag' '
test_cmp fetch.expected fetch.actual
'
+test_expect_success 'clone does not create ambiguous config' '
+ git init --bare ambiguous.git &&
+ git clone ambiguous &&
+ (
+ cd ambiguous &&
+ test_commit one &&
+ git push --all
+ ) &&
+ echo one >expect &&
+ (
+ cd ambiguous.git &&
+ git log -1 --format=%s
+ ) >actual &&
+ test_cmp expect actual
+'
+
test_done
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
git commit/push can fail silently when clone omits ".git"
Ladies and Gentlemen, I'm running git 1.7.9.5 on Ubuntu 12.04.1 LTS I got bitten by what follows. Yes, it's an edge case. Yes I now understand why it does what it does. Yes the right answer is "Don't do that, Jeff." :-) Still, it took me a little time to figure out what I'd done wrong because the failure is silent, so I thought I'd document it. Perhaps there's even some way to issue an error message for cases like this. The attached test script shows the issue in detail, but here's the basic failure: $ ls hello.git $ git clone hello # *Mistake!* Succeeds, but should have cloned "hello.git" or into something else. $ cd hello; touch foo; git add foo; git commit -am"add a new file" $ git status # says I'm a rev ahead of the origin $ git push # nothing pushed $ git status # says everything's okay At this point hello/foo still exists, there's nothing to commit, git diff origin/master reports nothing, yet foo was never pushed to hello.git. HTH! -- Jeffrey Haemer 720-837-8908 [cell], http://seejeffrun.blogspot.com [blog], http://www.youtube.com/user/goyishekop [vlog] פרייהייט? דאס איז יאַנג דינען וואָרט. clone-from-suffixless-gitrepo-issue.sh Description: Bourne shell script
