{"thread":{"id":"50739","subject":"[PATCH 2/4] t3000: move non-submodule repo test to separate file","startedAt":"2019-03-14T15:09:38Z","lastAt":"2019-04-04T07:24:53Z","messageCount":16,"participants":["Kyle Meyer","Junio C Hamano"],"isPatch":true,"patchVersion":1,"patchTotal":4},"messages":[{"id":"371507","messageId":"20190314150219.2040-3-kyle@kyleam.com","threadId":"50739","inReplyTo":"20190314150219.2040-1-kyle@kyleam.com","subject":"[PATCH 2/4] t3000: move non-submodule repo test to separate file","fromName":"Kyle Meyer","fromEmail":"kyle@kyleam.com","sentAt":"2019-03-14T15:02:17Z","receivedAt":"2019-03-14T15:09:38Z","isPatch":true,"sender":{"key":"kyle@kyleam.com","avatar":"https://avatars.githubusercontent.com/u/1297788?v=4"},"body":"a2d5156c2b (resolve_gitlink_ref: ignore non-repository paths,\n2016-01-22) added a test to t3000-ls-files-others.sh to check that\n'ls-files -o' does not die() when given a subdirectory that looks like\na repository but is actually a subdirectory containing a bogus .git\nfile.\n\nMove this test to a separate file in preparation for testing scenarios\nwith non-submodule repositories that are not bogus.\n\nSigned-off-by: Kyle Meyer <kyle@kyleam.com>\n---\n t/t3000-ls-files-others.sh              |  7 -------\n t/t3009-ls-files-others-nonsubmodule.sh | 21 +++++++++++++++++++++\n 2 files changed, 21 insertions(+), 7 deletions(-)\n create mode 100755 t/t3009-ls-files-others-nonsubmodule.sh\n\ndiff --git a/t/t3000-ls-files-others.sh b/t/t3000-ls-files-others.sh\nindex afd4756134..b4f9fc4580 100755\n--- a/t/t3000-ls-files-others.sh\n+++ b/t/t3000-ls-files-others.sh\n@@ -65,13 +65,6 @@ test_expect_success '--no-empty-directory hides empty directory' '\n \ttest_cmp expected3 output\n '\n \n-test_expect_success 'ls-files --others handles non-submodule .git' '\n-\tmkdir not-a-submodule &&\n-\techo foo >not-a-submodule/.git &&\n-\tgit ls-files -o >output &&\n-\ttest_cmp expected1 output\n-'\n-\n test_expect_success SYMLINKS 'ls-files --others with symlinked submodule' '\n \tgit init super &&\n \tgit init sub &&\ndiff --git a/t/t3009-ls-files-others-nonsubmodule.sh b/t/t3009-ls-files-others-nonsubmodule.sh\nnew file mode 100755\nindex 0000000000..cc66a4a14d\n--- /dev/null\n+++ b/t/t3009-ls-files-others-nonsubmodule.sh\n@@ -0,0 +1,21 @@\n+#!/bin/sh\n+\n+test_description='test git ls-files --others with non-submodule repositories'\n+\n+. ./test-lib.sh\n+\n+test_expect_success 'setup: expected output' '\n+\tcat >expected <<-EOF\n+\texpected\n+\toutput\n+\tEOF\n+'\n+\n+test_expect_success 'ls-files --others handles non-submodule .git' '\n+\tmkdir not-a-submodule &&\n+\techo foo >not-a-submodule/.git &&\n+\tgit ls-files -o >output &&\n+\ttest_cmp expected output\n+'\n+\n+test_done\n-- \n2.21.0\n\n"},{"id":"371508","messageId":"20190314150219.2040-1-kyle@kyleam.com","threadId":"50739","inReplyTo":null,"subject":"[PATCH 0/4] dir: Treat a repository without commits as a repository","fromName":"Kyle Meyer","fromEmail":"kyle@kyleam.com","sentAt":"2019-03-14T15:02:15Z","receivedAt":"2019-03-14T15:09:38Z","isPatch":true,"sender":{"key":"kyle@kyleam.com","avatar":"https://avatars.githubusercontent.com/u/1297788?v=4"},"body":"When the working tree contains a repository with no commits, it's\ntreated as an empty directory, not a repository:\n\n    $ git init\n    $ git init no-commit && touch no-commit/untracked\n    $ git init with-commit && touch with-commit/untracked\n    $ git -C with-commit commit --allow-empty -mmsg\n    $ git ls-files -o\n    no-commit/untracked\n    with-commit/\n\nThat's admittedly a weird case that is unlikely to happen in the wild,\nand indeed I didn't observe it in the wild.  In DataLad, we rely on\n`ls-files -o` to stop at the boundary of a submodule (like it does\nwith \"with-commit\" above), and we noticed the \"repository with no\ncommit\" exception on a minimal snippet that was meant for debugging\nanother issue [*].\n\nThis series makes the \"is repository?\" check in treat_directory() use\nis_nonbare_repository_dir() instead of resolve_gitlink_ref() so that a\nrepository without any commits is treated like a repository rather\nthan an empty directory.  This makes the reporting from commands like\nls-files consistent for the case shown above, and it also avoids the\ncase where 'git add no-commit' (no trailing slash) adds the untracked\nfiles of the no-commit/ repository to the index of the current\nrepository.\n\n[*]: https://github.com/datalad/datalad/issues/3139#issuecomment-460542647\n\nKyle Meyer (4):\n  submodule: refuse to add repository with no commits\n  t3000: move non-submodule repo test to separate file\n  t3009: test that ls-files -o traverses bogus repo\n  dir: do not traverse repositories with no commits\n\n dir.c                                   |  6 ++-\n git-submodule.sh                        | 12 +++++-\n t/t3000-ls-files-others.sh              |  7 ----\n t/t3009-ls-files-others-nonsubmodule.sh | 56 +++++++++++++++++++++++++\n t/t3700-add.sh                          |  1 +\n t/t7400-submodule-basic.sh              | 11 ++++-\n 6 files changed, 81 insertions(+), 12 deletions(-)\n create mode 100755 t/t3009-ls-files-others-nonsubmodule.sh\n\n-- \n2.21.0\n\n"},{"id":"371509","messageId":"20190314150219.2040-5-kyle@kyleam.com","threadId":"50739","inReplyTo":"20190314150219.2040-1-kyle@kyleam.com","subject":"[PATCH 4/4] dir: do not traverse repositories with no commits","fromName":"Kyle Meyer","fromEmail":"kyle@kyleam.com","sentAt":"2019-03-14T15:02:19Z","receivedAt":"2019-03-14T15:09:40Z","isPatch":true,"sender":{"key":"kyle@kyleam.com","avatar":"https://avatars.githubusercontent.com/u/1297788?v=4"},"body":"When treat_directory() encounters a directory that is not in the index\nand DIR_NO_GITLINKS is unset, it calls resolve_gitlink_ref() to decide\nif a directory looks like a repository, in which case the directory\nwon't be traversed.  As a result, 'status -uall' and 'ls-files -o'\nwill show only the directory, even when there are untracked files\nwithin the directory.\n\nFor the unusual case where a repository doesn't have any commits,\nresolve_gitlink_ref() returns -1 because HEAD cannot be resolved, and\nthe directory is treated as a normal directory (i.e. traversal does\nnot stop at the repository boundary).  The status and ls-files\ncommands above list untracked files within the repository rather than\nshowing only the top-level directory.\n\nThe above case is a corner case in an already unusual situation of the\nworking tree containing a repository that is not a tracked submodule,\nbut we might as well treat anything that looks like a repository\nconsistently.  Loosen the \"looks like a repository\" criteria in\ntreat_directory() by replacing resolve_gitlink_ref() with\nis_nonbare_repository_dir(), one of the checks that is performed\ndownstream when resolve_gitlink_ref() is called with an empty\nrepository.\n\nAs the required update to t3700-add shows, being looser with the check\nmeans that we're stricter when adding empty repositories to the index:\n\n  % git add repo\n  warning: adding embedded git repository: repo\n  hint: You've added another git repository inside your current repository.\n  hint: [...]\n  error: unable to index file 'repo/'\n  fatal: adding files failed\n\nThat error message isn't particularly helpful in this situation, but\nit seems preferable to the old behavior of adding the repository's\nuntracked files.  And if the caller really wants the previous\nbehavior, they can get it by adding a trailing slash.\n\nSigned-off-by: Kyle Meyer <kyle@kyleam.com>\n---\n dir.c                                   |  6 ++++--\n t/t3009-ls-files-others-nonsubmodule.sh | 22 +++++++++++++++++++++-\n t/t3700-add.sh                          |  1 +\n 3 files changed, 26 insertions(+), 3 deletions(-)\n\ndiff --git a/dir.c b/dir.c\nindex b2cabadf25..a4e59eb351 100644\n--- a/dir.c\n+++ b/dir.c\n@@ -1467,9 +1467,11 @@ static enum path_treatment treat_directory(struct dir_struct *dir,\n \t\t\treturn path_none;\n \t\t}\n \t\tif (!(dir->flags & DIR_NO_GITLINKS)) {\n-\t\t\tstruct object_id oid;\n-\t\t\tif (resolve_gitlink_ref(dirname, \"HEAD\", &oid) == 0)\n+\t\t\tstruct strbuf sb = STRBUF_INIT;\n+\t\t\tstrbuf_addstr(&sb, dirname);\n+\t\t\tif (is_nonbare_repository_dir(&sb))\n \t\t\t\treturn exclude ? path_excluded : path_untracked;\n+\t\t\tstrbuf_release(&sb);\n \t\t}\n \t\treturn path_recurse;\n \t}\ndiff --git a/t/t3009-ls-files-others-nonsubmodule.sh b/t/t3009-ls-files-others-nonsubmodule.sh\nindex 9ed75928aa..be4e7e26bc 100755\n--- a/t/t3009-ls-files-others-nonsubmodule.sh\n+++ b/t/t3009-ls-files-others-nonsubmodule.sh\n@@ -8,6 +8,14 @@ This test runs git ls-files --others with the following working tree:\n       directory with no files aside from a bogus .git file\n     repo-bogus-untracked-file/\n       directory with a bogus .git file and another untracked file\n+    repo-no-commit-no-files/\n+      git repository without a commit or a file\n+    repo-no-commit-untracked-file/\n+      git repository without a commit but with an untracked file\n+    repo-with-commit-no-files/\n+      git repository with a commit and no untracked files\n+    repo-with-commit-untracked-file/\n+      git repository with a commit and an untracked file\n '\n \n . ./test-lib.sh\n@@ -17,6 +25,10 @@ test_expect_success 'setup: expected output' '\n \texpected\n \toutput\n \trepo-bogus-untracked-file/untracked\n+\trepo-no-commit-no-files/\n+\trepo-no-commit-untracked-file/\n+\trepo-with-commit-no-files/\n+\trepo-with-commit-untracked-file/\n \tEOF\n '\n \n@@ -25,7 +37,15 @@ test_expect_success 'setup: directories' '\n \techo foo >repo-bogus-no-files/.git &&\n \tmkdir repo-bogus-untracked-file &&\n \techo foo >repo-bogus-untracked-file/.git &&\n-\t: >repo-bogus-untracked-file/untracked\n+\t: >repo-bogus-untracked-file/untracked &&\n+\tgit init repo-no-commit-no-files &&\n+\tgit init repo-no-commit-untracked-file &&\n+\t: >repo-no-commit-untracked-file/untracked &&\n+\tgit init repo-with-commit-no-files &&\n+\tgit -C repo-with-commit-no-files commit --allow-empty -mmsg &&\n+\tgit init repo-with-commit-untracked-file &&\n+\ttest_commit -C repo-with-commit-untracked-file msg &&\n+\t: >repo-with-commit-untracked-file/untracked\n '\n \n test_expect_success 'ls-files --others handles non-submodule .git' '\ndiff --git a/t/t3700-add.sh b/t/t3700-add.sh\nindex be582a513b..5a8425962b 100755\n--- a/t/t3700-add.sh\n+++ b/t/t3700-add.sh\n@@ -396,6 +396,7 @@ test_expect_success 'no file status change if no pathspec is given in subdir' '\n '\n \n test_expect_success 'all statuses changed in folder if . is given' '\n+\trm -fr empty &&\n \tgit add --chmod=+x . &&\n \ttest $(git ls-files --stage | grep ^100644 | wc -l) -eq 0 &&\n \tgit add --chmod=-x . &&\n-- \n2.21.0\n\n"},{"id":"371510","messageId":"20190314150219.2040-4-kyle@kyleam.com","threadId":"50739","inReplyTo":"20190314150219.2040-1-kyle@kyleam.com","subject":"[PATCH 3/4] t3009: test that ls-files -o traverses bogus repo","fromName":"Kyle Meyer","fromEmail":"kyle@kyleam.com","sentAt":"2019-03-14T15:02:18Z","receivedAt":"2019-03-14T15:09:42Z","isPatch":true,"sender":{"key":"kyle@kyleam.com","avatar":"https://avatars.githubusercontent.com/u/1297788?v=4"},"body":"When a2d5156c2b (resolve_gitlink_ref: ignore non-repository paths,\n2016-01-22) added this test, the purpose was to check the 'ls-files\n-o' didn't die() when processing the bogus repository.  The expected\noutput didn't even need to be adjusted for the addition because the\nbogus repository is treated as an empty directory.\n\nLet's add another scenario to test that 'ls-files -o' lists an\nuntracked file within this directory.  Doing so isn't useful for the\noriginal purpose of the test, but it will be helpful for highlighting\nthat this traversal does _not_ happen in the non-bogus scenarios that\nwill be added next.  Rename the original subdirectory to highlight the\ndifference with the new one.\n\nSigned-off-by: Kyle Meyer <kyle@kyleam.com>\n---\n t/t3009-ls-files-others-nonsubmodule.sh | 21 ++++++++++++++++++---\n 1 file changed, 18 insertions(+), 3 deletions(-)\n\ndiff --git a/t/t3009-ls-files-others-nonsubmodule.sh b/t/t3009-ls-files-others-nonsubmodule.sh\nindex cc66a4a14d..9ed75928aa 100755\n--- a/t/t3009-ls-files-others-nonsubmodule.sh\n+++ b/t/t3009-ls-files-others-nonsubmodule.sh\n@@ -1,6 +1,14 @@\n #!/bin/sh\n \n-test_description='test git ls-files --others with non-submodule repositories'\n+test_description='test git ls-files --others with non-submodule repositories\n+\n+This test runs git ls-files --others with the following working tree:\n+\n+    repo-bogus-no-files/\n+      directory with no files aside from a bogus .git file\n+    repo-bogus-untracked-file/\n+      directory with a bogus .git file and another untracked file\n+'\n \n . ./test-lib.sh\n \n@@ -8,12 +16,19 @@ test_expect_success 'setup: expected output' '\n \tcat >expected <<-EOF\n \texpected\n \toutput\n+\trepo-bogus-untracked-file/untracked\n \tEOF\n '\n \n+test_expect_success 'setup: directories' '\n+\tmkdir repo-bogus-no-files &&\n+\techo foo >repo-bogus-no-files/.git &&\n+\tmkdir repo-bogus-untracked-file &&\n+\techo foo >repo-bogus-untracked-file/.git &&\n+\t: >repo-bogus-untracked-file/untracked\n+'\n+\n test_expect_success 'ls-files --others handles non-submodule .git' '\n-\tmkdir not-a-submodule &&\n-\techo foo >not-a-submodule/.git &&\n \tgit ls-files -o >output &&\n \ttest_cmp expected output\n '\n-- \n2.21.0\n\n"},{"id":"371511","messageId":"20190314150219.2040-2-kyle@kyleam.com","threadId":"50739","inReplyTo":"20190314150219.2040-1-kyle@kyleam.com","subject":"[PATCH 1/4] submodule: refuse to add repository with no commits","fromName":"Kyle Meyer","fromEmail":"kyle@kyleam.com","sentAt":"2019-03-14T15:02:16Z","receivedAt":"2019-03-14T15:09:43Z","isPatch":true,"sender":{"key":"kyle@kyleam.com","avatar":"https://avatars.githubusercontent.com/u/1297788?v=4"},"body":"When the path given to 'git submodule add' is an existing repository\nthat is not in the index, the repository is passed to 'git add'.  If\nthis repository doesn't have any commits, we don't get a useful\nresult: there is no subproject OID to track, and any untracked files\nin the sub-repository are added to the current repository.\n\nDetect if the path is a repository with no commits and abort to avoid\ngetting into this state unless --force is used.  Reacting to --force\nisn't very useful, especially because an upcoming commit will make\n'git add' fail in this situation, but it allows us to use the same\n'git add --dry-run' condition as the ignored path case.\n\nSigned-off-by: Kyle Meyer <kyle@kyleam.com>\n---\n git-submodule.sh           | 12 ++++++++++--\n t/t7400-submodule-basic.sh | 11 ++++++++++-\n 2 files changed, 20 insertions(+), 3 deletions(-)\n\ndiff --git a/git-submodule.sh b/git-submodule.sh\nindex 514ede2596..6c74656027 100755\n--- a/git-submodule.sh\n+++ b/git-submodule.sh\n@@ -234,10 +234,18 @@ cmd_add()\n \tif test -z \"$force\" &&\n \t\t! git add --dry-run --ignore-missing --no-warn-embedded-repo \"$sm_path\" > /dev/null 2>&1\n \tthen\n-\t\teval_gettextln \"The following path is ignored by one of your .gitignore files:\n+\t\tif test -d \"$sm_path\" &&\n+\t\t\ttest -z $(git -C \"$sm_path\" rev-parse --show-cdup 2>/dev/null) &&\n+\t\t\t! git -C \"$sm_path\" rev-parse --verify -q HEAD >/dev/null\n+\t\tthen\n+\t\t\tdie \"$(eval_gettext \"'\\$sm_path' does not have any commits\")\"\n+\t\telse\n+\t\t\teval_gettextln \"\\\n+The following path is ignored by one of your .gitignore files:\n \\$sm_path\n Use -f if you really want to add it.\" >&2\n-\t\texit 1\n+\t\t\texit 1\n+\t\tfi\n \tfi\n \n \tif test -n \"$custom_name\"\ndiff --git a/t/t7400-submodule-basic.sh b/t/t7400-submodule-basic.sh\nindex aba2d4d6ee..6adf640143 100755\n--- a/t/t7400-submodule-basic.sh\n+++ b/t/t7400-submodule-basic.sh\n@@ -46,6 +46,15 @@ test_expect_success 'submodule update aborts on missing gitmodules url' '\n \ttest_must_fail git submodule init\n '\n \n+test_expect_success 'add aborts on repository with no commits' '\n+\tcat >expected <<-\\EOF &&\n+\t'\"'repo-no-commits'\"' does not have any commits\n+\tEOF\n+\tgit init repo-no-commits &&\n+\ttest_must_fail git submodule add ../a ./repo-no-commits 2>actual &&\n+\ttest_i18ncmp expected actual\n+'\n+\n test_expect_success 'setup - repository in init subdirectory' '\n \tmkdir init &&\n \t(\n@@ -809,7 +818,7 @@ test_expect_success '../bar/a/b/c works with relative local path - ../foo/bar.gi\n \t\tcp pristine-.git-config .git/config &&\n \t\tcp pristine-.gitmodules .gitmodules &&\n \t\tmkdir -p a/b/c &&\n-\t\t(cd a/b/c && git init) &&\n+\t\t(cd a/b/c && git init && test_commit msg) &&\n \t\tgit config remote.origin.url ../foo/bar.git &&\n \t\tgit submodule add ../bar/a/b/c ./a/b/c &&\n \t\tgit submodule init &&\n-- \n2.21.0\n\n"},{"id":"371656","messageId":"87lg1eq146.fsf@kyleam.com","threadId":"50739","inReplyTo":"20190314150219.2040-2-kyle@kyleam.com","subject":"Re: [PATCH 1/4] submodule: refuse to add repository with no commits","fromName":"Kyle Meyer","fromEmail":"kyle@kyleam.com","sentAt":"2019-03-16T15:40:09Z","receivedAt":"2019-03-16T15:40:19Z","isPatch":true,"sender":{"key":"kyle@kyleam.com","avatar":"https://avatars.githubusercontent.com/u/1297788?v=4"},"body":"Kyle Meyer <kyle@kyleam.com> writes:\n\n[...]\n\n> diff --git a/git-submodule.sh b/git-submodule.sh\n> index 514ede2596..6c74656027 100755\n> --- a/git-submodule.sh\n> +++ b/git-submodule.sh\n> @@ -234,10 +234,18 @@ cmd_add()\n>  \tif test -z \"$force\" &&\n>  \t\t! git add --dry-run --ignore-missing --no-warn-embedded-repo \"$sm_path\" > /dev/null 2>&1\n>  \tthen\n> -\t\teval_gettextln \"The following path is ignored by one of your .gitignore files:\n> +\t\tif test -d \"$sm_path\" &&\n> +\t\t\ttest -z $(git -C \"$sm_path\" rev-parse --show-cdup 2>/dev/null) &&\n> +\t\t\t! git -C \"$sm_path\" rev-parse --verify -q HEAD >/dev/null\n> +\t\tthen\n> +\t\t\tdie \"$(eval_gettext \"'\\$sm_path' does not have any commits\")\"\n> +\t\telse\n> +\t\t\teval_gettextln \"\\\n> +The following path is ignored by one of your .gitignore files:\n>  \\$sm_path\n>  Use -f if you really want to add it.\" >&2\n> -\t\texit 1\n> +\t\t\texit 1\n> +\t\tfi\n\nI didn't think through this check, which would have been obvious had I\nran the added test without the rest of the patches in this series.  It\nassumes that the 'git add --dry-run' call fails, but that failure\ndepends on the last patch of this series.  So I'd need to move this\npatch to the end or find a new place for this \"no commits\" check.\n"},{"id":"372988","messageId":"20190402183505.31512-1-kyle@kyleam.com","threadId":"50739","inReplyTo":"87lg1eq146.fsf@kyleam.com","subject":"[PATCH v2 0/4] dir: Treat a repository without commits as a repository","fromName":"Kyle Meyer","fromEmail":"kyle@kyleam.com","sentAt":"2019-04-02T18:35:01Z","receivedAt":"2019-04-02T18:35:39Z","isPatch":true,"sender":{"key":"kyle@kyleam.com","avatar":"https://avatars.githubusercontent.com/u/1297788?v=4"},"body":"> Kyle Meyer <kyle@kyleam.com> writes:\n>\n> [...]\n>\n>> diff --git a/git-submodule.sh b/git-submodule.sh\n>> index 514ede2596..6c74656027 100755\n>> --- a/git-submodule.sh\n>> +++ b/git-submodule.sh\n>> @@ -234,10 +234,18 @@ cmd_add()\n>>  \tif test -z \"$force\" &&\n>>  \t\t! git add --dry-run --ignore-missing --no-warn-embedded-repo \"$sm_path\" > /dev/null 2>&1\n>>  \tthen\n>> -\t\teval_gettextln \"The following path is ignored by one of your .gitignore files:\n>> +\t\tif test -d \"$sm_path\" &&\n>> +\t\t\ttest -z $(git -C \"$sm_path\" rev-parse --show-cdup 2>/dev/null) &&\n>> +\t\t\t! git -C \"$sm_path\" rev-parse --verify -q HEAD >/dev/null\n>> +\t\tthen\n>> +\t\t\tdie \"$(eval_gettext \"'\\$sm_path' does not have any commits\")\"\n>> +\t\telse\n>> +\t\t\teval_gettextln \"\\\n>> +The following path is ignored by one of your .gitignore files:\n>>  \\$sm_path\n>>  Use -f if you really want to add it.\" >&2\n>> -\t\texit 1\n>> +\t\t\texit 1\n>> +\t\tfi\n>\n> I didn't think through this check, which would have been obvious had I\n> ran the added test without the rest of the patches in this series.  It\n> assumes that the 'git add --dry-run' call fails, but that failure\n> depends on the last patch of this series.  So I'd need to move this\n> patch to the end or find a new place for this \"no commits\" check.\n\nv2 moves the \"no commits\" check outside of the 'git add --dry-run'\nfailure condition so that the first patch doesn't depend on the final\npatch in this series.\n\n\nKyle Meyer (4):\n  submodule: refuse to add repository with no commits\n  t3000: move non-submodule repo test to separate file\n  t3009: test that ls-files -o traverses bogus repo\n  dir: do not traverse repositories with no commits\n\n dir.c                                   |  6 ++-\n git-submodule.sh                        |  7 ++++\n t/t3000-ls-files-others.sh              |  7 ----\n t/t3009-ls-files-others-nonsubmodule.sh | 56 +++++++++++++++++++++++++\n t/t3700-add.sh                          |  1 +\n t/t7400-submodule-basic.sh              | 11 ++++-\n 6 files changed, 78 insertions(+), 10 deletions(-)\n create mode 100755 t/t3009-ls-files-others-nonsubmodule.sh\n\nRange-diff against v1:\n1:  e0db7e3c3c ! 1:  b080e2c557 submodule: refuse to add repository with no commits\n    @@ -9,10 +9,9 @@\n         in the sub-repository are added to the current repository.\n     \n         Detect if the path is a repository with no commits and abort to avoid\n    -    getting into this state unless --force is used.  Reacting to --force\n    -    isn't very useful, especially because an upcoming commit will make\n    -    'git add' fail in this situation, but it allows us to use the same\n    -    'git add --dry-run' condition as the ignored path case.\n    +    getting into this state.  Note that this check must come before the\n    +    'git add --dry-run' check because an upcoming commit will make 'git\n    +    add' fail in this situation.\n     \n         Signed-off-by: Kyle Meyer <kyle@kyleam.com>\n     \n    @@ -20,26 +19,19 @@\n      --- a/git-submodule.sh\n      +++ b/git-submodule.sh\n     @@\n    + \t\tdie \"$(eval_gettext \"'\\$sm_path' already exists in the index and is not a submodule\")\"\n    + \tfi\n    + \n    ++\tif test -d \"$sm_path\" &&\n    ++\t\ttest -z $(git -C \"$sm_path\" rev-parse --show-cdup 2>/dev/null)\n    ++\tthen\n    ++\t    git -C \"$sm_path\" rev-parse --verify -q HEAD >/dev/null ||\n    ++\t    die \"$(eval_gettext \"'\\$sm_path' does not have any commits\")\"\n    ++\tfi\n    ++\n      \tif test -z \"$force\" &&\n      \t\t! git add --dry-run --ignore-missing --no-warn-embedded-repo \"$sm_path\" > /dev/null 2>&1\n      \tthen\n    --\t\teval_gettextln \"The following path is ignored by one of your .gitignore files:\n    -+\t\tif test -d \"$sm_path\" &&\n    -+\t\t\ttest -z $(git -C \"$sm_path\" rev-parse --show-cdup 2>/dev/null) &&\n    -+\t\t\t! git -C \"$sm_path\" rev-parse --verify -q HEAD >/dev/null\n    -+\t\tthen\n    -+\t\t\tdie \"$(eval_gettext \"'\\$sm_path' does not have any commits\")\"\n    -+\t\telse\n    -+\t\t\teval_gettextln \"\\\n    -+The following path is ignored by one of your .gitignore files:\n    - \\$sm_path\n    - Use -f if you really want to add it.\" >&2\n    --\t\texit 1\n    -+\t\t\texit 1\n    -+\t\tfi\n    - \tfi\n    - \n    - \tif test -n \"$custom_name\"\n     \n      diff --git a/t/t7400-submodule-basic.sh b/t/t7400-submodule-basic.sh\n      --- a/t/t7400-submodule-basic.sh\n2:  6eed1f5daf = 2:  c027701842 t3000: move non-submodule repo test to separate file\n3:  7ba3209762 = 3:  97f53e30c0 t3009: test that ls-files -o traverses bogus repo\n4:  2901375dc1 = 4:  a926b87102 dir: do not traverse repositories with no commits\n-- \n2.21.0\n\n"},{"id":"372989","messageId":"20190402183505.31512-2-kyle@kyleam.com","threadId":"50739","inReplyTo":"20190402183505.31512-1-kyle@kyleam.com","subject":"[PATCH v2 1/4] submodule: refuse to add repository with no commits","fromName":"Kyle Meyer","fromEmail":"kyle@kyleam.com","sentAt":"2019-04-02T18:35:02Z","receivedAt":"2019-04-02T18:35:44Z","isPatch":true,"sender":{"key":"kyle@kyleam.com","avatar":"https://avatars.githubusercontent.com/u/1297788?v=4"},"body":"When the path given to 'git submodule add' is an existing repository\nthat is not in the index, the repository is passed to 'git add'.  If\nthis repository doesn't have any commits, we don't get a useful\nresult: there is no subproject OID to track, and any untracked files\nin the sub-repository are added to the current repository.\n\nDetect if the path is a repository with no commits and abort to avoid\ngetting into this state.  Note that this check must come before the\n'git add --dry-run' check because an upcoming commit will make 'git\nadd' fail in this situation.\n\nSigned-off-by: Kyle Meyer <kyle@kyleam.com>\n---\n git-submodule.sh           |  7 +++++++\n t/t7400-submodule-basic.sh | 11 ++++++++++-\n 2 files changed, 17 insertions(+), 1 deletion(-)\n\ndiff --git a/git-submodule.sh b/git-submodule.sh\nindex 514ede2596..5a9dc9bf7e 100755\n--- a/git-submodule.sh\n+++ b/git-submodule.sh\n@@ -231,6 +231,13 @@ cmd_add()\n \t\tdie \"$(eval_gettext \"'\\$sm_path' already exists in the index and is not a submodule\")\"\n \tfi\n \n+\tif test -d \"$sm_path\" &&\n+\t\ttest -z $(git -C \"$sm_path\" rev-parse --show-cdup 2>/dev/null)\n+\tthen\n+\t    git -C \"$sm_path\" rev-parse --verify -q HEAD >/dev/null ||\n+\t    die \"$(eval_gettext \"'\\$sm_path' does not have any commits\")\"\n+\tfi\n+\n \tif test -z \"$force\" &&\n \t\t! git add --dry-run --ignore-missing --no-warn-embedded-repo \"$sm_path\" > /dev/null 2>&1\n \tthen\ndiff --git a/t/t7400-submodule-basic.sh b/t/t7400-submodule-basic.sh\nindex aba2d4d6ee..6adf640143 100755\n--- a/t/t7400-submodule-basic.sh\n+++ b/t/t7400-submodule-basic.sh\n@@ -46,6 +46,15 @@ test_expect_success 'submodule update aborts on missing gitmodules url' '\n \ttest_must_fail git submodule init\n '\n \n+test_expect_success 'add aborts on repository with no commits' '\n+\tcat >expected <<-\\EOF &&\n+\t'\"'repo-no-commits'\"' does not have any commits\n+\tEOF\n+\tgit init repo-no-commits &&\n+\ttest_must_fail git submodule add ../a ./repo-no-commits 2>actual &&\n+\ttest_i18ncmp expected actual\n+'\n+\n test_expect_success 'setup - repository in init subdirectory' '\n \tmkdir init &&\n \t(\n@@ -809,7 +818,7 @@ test_expect_success '../bar/a/b/c works with relative local path - ../foo/bar.gi\n \t\tcp pristine-.git-config .git/config &&\n \t\tcp pristine-.gitmodules .gitmodules &&\n \t\tmkdir -p a/b/c &&\n-\t\t(cd a/b/c && git init) &&\n+\t\t(cd a/b/c && git init && test_commit msg) &&\n \t\tgit config remote.origin.url ../foo/bar.git &&\n \t\tgit submodule add ../bar/a/b/c ./a/b/c &&\n \t\tgit submodule init &&\n-- \n2.21.0\n\n"},{"id":"372990","messageId":"20190402183505.31512-3-kyle@kyleam.com","threadId":"50739","inReplyTo":"20190402183505.31512-1-kyle@kyleam.com","subject":"[PATCH v2 2/4] t3000: move non-submodule repo test to separate file","fromName":"Kyle Meyer","fromEmail":"kyle@kyleam.com","sentAt":"2019-04-02T18:35:03Z","receivedAt":"2019-04-02T18:35:59Z","isPatch":true,"sender":{"key":"kyle@kyleam.com","avatar":"https://avatars.githubusercontent.com/u/1297788?v=4"},"body":"a2d5156c2b (resolve_gitlink_ref: ignore non-repository paths,\n2016-01-22) added a test to t3000-ls-files-others.sh to check that\n'ls-files -o' does not die() when given a subdirectory that looks like\na repository but is actually a subdirectory containing a bogus .git\nfile.\n\nMove this test to a separate file in preparation for testing scenarios\nwith non-submodule repositories that are not bogus.\n\nSigned-off-by: Kyle Meyer <kyle@kyleam.com>\n---\n t/t3000-ls-files-others.sh              |  7 -------\n t/t3009-ls-files-others-nonsubmodule.sh | 21 +++++++++++++++++++++\n 2 files changed, 21 insertions(+), 7 deletions(-)\n create mode 100755 t/t3009-ls-files-others-nonsubmodule.sh\n\ndiff --git a/t/t3000-ls-files-others.sh b/t/t3000-ls-files-others.sh\nindex afd4756134..b4f9fc4580 100755\n--- a/t/t3000-ls-files-others.sh\n+++ b/t/t3000-ls-files-others.sh\n@@ -65,13 +65,6 @@ test_expect_success '--no-empty-directory hides empty directory' '\n \ttest_cmp expected3 output\n '\n \n-test_expect_success 'ls-files --others handles non-submodule .git' '\n-\tmkdir not-a-submodule &&\n-\techo foo >not-a-submodule/.git &&\n-\tgit ls-files -o >output &&\n-\ttest_cmp expected1 output\n-'\n-\n test_expect_success SYMLINKS 'ls-files --others with symlinked submodule' '\n \tgit init super &&\n \tgit init sub &&\ndiff --git a/t/t3009-ls-files-others-nonsubmodule.sh b/t/t3009-ls-files-others-nonsubmodule.sh\nnew file mode 100755\nindex 0000000000..cc66a4a14d\n--- /dev/null\n+++ b/t/t3009-ls-files-others-nonsubmodule.sh\n@@ -0,0 +1,21 @@\n+#!/bin/sh\n+\n+test_description='test git ls-files --others with non-submodule repositories'\n+\n+. ./test-lib.sh\n+\n+test_expect_success 'setup: expected output' '\n+\tcat >expected <<-EOF\n+\texpected\n+\toutput\n+\tEOF\n+'\n+\n+test_expect_success 'ls-files --others handles non-submodule .git' '\n+\tmkdir not-a-submodule &&\n+\techo foo >not-a-submodule/.git &&\n+\tgit ls-files -o >output &&\n+\ttest_cmp expected output\n+'\n+\n+test_done\n-- \n2.21.0\n\n"},{"id":"372991","messageId":"20190402183505.31512-4-kyle@kyleam.com","threadId":"50739","inReplyTo":"20190402183505.31512-1-kyle@kyleam.com","subject":"[PATCH v2 3/4] t3009: test that ls-files -o traverses bogus repo","fromName":"Kyle Meyer","fromEmail":"kyle@kyleam.com","sentAt":"2019-04-02T18:35:04Z","receivedAt":"2019-04-02T18:36:02Z","isPatch":true,"sender":{"key":"kyle@kyleam.com","avatar":"https://avatars.githubusercontent.com/u/1297788?v=4"},"body":"When a2d5156c2b (resolve_gitlink_ref: ignore non-repository paths,\n2016-01-22) added this test, the purpose was to check the 'ls-files\n-o' didn't die() when processing the bogus repository.  The expected\noutput didn't even need to be adjusted for the addition because the\nbogus repository is treated as an empty directory.\n\nLet's add another scenario to test that 'ls-files -o' lists an\nuntracked file within this directory.  Doing so isn't useful for the\noriginal purpose of the test, but it will be helpful for highlighting\nthat this traversal does _not_ happen in the non-bogus scenarios that\nwill be added next.  Rename the original subdirectory to highlight the\ndifference with the new one.\n\nSigned-off-by: Kyle Meyer <kyle@kyleam.com>\n---\n t/t3009-ls-files-others-nonsubmodule.sh | 21 ++++++++++++++++++---\n 1 file changed, 18 insertions(+), 3 deletions(-)\n\ndiff --git a/t/t3009-ls-files-others-nonsubmodule.sh b/t/t3009-ls-files-others-nonsubmodule.sh\nindex cc66a4a14d..9ed75928aa 100755\n--- a/t/t3009-ls-files-others-nonsubmodule.sh\n+++ b/t/t3009-ls-files-others-nonsubmodule.sh\n@@ -1,6 +1,14 @@\n #!/bin/sh\n \n-test_description='test git ls-files --others with non-submodule repositories'\n+test_description='test git ls-files --others with non-submodule repositories\n+\n+This test runs git ls-files --others with the following working tree:\n+\n+    repo-bogus-no-files/\n+      directory with no files aside from a bogus .git file\n+    repo-bogus-untracked-file/\n+      directory with a bogus .git file and another untracked file\n+'\n \n . ./test-lib.sh\n \n@@ -8,12 +16,19 @@ test_expect_success 'setup: expected output' '\n \tcat >expected <<-EOF\n \texpected\n \toutput\n+\trepo-bogus-untracked-file/untracked\n \tEOF\n '\n \n+test_expect_success 'setup: directories' '\n+\tmkdir repo-bogus-no-files &&\n+\techo foo >repo-bogus-no-files/.git &&\n+\tmkdir repo-bogus-untracked-file &&\n+\techo foo >repo-bogus-untracked-file/.git &&\n+\t: >repo-bogus-untracked-file/untracked\n+'\n+\n test_expect_success 'ls-files --others handles non-submodule .git' '\n-\tmkdir not-a-submodule &&\n-\techo foo >not-a-submodule/.git &&\n \tgit ls-files -o >output &&\n \ttest_cmp expected output\n '\n-- \n2.21.0\n\n"},{"id":"372992","messageId":"20190402183505.31512-5-kyle@kyleam.com","threadId":"50739","inReplyTo":"20190402183505.31512-1-kyle@kyleam.com","subject":"[PATCH v2 4/4] dir: do not traverse repositories with no commits","fromName":"Kyle Meyer","fromEmail":"kyle@kyleam.com","sentAt":"2019-04-02T18:35:05Z","receivedAt":"2019-04-02T18:36:08Z","isPatch":true,"sender":{"key":"kyle@kyleam.com","avatar":"https://avatars.githubusercontent.com/u/1297788?v=4"},"body":"When treat_directory() encounters a directory that is not in the index\nand DIR_NO_GITLINKS is unset, it calls resolve_gitlink_ref() to decide\nif a directory looks like a repository, in which case the directory\nwon't be traversed.  As a result, 'status -uall' and 'ls-files -o'\nwill show only the directory, even when there are untracked files\nwithin the directory.\n\nFor the unusual case where a repository doesn't have any commits,\nresolve_gitlink_ref() returns -1 because HEAD cannot be resolved, and\nthe directory is treated as a normal directory (i.e. traversal does\nnot stop at the repository boundary).  The status and ls-files\ncommands above list untracked files within the repository rather than\nshowing only the top-level directory.\n\nThe above case is a corner case in an already unusual situation of the\nworking tree containing a repository that is not a tracked submodule,\nbut we might as well treat anything that looks like a repository\nconsistently.  Loosen the \"looks like a repository\" criteria in\ntreat_directory() by replacing resolve_gitlink_ref() with\nis_nonbare_repository_dir(), one of the checks that is performed\ndownstream when resolve_gitlink_ref() is called with an empty\nrepository.\n\nAs the required update to t3700-add shows, being looser with the check\nmeans that we're stricter when adding empty repositories to the index:\n\n  % git add repo\n  warning: adding embedded git repository: repo\n  hint: You've added another git repository inside your current repository.\n  hint: [...]\n  error: unable to index file 'repo/'\n  fatal: adding files failed\n\nThat error message isn't particularly helpful in this situation, but\nit seems preferable to the old behavior of adding the repository's\nuntracked files.  And if the caller really wants the previous\nbehavior, they can get it by adding a trailing slash.\n\nSigned-off-by: Kyle Meyer <kyle@kyleam.com>\n---\n dir.c                                   |  6 ++++--\n t/t3009-ls-files-others-nonsubmodule.sh | 22 +++++++++++++++++++++-\n t/t3700-add.sh                          |  1 +\n 3 files changed, 26 insertions(+), 3 deletions(-)\n\ndiff --git a/dir.c b/dir.c\nindex b2cabadf25..a4e59eb351 100644\n--- a/dir.c\n+++ b/dir.c\n@@ -1467,9 +1467,11 @@ static enum path_treatment treat_directory(struct dir_struct *dir,\n \t\t\treturn path_none;\n \t\t}\n \t\tif (!(dir->flags & DIR_NO_GITLINKS)) {\n-\t\t\tstruct object_id oid;\n-\t\t\tif (resolve_gitlink_ref(dirname, \"HEAD\", &oid) == 0)\n+\t\t\tstruct strbuf sb = STRBUF_INIT;\n+\t\t\tstrbuf_addstr(&sb, dirname);\n+\t\t\tif (is_nonbare_repository_dir(&sb))\n \t\t\t\treturn exclude ? path_excluded : path_untracked;\n+\t\t\tstrbuf_release(&sb);\n \t\t}\n \t\treturn path_recurse;\n \t}\ndiff --git a/t/t3009-ls-files-others-nonsubmodule.sh b/t/t3009-ls-files-others-nonsubmodule.sh\nindex 9ed75928aa..be4e7e26bc 100755\n--- a/t/t3009-ls-files-others-nonsubmodule.sh\n+++ b/t/t3009-ls-files-others-nonsubmodule.sh\n@@ -8,6 +8,14 @@ This test runs git ls-files --others with the following working tree:\n       directory with no files aside from a bogus .git file\n     repo-bogus-untracked-file/\n       directory with a bogus .git file and another untracked file\n+    repo-no-commit-no-files/\n+      git repository without a commit or a file\n+    repo-no-commit-untracked-file/\n+      git repository without a commit but with an untracked file\n+    repo-with-commit-no-files/\n+      git repository with a commit and no untracked files\n+    repo-with-commit-untracked-file/\n+      git repository with a commit and an untracked file\n '\n \n . ./test-lib.sh\n@@ -17,6 +25,10 @@ test_expect_success 'setup: expected output' '\n \texpected\n \toutput\n \trepo-bogus-untracked-file/untracked\n+\trepo-no-commit-no-files/\n+\trepo-no-commit-untracked-file/\n+\trepo-with-commit-no-files/\n+\trepo-with-commit-untracked-file/\n \tEOF\n '\n \n@@ -25,7 +37,15 @@ test_expect_success 'setup: directories' '\n \techo foo >repo-bogus-no-files/.git &&\n \tmkdir repo-bogus-untracked-file &&\n \techo foo >repo-bogus-untracked-file/.git &&\n-\t: >repo-bogus-untracked-file/untracked\n+\t: >repo-bogus-untracked-file/untracked &&\n+\tgit init repo-no-commit-no-files &&\n+\tgit init repo-no-commit-untracked-file &&\n+\t: >repo-no-commit-untracked-file/untracked &&\n+\tgit init repo-with-commit-no-files &&\n+\tgit -C repo-with-commit-no-files commit --allow-empty -mmsg &&\n+\tgit init repo-with-commit-untracked-file &&\n+\ttest_commit -C repo-with-commit-untracked-file msg &&\n+\t: >repo-with-commit-untracked-file/untracked\n '\n \n test_expect_success 'ls-files --others handles non-submodule .git' '\ndiff --git a/t/t3700-add.sh b/t/t3700-add.sh\nindex be582a513b..5a8425962b 100755\n--- a/t/t3700-add.sh\n+++ b/t/t3700-add.sh\n@@ -396,6 +396,7 @@ test_expect_success 'no file status change if no pathspec is given in subdir' '\n '\n \n test_expect_success 'all statuses changed in folder if . is given' '\n+\trm -fr empty &&\n \tgit add --chmod=+x . &&\n \ttest $(git ls-files --stage | grep ^100644 | wc -l) -eq 0 &&\n \tgit add --chmod=-x . &&\n-- \n2.21.0\n\n"},{"id":"373006","messageId":"xmqqlg0rsent.fsf@gitster-ct.c.googlers.com","threadId":"50739","inReplyTo":"20190402183505.31512-3-kyle@kyleam.com","subject":"Re: [PATCH v2 2/4] t3000: move non-submodule repo test to separate file","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2019-04-03T07:59:02Z","receivedAt":"2019-04-03T07:59:07Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Kyle Meyer <kyle@kyleam.com> writes:\n\n> a2d5156c2b (resolve_gitlink_ref: ignore non-repository paths,\n> 2016-01-22) added a test to t3000-ls-files-others.sh to check that\n> 'ls-files -o' does not die() when given a subdirectory that looks like\n> a repository but is actually a subdirectory containing a bogus .git\n> file.\n>\n> Move this test to a separate file in preparation for testing scenarios\n> with non-submodule repositories that are not bogus.\n\nIt is unclear to me why this is needed.\n\n> +++ b/t/t3009-ls-files-others-nonsubmodule.sh\n> @@ -0,0 +1,21 @@\n> +#!/bin/sh\n> +\n> +test_description='test git ls-files --others with non-submodule repositories'\n> +\n> +. ./test-lib.sh\n> +\n> +test_expect_success 'setup: expected output' '\n> +\tcat >expected <<-EOF\n> +\texpected\n> +\toutput\n> +\tEOF\n> +'\n\nI think this is overkill.  Usually we have one expectation for a\nsingle test, so having the above inside the actual test below makes\nmore sense.\n\nOr are you planning to add more tests before the test_done we see\nbelow, all of which expect the above output?  It would make perfect\nsense if it were the case, but I do not think that is what is\nhappenning here...\n\n> +test_expect_success 'ls-files --others handles non-submodule .git' '\n> +\tmkdir not-a-submodule &&\n> +\techo foo >not-a-submodule/.git &&\n> +\tgit ls-files -o >output &&\n> +\ttest_cmp expected output\n> +'\n> +\n> +test_done\n"},{"id":"373007","messageId":"xmqqef6jsedj.fsf@gitster-ct.c.googlers.com","threadId":"50739","inReplyTo":"20190402183505.31512-5-kyle@kyleam.com","subject":"Re: [PATCH v2 4/4] dir: do not traverse repositories with no commits","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2019-04-03T08:05:12Z","receivedAt":"2019-04-03T08:05:17Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Kyle Meyer <kyle@kyleam.com> writes:\n\n> The above case is a corner case in an already unusual situation of the\n> working tree containing a repository that is not a tracked submodule,\n> but we might as well treat anything that looks like a repository\n> consistently.\n\nMakes sense.\n\n> As the required update to t3700-add shows, being looser with the check\n> means that we're stricter when adding empty repositories to the index:\n>\n>   % git add repo\n>   warning: adding embedded git repository: repo\n>   hint: You've added another git repository inside your current repository.\n>   hint: [...]\n>   error: unable to index file 'repo/'\n>   fatal: adding files failed\n\nHmph, why should we have the initial \"warning\" with hint?  Shouldn't\nthe above result in an outright error?  Something like:\n\n    $ git add repo ;# or git add repo/\n    error: repo does not have a commit checked out\n    fatal: adding files failed\n\nbecause the entire purpose of \"git add repo\" (or \"git add repo/\") when\n\"repo\" is a repository is to register the commit that is checked out\nin that sub-repository to the index of the top-level repository, as\npart of updating (or starting) a submodule.\n\nI very much like the direction these four patches want to take us.\nI just have small troubles here and there in their execution, though\n;-)\n\nThanks.\n"},{"id":"373078","messageId":"87d0m2bui0.fsf@kyleam.com","threadId":"50739","inReplyTo":"xmqqlg0rsent.fsf@gitster-ct.c.googlers.com","subject":"Re: [PATCH v2 2/4] t3000: move non-submodule repo test to separate file","fromName":"Kyle Meyer","fromEmail":"kyle@kyleam.com","sentAt":"2019-04-03T22:21:11Z","receivedAt":"2019-04-03T22:21:16Z","isPatch":true,"sender":{"key":"kyle@kyleam.com","avatar":"https://avatars.githubusercontent.com/u/1297788?v=4"},"body":"Junio C Hamano <gitster@pobox.com> writes:\n\n> Kyle Meyer <kyle@kyleam.com> writes:\n>\n>> a2d5156c2b (resolve_gitlink_ref: ignore non-repository paths,\n>> 2016-01-22) added a test to t3000-ls-files-others.sh to check that\n>> 'ls-files -o' does not die() when given a subdirectory that looks like\n>> a repository but is actually a subdirectory containing a bogus .git\n>> file.\n>>\n>> Move this test to a separate file in preparation for testing scenarios\n>> with non-submodule repositories that are not bogus.\n>\n> It is unclear to me why this is needed.\n\nIt's not needed.  My thinking, which I didn't do a good job of spelling\nout above, is\n\n    We're going to be adding a test that checks how 'ls-files -o'\n    handles a few different scenarios involving untracked repositories.\n    This new test should go into a separate file rather than\n    t3000-ls-files-others.sh because it substantially changes the shared\n    directory layout that the t3000 tests work on.  Like the upcoming\n    test, the \"non-submodule .git\" test from t3000 deals with a (bogus)\n    untracked repository, so let's split it off into a separate test\n    file that will be extended with the other scenarios.\n\nPerhaps that's not a good reason to touch t3000, though.  I could drop\nthis patch, as well as the next one, and just add the new test file in\nthe final patch.\n\n>> +++ b/t/t3009-ls-files-others-nonsubmodule.sh\n>> @@ -0,0 +1,21 @@\n>> +#!/bin/sh\n>> +\n>> +test_description='test git ls-files --others with non-submodule repositories'\n>> +\n>> +. ./test-lib.sh\n>> +\n>> +test_expect_success 'setup: expected output' '\n>> +\tcat >expected <<-EOF\n>> +\texpected\n>> +\toutput\n>> +\tEOF\n>> +'\n>\n> I think this is overkill.  Usually we have one expectation for a\n> single test, so having the above inside the actual test below makes\n> more sense.\n\nOK, I'll move this into the test.\n"},{"id":"373079","messageId":"87bm1mbua4.fsf@kyleam.com","threadId":"50739","inReplyTo":"xmqqef6jsedj.fsf@gitster-ct.c.googlers.com","subject":"Re: [PATCH v2 4/4] dir: do not traverse repositories with no commits","fromName":"Kyle Meyer","fromEmail":"kyle@kyleam.com","sentAt":"2019-04-03T22:25:55Z","receivedAt":"2019-04-03T22:26:00Z","isPatch":true,"sender":{"key":"kyle@kyleam.com","avatar":"https://avatars.githubusercontent.com/u/1297788?v=4"},"body":"Junio C Hamano <gitster@pobox.com> writes:\n\n> Kyle Meyer <kyle@kyleam.com> writes:\n\n>> As the required update to t3700-add shows, being looser with the check\n>> means that we're stricter when adding empty repositories to the index:\n>>\n>>   % git add repo\n>>   warning: adding embedded git repository: repo\n>>   hint: You've added another git repository inside your current repository.\n>>   hint: [...]\n>>   error: unable to index file 'repo/'\n>>   fatal: adding files failed\n>\n> Hmph, why should we have the initial \"warning\" with hint?  Shouldn't\n> the above result in an outright error?  Something like:\n>\n>     $ git add repo ;# or git add repo/\n>     error: repo does not have a commit checked out\n>     fatal: adding files failed\n>\n> because the entire purpose of \"git add repo\" (or \"git add repo/\") when\n> \"repo\" is a repository is to register the commit that is checked out\n> in that sub-repository to the index of the top-level repository, as\n> part of updating (or starting) a submodule.\n\nRight, the hint makes no sense here.  I'll look into adjusting the\noutput to be appropriate.\n\nAs a minor note: I believe your \"git add repo/\" comments above describe\nwhat should be and not what currently is.  The trailing slash results in\nadding the content from the specified repository to the current\nrepository rather than registering the repository as a submodule:\n\n  $ git --version\n  git version 2.21.0\n  $ git init\n  $ git init a\n  $ git -C a commit --allow-empty -mmsg\n  $ touch a/f\n  $ git add a/\n  $ git status -s\n  A  a/f\n\nThis behavior (and fixing it) was discussed at\nhttps://public-inbox.org/git/20180618111919.GA10085@book.hvoigt.net\n"},{"id":"373092","messageId":"xmqqv9zufd1d.fsf@gitster-ct.c.googlers.com","threadId":"50739","inReplyTo":"20190402183505.31512-2-kyle@kyleam.com","subject":"Re: [PATCH v2 1/4] submodule: refuse to add repository with no commits","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2019-04-04T07:24:46Z","receivedAt":"2019-04-04T07:24:53Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Kyle Meyer <kyle@kyleam.com> writes:\n\n> When the path given to 'git submodule add' is an existing repository\n> that is not in the index, the repository is passed to 'git add'.  If\n> this repository doesn't have any commits, we don't get a useful\n> result: there is no subproject OID to track, and any untracked files\n> in the sub-repository are added to the current repository.\n>\n> Detect if the path is a repository with no commits and abort to avoid\n> getting into this state.  Note that this check must come before the\n> 'git add --dry-run' check because an upcoming commit will make 'git\n> add' fail in this situation.\n\nOK, in short, in an already-populated repository,\n\n\t$ git init junk ;# there was no 'junk' subdirectory\n\t$ git add junk\n\nwould have been a successful no-op, and after the above two\ncommands,\n\n\t$ >junk/one\n\t$ git add junk\n\nwould have added junk/one as a normal blob to the top-level project,\nignoring the fact that the user wanted to eventually populate junk\nas a repository and add it as a submodule.  You'll be teaching \"git\nadd\" not to do so, and pay attention to the fact that junk/.git exists,\neven though junk/.git/HEAD is on an unborn branch.\n\nWhich makes sense to me.\n\n> Signed-off-by: Kyle Meyer <kyle@kyleam.com>\n> ---\n>  git-submodule.sh           |  7 +++++++\n>  t/t7400-submodule-basic.sh | 11 ++++++++++-\n>  2 files changed, 17 insertions(+), 1 deletion(-)\n>\n> diff --git a/git-submodule.sh b/git-submodule.sh\n> index 514ede2596..5a9dc9bf7e 100755\n> --- a/git-submodule.sh\n> +++ b/git-submodule.sh\n> @@ -231,6 +231,13 @@ cmd_add()\n>  \t\tdie \"$(eval_gettext \"'\\$sm_path' already exists in the index and is not a submodule\")\"\n>  \tfi\n>  \n> +\tif test -d \"$sm_path\" &&\n> +\t\ttest -z $(git -C \"$sm_path\" rev-parse --show-cdup 2>/dev/null)\n> +\tthen\n> +\t    git -C \"$sm_path\" rev-parse --verify -q HEAD >/dev/null ||\n> +\t    die \"$(eval_gettext \"'\\$sm_path' does not have any commits\")\"\n> +\tfi\n> +\n>  \tif test -z \"$force\" &&\n>  \t\t! git add --dry-run --ignore-missing --no-warn-embedded-repo \"$sm_path\" > /dev/null 2>&1\n>  \tthen\n> diff --git a/t/t7400-submodule-basic.sh b/t/t7400-submodule-basic.sh\n> index aba2d4d6ee..6adf640143 100755\n> --- a/t/t7400-submodule-basic.sh\n> +++ b/t/t7400-submodule-basic.sh\n> @@ -46,6 +46,15 @@ test_expect_success 'submodule update aborts on missing gitmodules url' '\n>  \ttest_must_fail git submodule init\n>  '\n>  \n> +test_expect_success 'add aborts on repository with no commits' '\n> +\tcat >expected <<-\\EOF &&\n> +\t'\"'repo-no-commits'\"' does not have any commits\n> +\tEOF\n> +\tgit init repo-no-commits &&\n> +\ttest_must_fail git submodule add ../a ./repo-no-commits 2>actual &&\n> +\ttest_i18ncmp expected actual\n> +'\n> +\n>  test_expect_success 'setup - repository in init subdirectory' '\n>  \tmkdir init &&\n>  \t(\n> @@ -809,7 +818,7 @@ test_expect_success '../bar/a/b/c works with relative local path - ../foo/bar.gi\n>  \t\tcp pristine-.git-config .git/config &&\n>  \t\tcp pristine-.gitmodules .gitmodules &&\n>  \t\tmkdir -p a/b/c &&\n> -\t\t(cd a/b/c && git init) &&\n> +\t\t(cd a/b/c && git init && test_commit msg) &&\n>  \t\tgit config remote.origin.url ../foo/bar.git &&\n>  \t\tgit submodule add ../bar/a/b/c ./a/b/c &&\n>  \t\tgit submodule init &&\n"}]}