From 48988c4d0c35b5c569a2645b61c9346f6062021d Mon Sep 17 00:00:00 2001 From: Jeff King Date: Fri, 30 Mar 2018 14:34:46 -0400 Subject: set_git_dir: die when setenv() fails The set_git_dir() function returns an error if setenv() fails, but there are zero callers who pay attention to this return value. If this ever were to happen, it could cause confusing results, as sub-processes would see a potentially stale GIT_DIR (e.g., if it is relative and we chdir()-ed to the root of the working tree). We _could_ try to fix each caller, but there's really nothing useful to do after this failure except die. Let's just lump setenv() failure into the same category as malloc failure: things that should never happen and cause us to abort catastrophically. Signed-off-by: Jeff King Signed-off-by: Junio C Hamano --- environment.c | 5 ++--- 1 file changed, 2 insertions(+), 3 deletions(-) (limited to 'environment.c') diff --git a/environment.c b/environment.c index d6dd64662c..e01acf8b11 100644 --- a/environment.c +++ b/environment.c @@ -296,13 +296,12 @@ char *get_graft_file(void) return the_repository->graft_file; } -int set_git_dir(const char *path) +void set_git_dir(const char *path) { if (setenv(GIT_DIR_ENVIRONMENT, path, 1)) - return error("Could not set GIT_DIR to '%s'", path); + die("could not set GIT_DIR to '%s'", path); repo_set_gitdir(the_repository, path); setup_git_env(); - return 0; } const char *get_log_output_encoding(void) -- cgit v1.2.3 From 8500e0de3fe2e33ba503d432a4a5301ce2fb60fa Mon Sep 17 00:00:00 2001 From: Jeff King Date: Fri, 30 Mar 2018 14:35:08 -0400 Subject: set_work_tree: use chdir_notify When we change to the top of the working tree, we manually re-adjust $GIT_DIR and call set_git_dir() again, in order to update any relative git-dir we'd compute earlier. Instead of the work-tree code having to know to call the git-dir code, let's use the new chdir_notify interface. There are two spots that need updating, with a few subtleties in each: 1. the set_git_dir() code needs to chdir_notify_register() so it can be told when to update its path. Technically we could push this down into repo_set_gitdir(), so that even repository structs besides the_repository could benefit from this. But that opens up a lot of complications: - we'd still need to touch set_git_dir(), because it does some other setup (like setting $GIT_DIR in the environment) - submodules using other repository structs get cleaned up, which means we'd need to remove them from the chdir_notify list - it's unlikely to fix any bugs, since we shouldn't generally chdir() in the middle of working on a submodule 2. setup_work_tree now needs to call chdir_notify(), and can lose its manual set_git_dir() call. Note that at first glance it looks like this undoes the absolute-to-relative optimization added by 044bbbcb63 (Make git_dir a path relative to work_tree in setup_work_tree(), 2008-06-19). But for the most part that optimization was just _undoing_ the relative-to-absolute conversion which the function was doing earlier (and which is now gone). It is true that if you already have an absolute git_dir that the setup_work_tree() function will no longer make it relative as a side effect. But: - we generally do have relative git-dir's due to the way the discovery code works - if we really care about making git-dir's relative when possible, then we should be relativizing them earlier (e.g., when we see an absolute $GIT_DIR we could turn it relative, whether we are going to chdir into a worktree or not). That would cover all cases, including ones that 044bbbcb63 did not. Signed-off-by: Jeff King Signed-off-by: Junio C Hamano --- environment.c | 23 ++++++++++++++++++++++- 1 file changed, 22 insertions(+), 1 deletion(-) (limited to 'environment.c') diff --git a/environment.c b/environment.c index e01acf8b11..903a6c9df7 100644 --- a/environment.c +++ b/environment.c @@ -13,6 +13,7 @@ #include "refs.h" #include "fmt-merge-msg.h" #include "commit.h" +#include "chdir-notify.h" int trust_executable_bit = 1; int trust_ctime = 1; @@ -296,7 +297,7 @@ char *get_graft_file(void) return the_repository->graft_file; } -void set_git_dir(const char *path) +static void set_git_dir_1(const char *path) { if (setenv(GIT_DIR_ENVIRONMENT, path, 1)) die("could not set GIT_DIR to '%s'", path); @@ -304,6 +305,26 @@ void set_git_dir(const char *path) setup_git_env(); } +static void update_relative_gitdir(const char *name, + const char *old_cwd, + const char *new_cwd, + void *data) +{ + char *path = reparent_relative_path(old_cwd, new_cwd, get_git_dir()); + trace_printf_key(&trace_setup_key, + "setup: move $GIT_DIR to '%s'", + path); + set_git_dir_1(path); + free(path); +} + +void set_git_dir(const char *path) +{ + set_git_dir_1(path); + if (!is_absolute_path(path)) + chdir_notify_register(NULL, update_relative_gitdir, NULL); +} + const char *get_log_output_encoding(void) { return git_log_output_encoding ? git_log_output_encoding -- cgit v1.2.3