From: Jens Lehmann <Jens.Lehmann@web.de>
To: Jonathan Nieder <jrnieder@gmail.com>
Cc: git@vger.kernel.org, Klaus Ethgen <Klaus@Ethgen.de>,
Sven Verdoolaege <skimo@kotnet.org>,
mlevedahl@gmail.com, ben@ben.com,
Junio C Hamano <gitster@pobox.com>
Subject: [PATCH] git submodule: Remove now obsolete tests before cloning a repo
Date: Sun, 05 Dec 2010 00:27:35 +0100 [thread overview]
Message-ID: <4CFACE67.1080206@web.de> (raw)
In-Reply-To: <20101203071037.GA18202@burratino>
Since 55892d23 "git clone" itself checks that the destination path is not
a file but an empty directory if it exists, so there is no need anymore
for module_clone() to check that too.
Two tests have been added to test the behavior of "git submodule add" when
path is a file or a directory (A subshell had to be added to the former
last test to stay in the right directory).
Signed-off-by: Jens Lehmann <Jens.Lehmann@web.de>
---
Am 03.12.2010 08:10, schrieb Jonathan Nieder:
> Jens Lehmann wrote:
>> Am 01.12.2010 19:50, schrieb Jonathan Nieder:
>
>>> Jens, any idea why git submodule
>>> is not using "clone --branch" directly?
>>
>> Nope, these lines date back to the time before I got involved in the
>> submodule business ... Seems like this "git checkout" was added in
>> March 2008 by Mark Levedahl (CCed), maybe he can shed some light on
>> that.
>
> Ah, so the problem is that "clone --branch" did not exist. Sorry for
> the noise.
>
> Another question can be also be easily answered by history examination:
> the series of checks in module_clone are because 70c7ac22d:git-clone.sh
> did not have checks of its own for the target directory.
>
> So there is some simplification within grasp.
Maybe something like this?
git-submodule.sh | 14 --------------
t/t7400-submodule-basic.sh | 28 +++++++++++++++++++++++-----
2 files changed, 23 insertions(+), 19 deletions(-)
diff --git a/git-submodule.sh b/git-submodule.sh
index 33bc41f..8085876 100755
--- a/git-submodule.sh
+++ b/git-submodule.sh
@@ -93,20 +93,6 @@ module_clone()
url=$2
reference="$3"
- # If there already is a directory at the submodule path,
- # expect it to be empty (since that is the default checkout
- # action) and try to remove it.
- # Note: if $path is a symlink to a directory the test will
- # succeed but the rmdir will fail. We might want to fix this.
- if test -d "$path"
- then
- rmdir "$path" 2>/dev/null ||
- die "Directory '$path' exists, but is neither empty nor a git repository"
- fi
-
- test -e "$path" &&
- die "A file already exist at path '$path'"
-
if test -n "$reference"
then
git-clone "$reference" -n "$url" "$path"
diff --git a/t/t7400-submodule-basic.sh b/t/t7400-submodule-basic.sh
index 782b0a3..2c49db9 100755
--- a/t/t7400-submodule-basic.sh
+++ b/t/t7400-submodule-basic.sh
@@ -421,11 +421,29 @@ test_expect_success 'add submodules without specifying an explicit path' '
git commit -m "repo commit 1"
) &&
git clone --bare repo/ bare.git &&
- cd addtest &&
- git submodule add "$submodurl/repo" &&
- git config -f .gitmodules submodule.repo.path repo &&
- git submodule add "$submodurl/bare.git" &&
- git config -f .gitmodules submodule.bare.path bare
+ (
+ cd addtest &&
+ git submodule add "$submodurl/repo" &&
+ git config -f .gitmodules submodule.repo.path repo &&
+ git submodule add "$submodurl/bare.git" &&
+ git config -f .gitmodules submodule.bare.path bare
+ )
+'
+
+test_expect_success 'add should fail when path is used by a file' '
+ (
+ cd addtest &&
+ touch file &&
+ test_must_fail git submodule add "$submodurl/repo" file
+ )
+'
+
+test_expect_success 'add should fail when path is used by an existing directory' '
+ (
+ cd addtest &&
+ mkdir empty-dir &&
+ test_must_fail git submodule add "$submodurl/repo" empty-dir
+ )
'
test_done
--
1.7.3.2.657.g92628.dirty
next prev parent reply other threads:[~2010-12-04 23:32 UTC|newest]
Thread overview: 16+ messages / expand[flat|nested] mbox.gz Atom feed top
[not found] <20101201171814.GC6439@ikki.ethgen.de>
2010-12-01 18:50 ` [RFC/PATCH] Re: git submodule -b ... of current HEAD fails Jonathan Nieder
2010-12-02 21:11 ` Jens Lehmann
2010-12-03 1:16 ` Mark Levedahl
2010-12-03 1:21 ` Ben Jackson
2010-12-03 7:10 ` Jonathan Nieder
2010-12-04 23:27 ` Jens Lehmann [this message]
2010-12-07 22:57 ` Junio C Hamano
2010-12-08 21:35 ` Jens Lehmann
2010-12-08 23:19 ` [PATCH] " Jens Lehmann
2010-12-08 23:45 ` Jonathan Nieder
2010-12-28 21:42 ` [RFC/PATCH] " Junio C Hamano
2010-12-29 0:05 ` Jens Lehmann
2010-12-29 0:34 ` Junio C Hamano
2010-12-29 9:04 ` Jens Lehmann
2010-12-29 20:53 ` Re* " Junio C Hamano
[not found] ` <4D1BB26D.1010502@web.de>
2010-12-29 22:23 ` Jens Lehmann
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=4CFACE67.1080206@web.de \
--to=jens.lehmann@web.de \
--cc=Klaus@Ethgen.de \
--cc=ben@ben.com \
--cc=git@vger.kernel.org \
--cc=gitster@pobox.com \
--cc=jrnieder@gmail.com \
--cc=mlevedahl@gmail.com \
--cc=skimo@kotnet.org \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.