From 2163e5dbb4cad43d65a4ffc8daeacff5eedd7af9 Mon Sep 17 00:00:00 2001 From: Jeff King Date: Fri, 8 Mar 2013 04:29:08 -0500 Subject: cache.h: drop LOCAL_REPO_ENV_SIZE We keep a static array of variables that should be cleared when invoking a sub-process on another repo. We statically size the array with the LOCAL_REPO_ENV_SIZE macro so that any readers do not have to count it themselves. As it turns out, no readers actually use the macro, and it creates a maintenance headache, as modifications to the array need to happen in two places (one to add the new element, and another to bump the size). Since it's NULL-terminated, we can just drop the size macro entirely. While we're at it, we'll clean up some comments around it, and add a new mention of it at the top of the list of environment variable macros. Even though local_repo_env is right below that list, it's easy to miss, and additions to that list should consider local_repo_env. Signed-off-by: Jeff King Signed-off-by: Junio C Hamano --- cache.h | 12 ++++++------ environment.c | 6 ++---- 2 files changed, 8 insertions(+), 10 deletions(-) diff --git a/cache.h b/cache.h index 2b192d24ac..139b911816 100644 --- a/cache.h +++ b/cache.h @@ -341,6 +341,7 @@ static inline enum object_type object_type(unsigned int mode) OBJ_BLOB; } +/* Double-check local_repo_env below if you add to this list. */ #define GIT_DIR_ENVIRONMENT "GIT_DIR" #define GIT_NAMESPACE_ENVIRONMENT "GIT_NAMESPACE" #define GIT_WORK_TREE_ENVIRONMENT "GIT_WORK_TREE" @@ -364,13 +365,12 @@ static inline enum object_type object_type(unsigned int mode) #define GIT_NOTES_REWRITE_MODE_ENVIRONMENT "GIT_NOTES_REWRITE_MODE" /* - * Repository-local GIT_* environment variables - * The array is NULL-terminated to simplify its usage in contexts such - * environment creation or simple walk of the list. - * The number of non-NULL entries is available as a macro. + * Repository-local GIT_* environment variables; these will be cleared + * when git spawns a sub-process that runs inside another repository. + * The array is NULL-terminated, which makes it easy to pass in the "env" + * parameter of a run-command invocation, or to do a simple walk. */ -#define LOCAL_REPO_ENV_SIZE 9 -extern const char *const local_repo_env[LOCAL_REPO_ENV_SIZE + 1]; +extern const char * const local_repo_env[]; extern int is_bare_repository_cfg; extern int is_bare_repository(void); diff --git a/environment.c b/environment.c index 85edd7f95a..81ffb4b666 100644 --- a/environment.c +++ b/environment.c @@ -76,11 +76,9 @@ static const char *git_dir; static char *git_object_dir, *git_index_file, *git_graft_file; /* - * Repository-local GIT_* environment variables - * Remember to update local_repo_env_size in cache.h when - * the size of the list changes + * Repository-local GIT_* environment variables; see cache.h for details. */ -const char * const local_repo_env[LOCAL_REPO_ENV_SIZE + 1] = { +const char * const local_repo_env[] = { ALTERNATE_DB_ENVIRONMENT, CONFIG_ENVIRONMENT, CONFIG_DATA_ENVIRONMENT, -- cgit v1.2.3 From a6f7f9a32532a636bd458c7a3dc2cc16fbe237d3 Mon Sep 17 00:00:00 2001 From: Jeff King Date: Fri, 8 Mar 2013 04:30:25 -0500 Subject: environment: add GIT_PREFIX to local_repo_env The GIT_PREFIX variable is set based on our location within the working tree. It should therefore be cleared whenever GIT_WORK_TREE is cleared. In practice, this doesn't cause any bugs, because none of the sub-programs we invoke with local_repo_env cleared actually care about GIT_PREFIX. But this is the right thing to do, and future proofs us against that assumption changing. While we're at it, let's define a GIT_PREFIX_ENVIRONMENT macro; this avoids repetition of the string literal, which can help catch any spelling mistakes in the code. Signed-off-by: Jeff King Signed-off-by: Junio C Hamano --- cache.h | 1 + environment.c | 1 + setup.c | 4 ++-- 3 files changed, 4 insertions(+), 2 deletions(-) diff --git a/cache.h b/cache.h index 139b911816..3c6b677418 100644 --- a/cache.h +++ b/cache.h @@ -345,6 +345,7 @@ static inline enum object_type object_type(unsigned int mode) #define GIT_DIR_ENVIRONMENT "GIT_DIR" #define GIT_NAMESPACE_ENVIRONMENT "GIT_NAMESPACE" #define GIT_WORK_TREE_ENVIRONMENT "GIT_WORK_TREE" +#define GIT_PREFIX_ENVIRONMENT "GIT_PREFIX" #define DEFAULT_GIT_DIR_ENVIRONMENT ".git" #define DB_ENVIRONMENT "GIT_OBJECT_DIRECTORY" #define INDEX_ENVIRONMENT "GIT_INDEX_FILE" diff --git a/environment.c b/environment.c index 81ffb4b666..4fb7ceac3a 100644 --- a/environment.c +++ b/environment.c @@ -88,6 +88,7 @@ const char * const local_repo_env[] = { GRAFT_ENVIRONMENT, INDEX_ENVIRONMENT, NO_REPLACE_OBJECTS_ENVIRONMENT, + GIT_PREFIX_ENVIRONMENT, NULL }; diff --git a/setup.c b/setup.c index 1b120175f2..d0e3ecdcd4 100644 --- a/setup.c +++ b/setup.c @@ -768,9 +768,9 @@ const char *setup_git_directory_gently(int *nongit_ok) prefix = setup_git_directory_gently_1(nongit_ok); if (prefix) - setenv("GIT_PREFIX", prefix, 1); + setenv(GIT_PREFIX_ENVIRONMENT, prefix, 1); else - setenv("GIT_PREFIX", "", 1); + setenv(GIT_PREFIX_ENVIRONMENT, "", 1); if (startup_info) { startup_info->have_repository = !nongit_ok || !*nongit_ok; -- cgit v1.2.3 From 2cd83d10bb6bcf768129e1c4e5a4dee4b6bcd27f Mon Sep 17 00:00:00 2001 From: Jeff King Date: Fri, 8 Mar 2013 04:32:22 -0500 Subject: setup: suppress implicit "." work-tree for bare repos If an explicit GIT_DIR is given without a working tree, we implicitly assume that the current working directory should be used as the working tree. E.g.,: GIT_DIR=/some/repo.git git status would compare against the cwd. Unfortunately, we fool this rule for sub-invocations of git by setting GIT_DIR internally ourselves. For example: git init foo cd foo/.git git status ;# fails, as we expect git config alias.st status git status ;# does not fail, but should What happens is that we run setup_git_directory when doing alias lookup (since we need to see the config), set GIT_DIR as a result, and then leave GIT_WORK_TREE blank (because we do not have one). Then when we actually run the status command, we do setup_git_directory again, which sees our explicit GIT_DIR and uses the cwd as an implicit worktree. It's tempting to argue that we should be suppressing that second invocation of setup_git_directory, as it could use the values we already found in memory. However, the problem still exists for sub-processes (e.g., if "git status" were an external command). You can see another example with the "--bare" option, which sets GIT_DIR explicitly. For example: git init foo cd foo/.git git status ;# fails git --bare status ;# does NOT fail We need some way of telling sub-processes "even though GIT_DIR is set, do not use cwd as an implicit working tree". We could do it by putting a special token into GIT_WORK_TREE, but the obvious choice (an empty string) has some portability problems. Instead, we add a new boolean variable, GIT_IMPLICIT_WORK_TREE, which suppresses the use of cwd as a working tree when GIT_DIR is set. We trigger the new variable when we know we are in a bare setting. The variable is left intentionally undocumented, as this is an internal detail (for now, anyway). If somebody comes up with a good alternate use for it, and once we are confident we have shaken any bugs out of it, we can consider promoting it further. Signed-off-by: Jeff King Signed-off-by: Junio C Hamano --- cache.h | 12 ++++++++++++ environment.c | 1 + git.c | 1 + setup.c | 8 ++++++++ t/t1510-repo-setup.sh | 19 +++++++++++++++++++ 5 files changed, 41 insertions(+) diff --git a/cache.h b/cache.h index 3c6b677418..c1fe67ffcf 100644 --- a/cache.h +++ b/cache.h @@ -365,6 +365,18 @@ static inline enum object_type object_type(unsigned int mode) #define GIT_NOTES_REWRITE_REF_ENVIRONMENT "GIT_NOTES_REWRITE_REF" #define GIT_NOTES_REWRITE_MODE_ENVIRONMENT "GIT_NOTES_REWRITE_MODE" +/* + * This environment variable is expected to contain a boolean indicating + * whether we should or should not treat: + * + * GIT_DIR=foo.git git ... + * + * as if GIT_WORK_TREE=. was given. It's not expected that users will make use + * of this, but we use it internally to communicate to sub-processes that we + * are in a bare repo. If not set, defaults to true. + */ +#define GIT_IMPLICIT_WORK_TREE_ENVIRONMENT "GIT_IMPLICIT_WORK_TREE" + /* * Repository-local GIT_* environment variables; these will be cleared * when git spawns a sub-process that runs inside another repository. diff --git a/environment.c b/environment.c index 4fb7ceac3a..92c5dff008 100644 --- a/environment.c +++ b/environment.c @@ -85,6 +85,7 @@ const char * const local_repo_env[] = { DB_ENVIRONMENT, GIT_DIR_ENVIRONMENT, GIT_WORK_TREE_ENVIRONMENT, + GIT_IMPLICIT_WORK_TREE_ENVIRONMENT, GRAFT_ENVIRONMENT, INDEX_ENVIRONMENT, NO_REPLACE_OBJECTS_ENVIRONMENT, diff --git a/git.c b/git.c index d33f9b32a2..0ffea570c7 100644 --- a/git.c +++ b/git.c @@ -125,6 +125,7 @@ static int handle_options(const char ***argv, int *argc, int *envchanged) static char git_dir[PATH_MAX+1]; is_bare_repository_cfg = 1; setenv(GIT_DIR_ENVIRONMENT, getcwd(git_dir, sizeof(git_dir)), 0); + setenv(GIT_IMPLICIT_WORK_TREE_ENVIRONMENT, "0", 1); if (envchanged) *envchanged = 1; } else if (!strcmp(cmd, "-c")) { diff --git a/setup.c b/setup.c index d0e3ecdcd4..01c5476eb7 100644 --- a/setup.c +++ b/setup.c @@ -497,6 +497,12 @@ static const char *setup_explicit_git_dir(const char *gitdirenv, set_git_work_tree(core_worktree); } } + else if (!git_env_bool(GIT_IMPLICIT_WORK_TREE_ENVIRONMENT, 1)) { + /* #16d */ + set_git_dir(gitdirenv); + free(gitfile); + return NULL; + } else /* #2, #10 */ set_git_work_tree("."); @@ -575,6 +581,8 @@ static const char *setup_bare_git_dir(char *cwd, int offset, int len, int *nongi if (check_repository_format_gently(".", nongit_ok)) return NULL; + setenv(GIT_IMPLICIT_WORK_TREE_ENVIRONMENT, "0", 1); + /* --work-tree is set without --git-dir; use discovered one */ if (getenv(GIT_WORK_TREE_ENVIRONMENT) || git_work_tree_cfg) { const char *gitdir; diff --git a/t/t1510-repo-setup.sh b/t/t1510-repo-setup.sh index 80aedfca8c..cf2ee7885a 100755 --- a/t/t1510-repo-setup.sh +++ b/t/t1510-repo-setup.sh @@ -517,6 +517,25 @@ test_expect_success '#16c: bare .git has no worktree' ' "$here/16c/.git" "(null)" "$here/16c/sub" "(null)" ' +test_expect_success '#16d: bareness preserved across alias' ' + setup_repo 16d unset "" unset && + ( + cd 16d/.git && + test_must_fail git status && + git config alias.st status && + test_must_fail git st + ) +' + +test_expect_success '#16e: bareness preserved by --bare' ' + setup_repo 16e unset "" unset && + ( + cd 16e/.git && + test_must_fail git status && + test_must_fail git --bare status + ) +' + test_expect_success '#17: GIT_WORK_TREE without explicit GIT_DIR is accepted (bare case)' ' # Just like #16. setup_repo 17a unset "" true && -- cgit v1.2.3