From 40d96325143ae04cb3f645ac96f6413d1f641b9b Mon Sep 17 00:00:00 2001 From: Stefan Beller Date: Tue, 24 Jan 2017 15:56:49 -0800 Subject: setup: add gentle version of resolve_git_dir This follows a93bedada (setup: add gentle version of read_gitfile, 2015-06-09), and assumes the same reasoning. resolve_git_dir is unsuited for speculative calls, so we want to use the gentle version to find out about potential errors. Signed-off-by: Stefan Beller Signed-off-by: Junio C Hamano --- cache.h | 4 +++- setup.c | 4 ++-- 2 files changed, 5 insertions(+), 3 deletions(-) diff --git a/cache.h b/cache.h index 00a029af3..cafa3d10a 100644 --- a/cache.h +++ b/cache.h @@ -509,7 +509,9 @@ extern int is_nonbare_repository_dir(struct strbuf *path); #define READ_GITFILE_ERR_TOO_LARGE 8 extern const char *read_gitfile_gently(const char *path, int *return_error_code); #define read_gitfile(path) read_gitfile_gently((path), NULL) -extern const char *resolve_gitdir(const char *suspect); +extern const char *resolve_gitdir_gently(const char *suspect, int *return_error_code); +#define resolve_gitdir(path) resolve_gitdir_gently((path), NULL) + extern void set_git_work_tree(const char *tree); #define ALTERNATE_DB_ENVIRONMENT "GIT_ALTERNATE_OBJECT_DIRECTORIES" diff --git a/setup.c b/setup.c index 1b534a750..4605fd3c3 100644 --- a/setup.c +++ b/setup.c @@ -1017,11 +1017,11 @@ const char *setup_git_directory(void) return setup_git_directory_gently(NULL); } -const char *resolve_gitdir(const char *suspect) +const char *resolve_gitdir_gently(const char *suspect, int *return_error_code) { if (is_git_directory(suspect)) return suspect; - return read_gitfile(suspect); + return read_gitfile_gently(suspect, return_error_code); } /* if any standard file descriptor is missing open it to /dev/null */ -- cgit v1.2.1 From 5f29433f1ca1efc35d546a056200d923e86e3fca Mon Sep 17 00:00:00 2001 From: Stefan Beller Date: Tue, 24 Jan 2017 15:56:50 -0800 Subject: cache.h: expose the dying procedure for reading gitlinks In a later patch we want to react to only a subset of errors, defaulting the rest to die as usual. Separate the block that takes care of dying into its own function so we have easy access to it. Signed-off-by: Stefan Beller Signed-off-by: Junio C Hamano --- cache.h | 1 + setup.c | 48 ++++++++++++++++++++++++++---------------------- 2 files changed, 27 insertions(+), 22 deletions(-) diff --git a/cache.h b/cache.h index cafa3d10a..d55f5dccb 100644 --- a/cache.h +++ b/cache.h @@ -507,6 +507,7 @@ extern int is_nonbare_repository_dir(struct strbuf *path); #define READ_GITFILE_ERR_NO_PATH 6 #define READ_GITFILE_ERR_NOT_A_REPO 7 #define READ_GITFILE_ERR_TOO_LARGE 8 +extern void read_gitfile_error_die(int error_code, const char *path, const char *dir); extern const char *read_gitfile_gently(const char *path, int *return_error_code); #define read_gitfile(path) read_gitfile_gently((path), NULL) extern const char *resolve_gitdir_gently(const char *suspect, int *return_error_code); diff --git a/setup.c b/setup.c index 4605fd3c3..967f289f1 100644 --- a/setup.c +++ b/setup.c @@ -486,6 +486,30 @@ int verify_repository_format(const struct repository_format *format, return 0; } +void read_gitfile_error_die(int error_code, const char *path, const char *dir) +{ + switch (error_code) { + case READ_GITFILE_ERR_STAT_FAILED: + case READ_GITFILE_ERR_NOT_A_FILE: + /* non-fatal; follow return path */ + break; + case READ_GITFILE_ERR_OPEN_FAILED: + die_errno("Error opening '%s'", path); + case READ_GITFILE_ERR_TOO_LARGE: + die("Too large to be a .git file: '%s'", path); + case READ_GITFILE_ERR_READ_FAILED: + die("Error reading %s", path); + case READ_GITFILE_ERR_INVALID_FORMAT: + die("Invalid gitfile format: %s", path); + case READ_GITFILE_ERR_NO_PATH: + die("No path in gitfile: %s", path); + case READ_GITFILE_ERR_NOT_A_REPO: + die("Not a git repository: %s", dir); + default: + die("BUG: unknown error code"); + } +} + /* * Try to read the location of the git directory from the .git file, * return path to git directory if found. @@ -559,28 +583,8 @@ const char *read_gitfile_gently(const char *path, int *return_error_code) cleanup_return: if (return_error_code) *return_error_code = error_code; - else if (error_code) { - switch (error_code) { - case READ_GITFILE_ERR_STAT_FAILED: - case READ_GITFILE_ERR_NOT_A_FILE: - /* non-fatal; follow return path */ - break; - case READ_GITFILE_ERR_OPEN_FAILED: - die_errno("Error opening '%s'", path); - case READ_GITFILE_ERR_TOO_LARGE: - die("Too large to be a .git file: '%s'", path); - case READ_GITFILE_ERR_READ_FAILED: - die("Error reading %s", path); - case READ_GITFILE_ERR_INVALID_FORMAT: - die("Invalid gitfile format: %s", path); - case READ_GITFILE_ERR_NO_PATH: - die("No path in gitfile: %s", path); - case READ_GITFILE_ERR_NOT_A_REPO: - die("Not a git repository: %s", dir); - default: - assert(0); - } - } + else if (error_code) + read_gitfile_error_die(error_code, path, dir); free(buf); return error_code ? NULL : path; -- cgit v1.2.1 From ec9629b3b9abc9fc9cb2a9e058bf8dccbc760433 Mon Sep 17 00:00:00 2001 From: Stefan Beller Date: Wed, 25 Jan 2017 15:04:50 -0800 Subject: submodule absorbing: fix worktree/gitdir pointers recursively for non-moves Consider having a submodule 'sub' and a nested submodule at 'sub/nested'. When nested is already absorbed into sub, but sub is not absorbed into its superproject, then we need to fixup the gitfile and core.worktree setting for 'nested' when absorbing 'sub', but we do not need to move its git dir around. Previously 'nested's gitfile contained "gitdir: ../.git/modules/nested"; it has to be corrected to "gitdir: ../../.git/modules/sub1/modules/nested". An alternative I considered to do this work lazily, i.e. when resolving "../.git/modules/nested", we would notice the ".git" being a gitfile linking to another path. That seemed to be robuster by design, but harder to get the implementation right. Maybe we have to do that anyway once we try to have submodules and worktrees working nicely together, but for now just produce 'correct' (i.e. direct) pointers. Signed-off-by: Stefan Beller Signed-off-by: Junio C Hamano --- submodule.c | 62 ++++++++++++++++++++++++++++---------- t/t7412-submodule-absorbgitdirs.sh | 27 +++++++++++++++++ 2 files changed, 73 insertions(+), 16 deletions(-) diff --git a/submodule.c b/submodule.c index 4c4f033e8..3b98766a6 100644 --- a/submodule.c +++ b/submodule.c @@ -1437,22 +1437,57 @@ void absorb_git_dir_into_superproject(const char *prefix, const char *path, unsigned flags) { - const char *sub_git_dir, *v; - char *real_sub_git_dir = NULL, *real_common_git_dir = NULL; + int err_code; + const char *sub_git_dir; struct strbuf gitdir = STRBUF_INIT; - strbuf_addf(&gitdir, "%s/.git", path); - sub_git_dir = resolve_gitdir(gitdir.buf); + sub_git_dir = resolve_gitdir_gently(gitdir.buf, &err_code); /* Not populated? */ - if (!sub_git_dir) - goto out; + if (!sub_git_dir) { + char *real_new_git_dir; + const char *new_git_dir; + const struct submodule *sub; + + if (err_code == READ_GITFILE_ERR_STAT_FAILED) { + /* unpopulated as expected */ + strbuf_release(&gitdir); + return; + } + + if (err_code != READ_GITFILE_ERR_NOT_A_REPO) + /* We don't know what broke here. */ + read_gitfile_error_die(err_code, path, NULL); + + /* + * Maybe populated, but no git directory was found? + * This can happen if the superproject is a submodule + * itself and was just absorbed. The absorption of the + * superproject did not rewrite the git file links yet, + * fix it now. + */ + sub = submodule_from_path(null_sha1, path); + if (!sub) + die(_("could not lookup name for submodule '%s'"), path); + new_git_dir = git_path("modules/%s", sub->name); + if (safe_create_leading_directories_const(new_git_dir) < 0) + die(_("could not create directory '%s'"), new_git_dir); + real_new_git_dir = real_pathdup(new_git_dir); + connect_work_tree_and_git_dir(path, real_new_git_dir); + + free(real_new_git_dir); + } else { + /* Is it already absorbed into the superprojects git dir? */ + char *real_sub_git_dir = real_pathdup(sub_git_dir); + char *real_common_git_dir = real_pathdup(get_git_common_dir()); - /* Is it already absorbed into the superprojects git dir? */ - real_sub_git_dir = real_pathdup(sub_git_dir); - real_common_git_dir = real_pathdup(get_git_common_dir()); - if (!skip_prefix(real_sub_git_dir, real_common_git_dir, &v)) - relocate_single_git_dir_into_superproject(prefix, path); + if (!starts_with(real_sub_git_dir, real_common_git_dir)) + relocate_single_git_dir_into_superproject(prefix, path); + + free(real_sub_git_dir); + free(real_common_git_dir); + } + strbuf_release(&gitdir); if (flags & ABSORB_GITDIR_RECURSE_SUBMODULES) { struct child_process cp = CHILD_PROCESS_INIT; @@ -1478,9 +1513,4 @@ void absorb_git_dir_into_superproject(const char *prefix, strbuf_release(&sb); } - -out: - strbuf_release(&gitdir); - free(real_sub_git_dir); - free(real_common_git_dir); } diff --git a/t/t7412-submodule-absorbgitdirs.sh b/t/t7412-submodule-absorbgitdirs.sh index 1c47780e2..e2bbb449b 100755 --- a/t/t7412-submodule-absorbgitdirs.sh +++ b/t/t7412-submodule-absorbgitdirs.sh @@ -64,6 +64,33 @@ test_expect_success 'absorb the git dir in a nested submodule' ' test_cmp expect.2 actual.2 ' +test_expect_success 're-setup nested submodule' ' + # un-absorb the direct submodule, to test if the nested submodule + # is still correct (needs a rewrite of the gitfile only) + rm -rf sub1/.git && + mv .git/modules/sub1 sub1/.git && + GIT_WORK_TREE=. git -C sub1 config --unset core.worktree && + # fixup the nested submodule + echo "gitdir: ../.git/modules/nested" >sub1/nested/.git && + GIT_WORK_TREE=../../../nested git -C sub1/.git/modules/nested config \ + core.worktree "../../../nested" && + # make sure this re-setup is correct + git status --ignore-submodules=none +' + +test_expect_success 'absorb the git dir in a nested submodule' ' + git status >expect.1 && + git -C sub1/nested rev-parse HEAD >expect.2 && + git submodule absorbgitdirs && + test -f sub1/.git && + test -f sub1/nested/.git && + test -d .git/modules/sub1/modules/nested && + git status >actual.1 && + git -C sub1/nested rev-parse HEAD >actual.2 && + test_cmp expect.1 actual.1 && + test_cmp expect.2 actual.2 +' + test_expect_success 'setup a gitlink with missing .gitmodules entry' ' git init sub2 && test_commit -C sub2 first && -- cgit v1.2.1