From 4b0d1eebe95b8ed187ff06ae46d69d517c2b759f Mon Sep 17 00:00:00 2001 From: Jeff King Date: Fri, 11 Mar 2016 17:36:45 -0500 Subject: setup: document check_repository_format() This function's interface is rather enigmatic, so let's document it further. While we're here, let's also drop the return value. It will always either be "0" or the function will die (consequently, neither of its two callers bothered to check the return). Signed-off-by: Jeff King Signed-off-by: Junio C Hamano --- setup.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) (limited to 'setup.c') diff --git a/setup.c b/setup.c index de1a2a7ea5..b2f2e690e4 100644 --- a/setup.c +++ b/setup.c @@ -982,9 +982,9 @@ int check_repository_format_version(const char *var, const char *value, void *cb return 0; } -int check_repository_format(void) +void check_repository_format(void) { - return check_repository_format_gently(get_git_dir(), NULL); + check_repository_format_gently(get_git_dir(), NULL); } /* -- cgit v1.2.1 From 7875acb6ecf85a7dc29554d193955ce5e265764f Mon Sep 17 00:00:00 2001 From: Jeff King Date: Fri, 11 Mar 2016 17:36:49 -0500 Subject: wrap shared_repository global in get/set accessors It would be useful to control access to the global shared_repository, so that we can lazily load its config. The first step to doing so is to make sure all access goes through a set of functions. This step is purely mechanical, and should result in no change of behavior. Signed-off-by: Jeff King Signed-off-by: Junio C Hamano --- setup.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) (limited to 'setup.c') diff --git a/setup.c b/setup.c index b2f2e690e4..ac777c5f10 100644 --- a/setup.c +++ b/setup.c @@ -377,7 +377,7 @@ static int check_repo_format(const char *var, const char *value, void *cb) if (strcmp(var, "core.repositoryformatversion") == 0) repository_format_version = git_config_int(var, value); else if (strcmp(var, "core.sharedrepository") == 0) - shared_repository = git_config_perm(var, value); + set_shared_repository(git_config_perm(var, value)); else if (skip_prefix(var, "extensions.", &ext)) { /* * record any known extensions here; otherwise, -- cgit v1.2.1 From ae5f67763b2a3eea7e7675febd8f87bf2f4c1219 Mon Sep 17 00:00:00 2001 From: Jeff King Date: Fri, 11 Mar 2016 17:36:53 -0500 Subject: lazily load core.sharedrepository The "shared_repository" config is loaded as part of check_repository_format_version, but it's not quite like the other values we check there. Something like core.repositoryformatversion only makes sense in per-repo config, but core.sharedrepository can be set in a per-user config (e.g., to make all "git init" invocations shared by default). So it would make more sense as part of git_default_config. Commit 457f06d (Introduce core.sharedrepository, 2005-12-22) says: [...]the config variable is set in the function which checks the repository format. If this were done in git_default_config instead, a lot of programs would need to be modified to call git_config(git_default_config) first. This is still the case today, but we have one extra trick up our sleeve. Now that we have the git_configset infrastructure, it's not so expensive for us to ask for a single value. So we can simply lazy-load it on demand. This should be OK to do in general. There are some problems with loading config before setup_git_directory() is called, but we shouldn't be accessing the value before then (if we were, then it would already be broken, as the variable would not have been set by check_repository_format_version!). The trickiest caller is git-init, but it handles the values manually itself. Signed-off-by: Jeff King Signed-off-by: Junio C Hamano --- setup.c | 2 -- 1 file changed, 2 deletions(-) (limited to 'setup.c') diff --git a/setup.c b/setup.c index ac777c5f10..a02932b969 100644 --- a/setup.c +++ b/setup.c @@ -376,8 +376,6 @@ static int check_repo_format(const char *var, const char *value, void *cb) if (strcmp(var, "core.repositoryformatversion") == 0) repository_format_version = git_config_int(var, value); - else if (strcmp(var, "core.sharedrepository") == 0) - set_shared_repository(git_config_perm(var, value)); else if (skip_prefix(var, "extensions.", &ext)) { /* * record any known extensions here; otherwise, -- cgit v1.2.1 From 21627f9b6ddddfeb40a53e116459a86be0520a4e Mon Sep 17 00:00:00 2001 From: Jeff King Date: Fri, 11 Mar 2016 17:36:59 -0500 Subject: check_repository_format_gently: stop using git_config_early There's a chicken-and-egg problem with using the regular git_config during the repository setup process. We get around it here by using a special interface that lets us specify the per-repo config, and avoid calling git_pathdup(). But this interface doesn't actually make sense. It will look in the system and per-user config, too; we definitely would not want to accept a core.repositoryformatversion from there. The git_config_from_file interface is a better match, as it lets us look at a single file. Signed-off-by: Jeff King Signed-off-by: Junio C Hamano --- setup.c | 11 +++-------- 1 file changed, 3 insertions(+), 8 deletions(-) (limited to 'setup.c') diff --git a/setup.c b/setup.c index a02932b969..a6013e6dd3 100644 --- a/setup.c +++ b/setup.c @@ -409,15 +409,10 @@ static int check_repository_format_gently(const char *gitdir, int *nongit_ok) 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() - * set git_dir to ".git". - * - * We are in gitdir setup, no git dir has been found useable yet. - * Use a gentler version of git_config() to check if this repo - * is a good one. + * Ignore return value; for historical reasons, we must treat a missing + * config file as a noop (git-init relies on this). */ - git_config_early(fn, NULL, repo_config); + git_config_from_file(fn, repo_config, NULL); if (GIT_REPO_VERSION_READ < repository_format_version) { if (!nongit_ok) die ("Expected git repo version <= %d, found %d", -- cgit v1.2.1 From 2cc7c2c737f2af16915b3d6cb6245111e1349609 Mon Sep 17 00:00:00 2001 From: Jeff King Date: Fri, 11 Mar 2016 17:37:07 -0500 Subject: setup: refactor repo format reading and verification When we want to know if we're in a git repository of reasonable vintage, we can call check_repository_format_gently(), which does three things: 1. Reads the config from the .git/config file. 2. Verifies that the version info we read is sane. 3. Writes some global variables based on this. There are a few things we could improve here. One is that steps 1 and 3 happen together. So if the verification in step 2 fails, we still clobber the global variables. This is especially bad if we go on to try another repository directory; we may end up with a state of mixed config variables. The second is there's no way to ask about the repository version for anything besides the main repository we're in. git-init wants to do this, and it's possible that we would want to start doing so for submodules (e.g., to find out which ref backend they're using). We can improve both by splitting the first two steps into separate functions. Now check_repository_format_gently() calls out to steps 1 and 2, and does 3 only if step 2 succeeds. Note that the public interface for read_repository_format() and what check_repository_format_gently() needs from it are not quite the same, leading us to have an extra read_repository_format_1() helper. The extra needs from check_repository_format_gently() will go away in a future patch, and we can simplify this then to just the public interface. Signed-off-by: Jeff King Signed-off-by: Junio C Hamano --- setup.c | 118 +++++++++++++++++++++++++++++++++++++++++++--------------------- 1 file changed, 79 insertions(+), 39 deletions(-) (limited to 'setup.c') diff --git a/setup.c b/setup.c index a6013e6dd3..8aa49a9570 100644 --- a/setup.c +++ b/setup.c @@ -5,7 +5,6 @@ static int inside_git_dir = -1; static int inside_work_tree = -1; static int work_tree_config_is_bogus; -static struct string_list unknown_extensions = STRING_LIST_INIT_DUP; /* * The input parameter must contain an absolute path, and it must already be @@ -370,12 +369,13 @@ void setup_work_tree(void) initialized = 1; } -static int check_repo_format(const char *var, const char *value, void *cb) +static int check_repo_format(const char *var, const char *value, void *vdata) { + struct repository_format *data = vdata; const char *ext; if (strcmp(var, "core.repositoryformatversion") == 0) - repository_format_version = git_config_int(var, value); + data->version = git_config_int(var, value); else if (skip_prefix(var, "extensions.", &ext)) { /* * record any known extensions here; otherwise, @@ -385,61 +385,104 @@ static int check_repo_format(const char *var, const char *value, void *cb) if (!strcmp(ext, "noop")) ; else if (!strcmp(ext, "preciousobjects")) - repository_format_precious_objects = git_config_bool(var, value); + data->precious_objects = git_config_bool(var, value); else - string_list_append(&unknown_extensions, ext); + string_list_append(&data->unknown_extensions, ext); } return 0; } +static int read_repository_format_1(struct repository_format *, config_fn_t, + const char *); + static int check_repository_format_gently(const char *gitdir, int *nongit_ok) { struct strbuf sb = STRBUF_INIT; - const char *repo_config; + struct strbuf err = STRBUF_INIT; + struct repository_format candidate; config_fn_t fn; - int ret = 0; - - string_list_clear(&unknown_extensions, 0); if (get_common_dir(&sb, gitdir)) fn = check_repo_format; else fn = check_repository_format_version; + strbuf_addstr(&sb, "/config"); - repo_config = sb.buf; + read_repository_format_1(&candidate, fn, sb.buf); + strbuf_release(&sb); /* - * Ignore return value; for historical reasons, we must treat a missing - * config file as a noop (git-init relies on this). + * For historical use of check_repository_format() in git-init, + * we treat a missing config as a silent "ok", even when nongit_ok + * is unset. */ - git_config_from_file(fn, repo_config, NULL); - if (GIT_REPO_VERSION_READ < repository_format_version) { - if (!nongit_ok) - die ("Expected git repo version <= %d, found %d", - GIT_REPO_VERSION_READ, repository_format_version); - warning("Expected git repo version <= %d, found %d", - GIT_REPO_VERSION_READ, repository_format_version); - warning("Please upgrade Git"); - *nongit_ok = -1; - ret = -1; + if (candidate.version < 0) + return 0; + + if (verify_repository_format(&candidate, &err) < 0) { + if (nongit_ok) { + warning("%s", err.buf); + strbuf_release(&err); + *nongit_ok = -1; + return -1; + } + die("%s", err.buf); } - if (repository_format_version >= 1 && unknown_extensions.nr) { + repository_format_version = candidate.version; + repository_format_precious_objects = candidate.precious_objects; + string_list_clear(&candidate.unknown_extensions, 0); + if (candidate.is_bare != -1) { + is_bare_repository_cfg = candidate.is_bare; + if (is_bare_repository_cfg == 1) + inside_work_tree = -1; + } + if (candidate.work_tree) { + free(git_work_tree_cfg); + git_work_tree_cfg = candidate.work_tree; + inside_work_tree = -1; + } + + return 0; +} + +static int read_repository_format_1(struct repository_format *format, + config_fn_t fn, const char *path) +{ + memset(format, 0, sizeof(*format)); + format->version = -1; + format->is_bare = -1; + string_list_init(&format->unknown_extensions, 1); + git_config_from_file(fn, path, format); + return format->version; +} + +int read_repository_format(struct repository_format *format, const char *path) +{ + return read_repository_format_1(format, check_repository_format_version, path); +} + +int verify_repository_format(const struct repository_format *format, + struct strbuf *err) +{ + if (GIT_REPO_VERSION_READ < format->version) { + strbuf_addf(err, "Expected git repo version <= %d, found %d", + GIT_REPO_VERSION_READ, format->version); + return -1; + } + + if (format->version >= 1 && format->unknown_extensions.nr) { int i; - if (!nongit_ok) - die("unknown repository extension: %s", - unknown_extensions.items[0].string); + strbuf_addstr(err, "unknown repository extensions found:"); - for (i = 0; i < unknown_extensions.nr; i++) - warning("unknown repository extension: %s", - unknown_extensions.items[i].string); - *nongit_ok = -1; - ret = -1; + for (i = 0; i < format->unknown_extensions.nr; i++) + strbuf_addf(err, "\n\t%s", + format->unknown_extensions.items[i].string); + return -1; } - strbuf_release(&sb); - return ret; + return 0; } /* @@ -958,19 +1001,16 @@ int git_config_perm(const char *var, const char *value) int check_repository_format_version(const char *var, const char *value, void *cb) { + struct repository_format *data = cb; int ret = check_repo_format(var, value, cb); if (ret) return ret; if (strcmp(var, "core.bare") == 0) { - is_bare_repository_cfg = git_config_bool(var, value); - if (is_bare_repository_cfg == 1) - inside_work_tree = -1; + data->is_bare = git_config_bool(var, value); } else if (strcmp(var, "core.worktree") == 0) { if (!value) return config_error_nonbool(var); - free(git_work_tree_cfg); - git_work_tree_cfg = xstrdup(value); - inside_work_tree = -1; + data->work_tree = xstrdup(value); } return 0; } -- cgit v1.2.1 From 652f18ee8734ffb4a98271e5020dfa550db0f37b Mon Sep 17 00:00:00 2001 From: Jeff King Date: Fri, 11 Mar 2016 17:37:14 -0500 Subject: setup: unify repository version callbacks Once upon a time, check_repository_format_gently would parse the config with a single callback, and that callback would set up a bunch of global variables. But now that we have separate workdirs, we have to be more careful. Commit 31e26eb (setup.c: support multi-checkout repo setup, 2014-11-30) introduced a reduced callback which omits some values like core.worktree. In the "main" callback we call the reduced one, and then add back in the missing variables. Now that we have split the config-parsing from the munging of the global variables, we can do it all with a single callback, and keep all of the "are we in a separate workdir" logic together. Signed-off-by: Jeff King Signed-off-by: Junio C Hamano --- setup.c | 65 +++++++++++++++++++++++------------------------------------------ 1 file changed, 23 insertions(+), 42 deletions(-) (limited to 'setup.c') diff --git a/setup.c b/setup.c index 8aa49a9570..fbe7ec16dd 100644 --- a/setup.c +++ b/setup.c @@ -388,27 +388,26 @@ static int check_repo_format(const char *var, const char *value, void *vdata) data->precious_objects = git_config_bool(var, value); else string_list_append(&data->unknown_extensions, ext); + } else if (strcmp(var, "core.bare") == 0) { + data->is_bare = git_config_bool(var, value); + } else if (strcmp(var, "core.worktree") == 0) { + if (!value) + return config_error_nonbool(var); + data->work_tree = xstrdup(value); } return 0; } -static int read_repository_format_1(struct repository_format *, config_fn_t, - const char *); - static int check_repository_format_gently(const char *gitdir, int *nongit_ok) { struct strbuf sb = STRBUF_INIT; struct strbuf err = STRBUF_INIT; struct repository_format candidate; - config_fn_t fn; - - if (get_common_dir(&sb, gitdir)) - fn = check_repo_format; - else - fn = check_repository_format_version; + int has_common; + has_common = get_common_dir(&sb, gitdir); strbuf_addstr(&sb, "/config"); - read_repository_format_1(&candidate, fn, sb.buf); + read_repository_format(&candidate, sb.buf); strbuf_release(&sb); /* @@ -432,36 +431,34 @@ static int check_repository_format_gently(const char *gitdir, int *nongit_ok) repository_format_version = candidate.version; repository_format_precious_objects = candidate.precious_objects; string_list_clear(&candidate.unknown_extensions, 0); - if (candidate.is_bare != -1) { - is_bare_repository_cfg = candidate.is_bare; - if (is_bare_repository_cfg == 1) + if (!has_common) { + if (candidate.is_bare != -1) { + is_bare_repository_cfg = candidate.is_bare; + if (is_bare_repository_cfg == 1) + inside_work_tree = -1; + } + if (candidate.work_tree) { + free(git_work_tree_cfg); + git_work_tree_cfg = candidate.work_tree; inside_work_tree = -1; - } - if (candidate.work_tree) { - free(git_work_tree_cfg); - git_work_tree_cfg = candidate.work_tree; - inside_work_tree = -1; + } + } else { + free(candidate.work_tree); } return 0; } -static int read_repository_format_1(struct repository_format *format, - config_fn_t fn, const char *path) +int read_repository_format(struct repository_format *format, const char *path) { memset(format, 0, sizeof(*format)); format->version = -1; format->is_bare = -1; string_list_init(&format->unknown_extensions, 1); - git_config_from_file(fn, path, format); + git_config_from_file(check_repo_format, path, format); return format->version; } -int read_repository_format(struct repository_format *format, const char *path) -{ - return read_repository_format_1(format, check_repository_format_version, path); -} - int verify_repository_format(const struct repository_format *format, struct strbuf *err) { @@ -999,22 +996,6 @@ int git_config_perm(const char *var, const char *value) return -(i & 0666); } -int check_repository_format_version(const char *var, const char *value, void *cb) -{ - struct repository_format *data = cb; - int ret = check_repo_format(var, value, cb); - if (ret) - return ret; - if (strcmp(var, "core.bare") == 0) { - data->is_bare = git_config_bool(var, value); - } else if (strcmp(var, "core.worktree") == 0) { - if (!value) - return config_error_nonbool(var); - data->work_tree = xstrdup(value); - } - return 0; -} - void check_repository_format(void) { check_repository_format_gently(get_git_dir(), NULL); -- cgit v1.2.1 From c90e5293d11bc2c671312778aea33922a4ee1725 Mon Sep 17 00:00:00 2001 From: Jeff King Date: Fri, 11 Mar 2016 17:37:18 -0500 Subject: setup: drop repository_format_version global Nobody reads this anymore, and they're not likely to; the interesting thing is whether or not we passed check_repository_format(), and possibly the individual "extension" variables. Signed-off-by: Jeff King Signed-off-by: Junio C Hamano --- setup.c | 1 - 1 file changed, 1 deletion(-) (limited to 'setup.c') diff --git a/setup.c b/setup.c index fbe7ec16dd..1314c170ab 100644 --- a/setup.c +++ b/setup.c @@ -428,7 +428,6 @@ static int check_repository_format_gently(const char *gitdir, int *nongit_ok) die("%s", err.buf); } - repository_format_version = candidate.version; repository_format_precious_objects = candidate.precious_objects; string_list_clear(&candidate.unknown_extensions, 0); if (!has_common) { -- cgit v1.2.1 From 274db840b48d144a8f0f8d8bd324365670c67275 Mon Sep 17 00:00:00 2001 From: Jeff King Date: Fri, 11 Mar 2016 17:37:22 -0500 Subject: verify_repository_format: mark messages for translation These messages are human-readable and should be translated. Signed-off-by: Jeff King Signed-off-by: Junio C Hamano --- setup.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) (limited to 'setup.c') diff --git a/setup.c b/setup.c index 1314c170ab..09d37204f9 100644 --- a/setup.c +++ b/setup.c @@ -462,7 +462,7 @@ int verify_repository_format(const struct repository_format *format, struct strbuf *err) { if (GIT_REPO_VERSION_READ < format->version) { - strbuf_addf(err, "Expected git repo version <= %d, found %d", + strbuf_addf(err, _("Expected git repo version <= %d, found %d"), GIT_REPO_VERSION_READ, format->version); return -1; } @@ -470,7 +470,7 @@ int verify_repository_format(const struct repository_format *format, if (format->version >= 1 && format->unknown_extensions.nr) { int i; - strbuf_addstr(err, "unknown repository extensions found:"); + strbuf_addstr(err, _("unknown repository extensions found:")); for (i = 0; i < format->unknown_extensions.nr; i++) strbuf_addf(err, "\n\t%s", -- cgit v1.2.1