Am 10.04.2010 01:11, schrieb Jens Lehmann: > Am 09.04.2010 23:59, schrieb Junio C Hamano: >> Jens Lehmann <Jens.Lehmann@xxxxxx> writes: >>> + strbuf_addf(&buf, "%s/.git/", path); >>> + if (!is_directory(buf.buf)) { >>> + strbuf_release(&buf); >>> + /* The submodule is not populated, so we can't check it out */ >>> + return 0; >>> + } >> >> This would give you an incorrect result if .git is a file that records >> "gitdir: overthere" (see read_gitfile_gently() in setup.c); I would expect >> it would become a fairly important ingredient if we ever enhance the >> submodule support to add submodule that disappears/reappears in the >> history. > > Right. This assumption is also present in add_submodule_odb() (used by > show_submodule_summary()) and is_submodule_modified(), so i just reused > it. This should be addressed in another patch. What about this one: -----------------8<--------------- Subject: [PATCH] Teach diff --submodule and status to handle .git files in submodules The simple test for an existing .git directory gives an incorrect result if .git is a file that records "gitdir: overthere". So for submodules that use a .git file, "git status" and the diff family - when the "--submodule" option is given - did assume the submodule was not populated at all when a .git file was used, thus generating wrong output or no output at all. This is fixed by using read_gitfile_gently() to get the correct location of the .git directory. While at it, is_submodule_modified() was cleaned up to use the "dir" member of "struct child_process" instead of setting the GIT_WORK_TREE and GIT_DIR environment variables. Signed-off-by: Jens Lehmann <Jens.Lehmann@xxxxxx> --- submodule.c | 33 ++++++++++++++++----------------- t/t4041-diff-submodule.sh | 15 +++++++++++++++ t/t7506-status-submodule.sh | 16 ++++++++++++++++ 3 files changed, 47 insertions(+), 17 deletions(-) diff --git a/submodule.c b/submodule.c index b3b8bc1..676d48f 100644 --- a/submodule.c +++ b/submodule.c @@ -12,8 +12,15 @@ static int add_submodule_odb(const char *path) struct strbuf objects_directory = STRBUF_INIT; struct alternate_object_database *alt_odb; int ret = 0; + const char *git_dir; - strbuf_addf(&objects_directory, "%s/.git/objects/", path); + strbuf_addf(&objects_directory, "%s/.git", path); + git_dir = read_gitfile_gently(objects_directory.buf); + if (git_dir) { + strbuf_reset(&objects_directory); + strbuf_addstr(&objects_directory, git_dir); + } + strbuf_addstr(&objects_directory, "/objects/"); if (!is_directory(objects_directory.buf)) { ret = -1; goto done; @@ -132,7 +139,6 @@ void show_submodule_summary(FILE *f, const char *path, unsigned is_submodule_modified(const char *path, int ignore_untracked) { - int i; ssize_t len; struct child_process cp; const char *argv[] = { @@ -141,16 +147,16 @@ unsigned is_submodule_modified(const char *path, int ignore_untracked) NULL, NULL, }; - const char *env[LOCAL_REPO_ENV_SIZE + 3]; struct strbuf buf = STRBUF_INIT; unsigned dirty_submodule = 0; const char *line, *next_line; + const char *git_dir; - for (i = 0; i < LOCAL_REPO_ENV_SIZE; i++) - env[i] = local_repo_env[i]; - - strbuf_addf(&buf, "%s/.git/", path); - if (!is_directory(buf.buf)) { + strbuf_addf(&buf, "%s/.git", path); + git_dir = read_gitfile_gently(buf.buf); + if (!git_dir) + git_dir = buf.buf; + if (!is_directory(git_dir)) { strbuf_release(&buf); /* The submodule is not checked out, so it is not modified */ return 0; @@ -158,21 +164,16 @@ unsigned is_submodule_modified(const char *path, int ignore_untracked) } strbuf_reset(&buf); - strbuf_addf(&buf, "GIT_WORK_TREE=%s", path); - env[i++] = strbuf_detach(&buf, NULL); - strbuf_addf(&buf, "GIT_DIR=%s/.git", path); - env[i++] = strbuf_detach(&buf, NULL); - env[i] = NULL; - if (ignore_untracked) argv[2] = "-uno"; memset(&cp, 0, sizeof(cp)); cp.argv = argv; - cp.env = env; + cp.env = local_repo_env; cp.git_cmd = 1; cp.no_stdin = 1; cp.out = -1; + cp.dir = path; if (start_command(&cp)) die("Could not run git status --porcelain"); @@ -201,8 +202,6 @@ unsigned is_submodule_modified(const char *path, int ignore_untracked) if (finish_command(&cp)) die("git status --porcelain failed"); - for (i = LOCAL_REPO_ENV_SIZE; env[i]; i++) - free((char *)env[i]); strbuf_release(&buf); return dirty_submodule; } diff --git a/t/t4041-diff-submodule.sh b/t/t4041-diff-submodule.sh index 11b1997..019acb9 100755 --- a/t/t4041-diff-submodule.sh +++ b/t/t4041-diff-submodule.sh @@ -329,4 +329,19 @@ index 0000000..$head7 EOF " +test_expect_success 'setup .git file for sm2' ' + (cd sm2 && + REAL="$(pwd)/../.real" && + mv .git "$REAL" + echo "gitdir: $REAL" >.git) +' + +test_expect_success 'diff --submodule with .git file' ' + git diff --submodule HEAD^ >actual && + diff actual - <<-EOF +Submodule sm1 $head6...0000000 (submodule deleted) +Submodule sm2 0000000...$head7 (new submodule) +EOF +' + test_done diff --git a/t/t7506-status-submodule.sh b/t/t7506-status-submodule.sh index aeec1f6..3d4f85d 100755 --- a/t/t7506-status-submodule.sh +++ b/t/t7506-status-submodule.sh @@ -157,6 +157,22 @@ test_expect_success 'status with added and untracked file in modified submodule EOF ' +test_expect_success 'setup .git file for sub' ' + (cd sub && + rm -f new-file + REAL="$(pwd)/../.real" && + mv .git "$REAL" + echo "gitdir: $REAL" >.git) && + echo .real >>.gitignore && + git commit -m "added .real to .gitignore" .gitignore +' + +test_expect_success 'status with added file in modified submodule with .git file' ' + (cd sub && git reset --hard && echo >foo && git add foo) && + git status >output && + grep "modified: sub (new commits, modified content)" output +' + test_expect_success 'rm submodule contents' ' rm -rf sub/* sub/.git ' -- 1.7.1.rc0.265.g16922.dirty -- To unsubscribe from this list: send the line "unsubscribe git" in the body of a message to majordomo@xxxxxxxxxxxxxxx More majordomo info at http://vger.kernel.org/majordomo-info.html