Re: [PATCH v3 17/25] setup.c: detect $GIT_COMMON_DIR check_repository_format_gently()

2 messages, 2 authors, 2016-06-15 · open the first message on its own page

Re: [PATCH v3 17/25] setup.c: detect $GIT_COMMON_DIR check_repository_format_gently()

From: Junio C Hamano <hidden>
Date: 2016-06-15 23:00:01

Nguyễn Thái Ngọc Duy  [off-list ref] writes:
Signed-off-by: Nguyễn Thái Ngọc Duy <redacted>
---
It is a good thing to do to read config from the real repository we
are borrowing from when we have .git/commondir, but it makes me
wonder if we should signal some kind of error if we find .git/config
in such a borrowing repository---which will be silently ignored.

My gut feeling is that such a check may be necessary, but may not
belong to this function.
quoted hunk
 setup.c | 6 ++++--
 1 file changed, 4 insertions(+), 2 deletions(-)
diff --git a/setup.c b/setup.c
index 282fdc9..e56ec11 100644
--- a/setup.c
+++ b/setup.c
@@ -285,6 +285,10 @@ static int check_repository_format_gently(const char *gitdir, int *nongit_ok)
 	const char *repo_config;
 	int ret = 0;
 
+	get_common_dir(&sb, gitdir);
+	strbuf_addstr(&sb, "/config");
+	repo_config = sb.buf;
+
 	/*
 	 * git_config() can't be used here because it calls git_pathdup()
 	 * to get $GIT_CONFIG/config. That call will make setup_git_env()
@@ -294,8 +298,6 @@ static int check_repository_format_gently(const char *gitdir, int *nongit_ok)
 	 * Use a gentler version of git_config() to check if this repo
 	 * is a good one.
 	 */
-	strbuf_addf(&sb, "%s/config", gitdir);
-	repo_config = sb.buf;
 	git_config_early(check_repository_format_version, NULL, repo_config);
 	if (GIT_REPO_VERSION < repository_format_version) {
 		if (!nongit_ok)

Re: [PATCH v3 17/25] setup.c: detect $GIT_COMMON_DIR check_repository_format_gently()

From: Duy Nguyen <hidden>
Date: 2016-06-15 23:00:01

On Thu, Feb 27, 2014 at 7:22 AM, Junio C Hamano [off-list ref] wrote:
Nguyễn Thái Ngọc Duy  [off-list ref] writes:
quoted
Signed-off-by: Nguyễn Thái Ngọc Duy <redacted>
---
It is a good thing to do to read config from the real repository we
are borrowing from when we have .git/commondir, but it makes me
wonder if we should signal some kind of error if we find .git/config
in such a borrowing repository---which will be silently ignored.

My gut feeling is that such a check may be necessary, but may not
belong to this function.
Just checking. Once we do this, what about .git/refs/.. that is also
ignored in such a repo? I can see the point that config could be
manually edited so the user may edit the file at the wrong place, so
it's good the we catch this exception.
-- 
Duy
Keyboard shortcuts
hback out one level
jnext message in thread
kprevious message in thread
ldrill in
Escclose help / fold thread tree
?toggle this help