diff mbox series

[v2,1/4] submodule: refuse to add repository with no commits

Message ID 20190402183505.31512-2-kyle@kyleam.com (mailing list archive)
State New, archived
Headers show
Series dir: Treat a repository without commits as a repository | expand

Commit Message

Kyle Meyer April 2, 2019, 6:35 p.m. UTC
When the path given to 'git submodule add' is an existing repository
that is not in the index, the repository is passed to 'git add'.  If
this repository doesn't have any commits, we don't get a useful
result: there is no subproject OID to track, and any untracked files
in the sub-repository are added to the current repository.

Detect if the path is a repository with no commits and abort to avoid
getting into this state.  Note that this check must come before the
'git add --dry-run' check because an upcoming commit will make 'git
add' fail in this situation.

Signed-off-by: Kyle Meyer <kyle@kyleam.com>
---
 git-submodule.sh           |  7 +++++++
 t/t7400-submodule-basic.sh | 11 ++++++++++-
 2 files changed, 17 insertions(+), 1 deletion(-)

Comments

Junio C Hamano April 4, 2019, 7:24 a.m. UTC | #1
Kyle Meyer <kyle@kyleam.com> writes:

> When the path given to 'git submodule add' is an existing repository
> that is not in the index, the repository is passed to 'git add'.  If
> this repository doesn't have any commits, we don't get a useful
> result: there is no subproject OID to track, and any untracked files
> in the sub-repository are added to the current repository.
>
> Detect if the path is a repository with no commits and abort to avoid
> getting into this state.  Note that this check must come before the
> 'git add --dry-run' check because an upcoming commit will make 'git
> add' fail in this situation.

OK, in short, in an already-populated repository,

	$ git init junk ;# there was no 'junk' subdirectory
	$ git add junk

would have been a successful no-op, and after the above two
commands,

	$ >junk/one
	$ git add junk

would have added junk/one as a normal blob to the top-level project,
ignoring the fact that the user wanted to eventually populate junk
as a repository and add it as a submodule.  You'll be teaching "git
add" not to do so, and pay attention to the fact that junk/.git exists,
even though junk/.git/HEAD is on an unborn branch.

Which makes sense to me.

> Signed-off-by: Kyle Meyer <kyle@kyleam.com>
> ---
>  git-submodule.sh           |  7 +++++++
>  t/t7400-submodule-basic.sh | 11 ++++++++++-
>  2 files changed, 17 insertions(+), 1 deletion(-)
>
> diff --git a/git-submodule.sh b/git-submodule.sh
> index 514ede2596..5a9dc9bf7e 100755
> --- a/git-submodule.sh
> +++ b/git-submodule.sh
> @@ -231,6 +231,13 @@ cmd_add()
>  		die "$(eval_gettext "'\$sm_path' already exists in the index and is not a submodule")"
>  	fi
>  
> +	if test -d "$sm_path" &&
> +		test -z $(git -C "$sm_path" rev-parse --show-cdup 2>/dev/null)
> +	then
> +	    git -C "$sm_path" rev-parse --verify -q HEAD >/dev/null ||
> +	    die "$(eval_gettext "'\$sm_path' does not have any commits")"
> +	fi
> +
>  	if test -z "$force" &&
>  		! git add --dry-run --ignore-missing --no-warn-embedded-repo "$sm_path" > /dev/null 2>&1
>  	then
> diff --git a/t/t7400-submodule-basic.sh b/t/t7400-submodule-basic.sh
> index aba2d4d6ee..6adf640143 100755
> --- a/t/t7400-submodule-basic.sh
> +++ b/t/t7400-submodule-basic.sh
> @@ -46,6 +46,15 @@ test_expect_success 'submodule update aborts on missing gitmodules url' '
>  	test_must_fail git submodule init
>  '
>  
> +test_expect_success 'add aborts on repository with no commits' '
> +	cat >expected <<-\EOF &&
> +	'"'repo-no-commits'"' does not have any commits
> +	EOF
> +	git init repo-no-commits &&
> +	test_must_fail git submodule add ../a ./repo-no-commits 2>actual &&
> +	test_i18ncmp expected actual
> +'
> +
>  test_expect_success 'setup - repository in init subdirectory' '
>  	mkdir init &&
>  	(
> @@ -809,7 +818,7 @@ test_expect_success '../bar/a/b/c works with relative local path - ../foo/bar.gi
>  		cp pristine-.git-config .git/config &&
>  		cp pristine-.gitmodules .gitmodules &&
>  		mkdir -p a/b/c &&
> -		(cd a/b/c && git init) &&
> +		(cd a/b/c && git init && test_commit msg) &&
>  		git config remote.origin.url ../foo/bar.git &&
>  		git submodule add ../bar/a/b/c ./a/b/c &&
>  		git submodule init &&
diff mbox series

Patch

diff --git a/git-submodule.sh b/git-submodule.sh
index 514ede2596..5a9dc9bf7e 100755
--- a/git-submodule.sh
+++ b/git-submodule.sh
@@ -231,6 +231,13 @@  cmd_add()
 		die "$(eval_gettext "'\$sm_path' already exists in the index and is not a submodule")"
 	fi
 
+	if test -d "$sm_path" &&
+		test -z $(git -C "$sm_path" rev-parse --show-cdup 2>/dev/null)
+	then
+	    git -C "$sm_path" rev-parse --verify -q HEAD >/dev/null ||
+	    die "$(eval_gettext "'\$sm_path' does not have any commits")"
+	fi
+
 	if test -z "$force" &&
 		! git add --dry-run --ignore-missing --no-warn-embedded-repo "$sm_path" > /dev/null 2>&1
 	then
diff --git a/t/t7400-submodule-basic.sh b/t/t7400-submodule-basic.sh
index aba2d4d6ee..6adf640143 100755
--- a/t/t7400-submodule-basic.sh
+++ b/t/t7400-submodule-basic.sh
@@ -46,6 +46,15 @@  test_expect_success 'submodule update aborts on missing gitmodules url' '
 	test_must_fail git submodule init
 '
 
+test_expect_success 'add aborts on repository with no commits' '
+	cat >expected <<-\EOF &&
+	'"'repo-no-commits'"' does not have any commits
+	EOF
+	git init repo-no-commits &&
+	test_must_fail git submodule add ../a ./repo-no-commits 2>actual &&
+	test_i18ncmp expected actual
+'
+
 test_expect_success 'setup - repository in init subdirectory' '
 	mkdir init &&
 	(
@@ -809,7 +818,7 @@  test_expect_success '../bar/a/b/c works with relative local path - ../foo/bar.gi
 		cp pristine-.git-config .git/config &&
 		cp pristine-.gitmodules .gitmodules &&
 		mkdir -p a/b/c &&
-		(cd a/b/c && git init) &&
+		(cd a/b/c && git init && test_commit msg) &&
 		git config remote.origin.url ../foo/bar.git &&
 		git submodule add ../bar/a/b/c ./a/b/c &&
 		git submodule init &&