From c0c08897c47eb46179c8d2408dc1a91713307842 Mon Sep 17 00:00:00 2001 From: Jeff King Date: Mon, 12 Sep 2016 20:23:48 -0700 Subject: pager: make pager_program a file-local static This variable is only ever used by the routines in pager.c, and other parts of the code should always use those routines (like git_pager()) to make decisions about which pager to use. Let's reduce its scope to prevent accidents. Signed-off-by: Jeff King Signed-off-by: Junio C Hamano --- cache.h | 1 - 1 file changed, 1 deletion(-) (limited to 'cache.h') diff --git a/cache.h b/cache.h index 4ff196c25..5e50887d6 100644 --- a/cache.h +++ b/cache.h @@ -1733,7 +1733,6 @@ extern int write_file_gently(const char *path, const char *fmt, ...); /* pager.c */ extern void setup_pager(void); -extern const char *pager_program; extern int pager_in_use(void); extern int pager_use_color; extern int term_columns(void); -- cgit v1.2.1 From b9605bc4f2e44042824571f70b9a3a74eeebabff Mon Sep 17 00:00:00 2001 From: Jeff King Date: Mon, 12 Sep 2016 20:24:15 -0700 Subject: config: only read .git/config from configured repos When git_config() runs, it looks in the system, user-wide, and repo-level config files. It gets the latter by calling git_pathdup(), which in turn calls get_git_dir(). If we haven't set up the git repository yet, this may simply return ".git", and we will look at ".git/config". This seems like it would be helpful (presumably we haven't set up the repository yet, so it tries to find it), but it turns out to be a bad idea for a few reasons: - it's not sufficient, and therefore hides bugs in a confusing way. Config will be respected if commands are run from the top-level of the working tree, but not from a subdirectory. - it's not always true that we haven't set up the repository _yet_; we may not want to do it at all. For instance, if you run "git init /some/path" from inside another repository, it should not load config from the existing repository. - there might be a path ".git/config", but it is not the actual repository we would find via setup_git_directory(). This may happen, e.g., if you are storing a git repository inside another git repository, but have munged one of the files in such a way that the inner repository is not valid (e.g., by removing HEAD). We have at least two bugs of the second type in git-init, introduced by ae5f677 (lazily load core.sharedrepository, 2016-03-11). It causes init to use git_configset(), which loads all of the config, including values from the current repo (if any). This shows up in two ways: 1. If we happen to be in an existing repository directory, we'll read and respect core.sharedrepository from it, even though it should have no bearing on the new repository. A new test in t1301 covers this. 2. Similarly, if we're in an existing repo that sets core.logallrefupdates, that will cause init to fail to set it in a newly created repository (because it thinks that the user's templates already did so). A new test in t0001 covers this. We also need to adjust an existing test in t1302, which gives another example of why this patch is an improvement. That test creates an embedded repository with a bogus core.repositoryformatversion of "99". It wants to make sure that we actually stop at the bogus repo rather than continuing upward to find the outer repo. So it checks that "git config core.repositoryformatversion" returns 99. But that only works because we blindly read ".git/config", even though we _know_ we're in a repository whose vintage we do not understand. After this patch, we avoid reading config from the unknown vintage repository at all, which is a safer choice. But we need to tweak the test, since core.repositoryformatversion will not return 99; it will claim that it could not find the variable at all. Signed-off-by: Jeff King Signed-off-by: Junio C Hamano --- cache.h | 6 ++++++ 1 file changed, 6 insertions(+) (limited to 'cache.h') diff --git a/cache.h b/cache.h index 5e50887d6..67ec356c7 100644 --- a/cache.h +++ b/cache.h @@ -453,6 +453,12 @@ static inline enum object_type object_type(unsigned int mode) */ extern const char * const local_repo_env[]; +/* + * Returns true iff we have a configured git repository (either via + * setup_git_directory, or in the environment via $GIT_DIR). + */ +int have_git_dir(void); + extern int is_bare_repository_cfg; extern int is_bare_repository(void); extern int is_inside_git_dir(void); -- cgit v1.2.1 From 4543926ba8f8ec596dd4e45f206f4fbc29350567 Mon Sep 17 00:00:00 2001 From: Jeff King Date: Mon, 12 Sep 2016 20:24:23 -0700 Subject: init: reset cached config when entering new repo After we copy the templates into place, we re-read the config in case we copied in a default config file. But since git_config() is backed by a cache these days, it's possible that the call will not actually touch the filesystem at all; we need to tell it that something has changed behind the scenes. Note that we also need to reset the shared_repository config. At first glance, it seems like this should probably just be folded into git_config_clear(). But unfortunately that is not quite right. The shared repository value may come from config, _or_ it may have been set manually. So only the caller who knows whether or not they set it is the one who can clear it (and indeed, if you _do_ put it into git_config_clear(), then many tests fail, as we have to clear the config cache any time we set a new config variable). There are three tests here. The first two actually pass already, though it's largely luck: they just don't happen to actually read any config before we enter the new repo. But the third one does fail without this patch; we look at core.sharedrepository while creating the directory, but need to make sure the value from the template config overrides it. Signed-off-by: Jeff King Signed-off-by: Junio C Hamano --- cache.h | 7 +++++++ 1 file changed, 7 insertions(+) (limited to 'cache.h') diff --git a/cache.h b/cache.h index 67ec356c7..b2d77f3eb 100644 --- a/cache.h +++ b/cache.h @@ -669,8 +669,15 @@ extern size_t delta_base_cache_limit; extern unsigned long big_file_threshold; extern unsigned long pack_size_limit_cfg; +/* + * Accessors for the core.sharedrepository config which lazy-load the value + * from the config (if not already set). The "reset" function can be + * used to unset "set" or cached value, meaning that the value will be loaded + * fresh from the config file on the next call to get_shared_repository(). + */ void set_shared_repository(int value); int get_shared_repository(void); +void reset_shared_repository(void); /* * Do replace refs need to be checked this run? This variable is -- cgit v1.2.1