git@vger.kernel.org mailing list mirror (one of many)
 help / color / mirror / code / Atom feed
From: Kyle Meyer <kyle@kyleam.com>
To: Kyle Meyer <kyle@kyleam.com>, git@vger.kernel.org
Cc: debian@onerussian.com
Subject: [PATCH v2 4/4] dir: do not traverse repositories with no commits
Date: Tue,  2 Apr 2019 14:35:05 -0400	[thread overview]
Message-ID: <20190402183505.31512-5-kyle@kyleam.com> (raw)
In-Reply-To: <20190402183505.31512-1-kyle@kyleam.com>

When treat_directory() encounters a directory that is not in the index
and DIR_NO_GITLINKS is unset, it calls resolve_gitlink_ref() to decide
if a directory looks like a repository, in which case the directory
won't be traversed.  As a result, 'status -uall' and 'ls-files -o'
will show only the directory, even when there are untracked files
within the directory.

For the unusual case where a repository doesn't have any commits,
resolve_gitlink_ref() returns -1 because HEAD cannot be resolved, and
the directory is treated as a normal directory (i.e. traversal does
not stop at the repository boundary).  The status and ls-files
commands above list untracked files within the repository rather than
showing only the top-level directory.

The above case is a corner case in an already unusual situation of the
working tree containing a repository that is not a tracked submodule,
but we might as well treat anything that looks like a repository
consistently.  Loosen the "looks like a repository" criteria in
treat_directory() by replacing resolve_gitlink_ref() with
is_nonbare_repository_dir(), one of the checks that is performed
downstream when resolve_gitlink_ref() is called with an empty
repository.

As the required update to t3700-add shows, being looser with the check
means that we're stricter when adding empty repositories to the index:

  % git add repo
  warning: adding embedded git repository: repo
  hint: You've added another git repository inside your current repository.
  hint: [...]
  error: unable to index file 'repo/'
  fatal: adding files failed

That error message isn't particularly helpful in this situation, but
it seems preferable to the old behavior of adding the repository's
untracked files.  And if the caller really wants the previous
behavior, they can get it by adding a trailing slash.

Signed-off-by: Kyle Meyer <kyle@kyleam.com>
---
 dir.c                                   |  6 ++++--
 t/t3009-ls-files-others-nonsubmodule.sh | 22 +++++++++++++++++++++-
 t/t3700-add.sh                          |  1 +
 3 files changed, 26 insertions(+), 3 deletions(-)

diff --git a/dir.c b/dir.c
index b2cabadf25..a4e59eb351 100644
--- a/dir.c
+++ b/dir.c
@@ -1467,9 +1467,11 @@ static enum path_treatment treat_directory(struct dir_struct *dir,
 			return path_none;
 		}
 		if (!(dir->flags & DIR_NO_GITLINKS)) {
-			struct object_id oid;
-			if (resolve_gitlink_ref(dirname, "HEAD", &oid) == 0)
+			struct strbuf sb = STRBUF_INIT;
+			strbuf_addstr(&sb, dirname);
+			if (is_nonbare_repository_dir(&sb))
 				return exclude ? path_excluded : path_untracked;
+			strbuf_release(&sb);
 		}
 		return path_recurse;
 	}
diff --git a/t/t3009-ls-files-others-nonsubmodule.sh b/t/t3009-ls-files-others-nonsubmodule.sh
index 9ed75928aa..be4e7e26bc 100755
--- a/t/t3009-ls-files-others-nonsubmodule.sh
+++ b/t/t3009-ls-files-others-nonsubmodule.sh
@@ -8,6 +8,14 @@ This test runs git ls-files --others with the following working tree:
       directory with no files aside from a bogus .git file
     repo-bogus-untracked-file/
       directory with a bogus .git file and another untracked file
+    repo-no-commit-no-files/
+      git repository without a commit or a file
+    repo-no-commit-untracked-file/
+      git repository without a commit but with an untracked file
+    repo-with-commit-no-files/
+      git repository with a commit and no untracked files
+    repo-with-commit-untracked-file/
+      git repository with a commit and an untracked file
 '
 
 . ./test-lib.sh
@@ -17,6 +25,10 @@ test_expect_success 'setup: expected output' '
 	expected
 	output
 	repo-bogus-untracked-file/untracked
+	repo-no-commit-no-files/
+	repo-no-commit-untracked-file/
+	repo-with-commit-no-files/
+	repo-with-commit-untracked-file/
 	EOF
 '
 
@@ -25,7 +37,15 @@ test_expect_success 'setup: directories' '
 	echo foo >repo-bogus-no-files/.git &&
 	mkdir repo-bogus-untracked-file &&
 	echo foo >repo-bogus-untracked-file/.git &&
-	: >repo-bogus-untracked-file/untracked
+	: >repo-bogus-untracked-file/untracked &&
+	git init repo-no-commit-no-files &&
+	git init repo-no-commit-untracked-file &&
+	: >repo-no-commit-untracked-file/untracked &&
+	git init repo-with-commit-no-files &&
+	git -C repo-with-commit-no-files commit --allow-empty -mmsg &&
+	git init repo-with-commit-untracked-file &&
+	test_commit -C repo-with-commit-untracked-file msg &&
+	: >repo-with-commit-untracked-file/untracked
 '
 
 test_expect_success 'ls-files --others handles non-submodule .git' '
diff --git a/t/t3700-add.sh b/t/t3700-add.sh
index be582a513b..5a8425962b 100755
--- a/t/t3700-add.sh
+++ b/t/t3700-add.sh
@@ -396,6 +396,7 @@ test_expect_success 'no file status change if no pathspec is given in subdir' '
 '
 
 test_expect_success 'all statuses changed in folder if . is given' '
+	rm -fr empty &&
 	git add --chmod=+x . &&
 	test $(git ls-files --stage | grep ^100644 | wc -l) -eq 0 &&
 	git add --chmod=-x . &&
-- 
2.21.0


  parent reply	other threads:[~2019-04-02 18:36 UTC|newest]

Thread overview: 16+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2019-03-14 15:02 [PATCH 0/4] dir: Treat a repository without commits as a repository Kyle Meyer
2019-03-14 15:02 ` [PATCH 1/4] submodule: refuse to add repository with no commits Kyle Meyer
2019-03-16 15:40   ` Kyle Meyer
2019-04-02 18:35     ` [PATCH v2 0/4] dir: Treat a repository without commits as a repository Kyle Meyer
2019-04-02 18:35       ` [PATCH v2 1/4] submodule: refuse to add repository with no commits Kyle Meyer
2019-04-04  7:24         ` Junio C Hamano
2019-04-02 18:35       ` [PATCH v2 2/4] t3000: move non-submodule repo test to separate file Kyle Meyer
2019-04-03  7:59         ` Junio C Hamano
2019-04-03 22:21           ` Kyle Meyer
2019-04-02 18:35       ` [PATCH v2 3/4] t3009: test that ls-files -o traverses bogus repo Kyle Meyer
2019-04-02 18:35       ` Kyle Meyer [this message]
2019-04-03  8:05         ` [PATCH v2 4/4] dir: do not traverse repositories with no commits Junio C Hamano
2019-04-03 22:25           ` Kyle Meyer
2019-03-14 15:02 ` [PATCH 2/4] t3000: move non-submodule repo test to separate file Kyle Meyer
2019-03-14 15:02 ` [PATCH 3/4] t3009: test that ls-files -o traverses bogus repo Kyle Meyer
2019-03-14 15:02 ` [PATCH 4/4] dir: do not traverse repositories with no commits Kyle Meyer

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

  List information: http://vger.kernel.org/majordomo-info.html

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20190402183505.31512-5-kyle@kyleam.com \
    --to=kyle@kyleam.com \
    --cc=debian@onerussian.com \
    --cc=git@vger.kernel.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.
Code repositories for project(s) associated with this public inbox

	https://80x24.org/mirrors/git.git

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).