Merge branch 'jk/relative-directory-fix' into pupu

Some codepaths, including the refs API, get and keep relative
paths, that go out of sync when the process does chdir(2).  The
chdir-notify API is introduced to let these codepaths adjust these
cached paths to the new current directory.

* jk/relative-directory-fix:
  refs: use chdir_notify to update cached relative paths
  set_work_tree: use chdir_notify
  add chdir-notify API
  trace.c: export trace_setup_key
  set_git_dir: die when setenv() fails

11 files changed, 223 insertions, 17 deletions

diff --git a/Makefile b/Makefile
index 4571b3080c..a1a4956dab 100644
--- a/Makefile
+++ b/Makefile
@@ -809,6 +809,7 @@ LIB_OBJS += branch.o
 LIB_OBJS += bulk-checkin.o
 LIB_OBJS += bundle.o
 LIB_OBJS += cache-tree.o
+LIB_OBJS += chdir-notify.o
 LIB_OBJS += checkout.o
 LIB_OBJS += color.o
 LIB_OBJS += column.o
diff --git a/cache.h b/cache.h
index 5fe250dcd8..257f00f69d 100644
--- a/cache.h
+++ b/cache.h
@@ -480,7 +480,7 @@ extern const char *get_git_common_dir(void);
 extern char *get_object_directory(void);
 extern char *get_index_file(void);
 extern char *get_graft_file(void);
-extern int set_git_dir(const char *path);
+extern void set_git_dir(const char *path);
 extern int get_common_dir_noenv(struct strbuf *sb, const char *gitdir);
 extern int get_common_dir(struct strbuf *sb, const char *gitdir);
 extern const char *get_git_namespace(void);
diff --git a/chdir-notify.c b/chdir-notify.c
new file mode 100644
index 0000000000..5f7f2c2ac2
--- /dev/null
+++ b/chdir-notify.c
@@ -0,0 +1,93 @@
+#include "cache.h"
+#include "chdir-notify.h"
+#include "list.h"
+#include "strbuf.h"
+
+struct chdir_notify_entry {
+	const char *name;
+	chdir_notify_callback cb;
+	void *data;
+	struct list_head list;
+};
+static LIST_HEAD(chdir_notify_entries);
+
+void chdir_notify_register(const char *name,
+			   chdir_notify_callback cb,
+			   void *data)
+{
+	struct chdir_notify_entry *e = xmalloc(sizeof(*e));
+	e->name = name;
+	e->cb = cb;
+	e->data = data;
+	list_add_tail(&e->list, &chdir_notify_entries);
+}
+
+static void reparent_cb(const char *name,
+			const char *old_cwd,
+			const char *new_cwd,
+			void *data)
+{
+	char **path = data;
+	char *tmp = *path;
+
+	if (!tmp)
+		return;
+
+	*path = reparent_relative_path(old_cwd, new_cwd, tmp);
+	free(tmp);
+
+	if (name) {
+		trace_printf_key(&trace_setup_key,
+				 "setup: reparent %s to '%s'",
+				 name, *path);
+	}
+}
+
+void chdir_notify_reparent(const char *name, char **path)
+{
+	chdir_notify_register(name, reparent_cb, path);
+}
+
+int chdir_notify(const char *new_cwd)
+{
+	struct strbuf old_cwd = STRBUF_INIT;
+	struct list_head *pos;
+
+	if (strbuf_getcwd(&old_cwd) < 0)
+		return -1;
+	if (chdir(new_cwd) < 0) {
+		int saved_errno = errno;
+		strbuf_release(&old_cwd);
+		errno = saved_errno;
+		return -1;
+	}
+
+	trace_printf_key(&trace_setup_key,
+			 "setup: chdir from '%s' to '%s'",
+			 old_cwd.buf, new_cwd);
+
+	list_for_each(pos, &chdir_notify_entries) {
+		struct chdir_notify_entry *e =
+			list_entry(pos, struct chdir_notify_entry, list);
+		e->cb(e->name, old_cwd.buf, new_cwd, e->data);
+	}
+
+	strbuf_release(&old_cwd);
+	return 0;
+}
+
+char *reparent_relative_path(const char *old_cwd,
+			     const char *new_cwd,
+			     const char *path)
+{
+	char *ret, *full;
+
+	if (is_absolute_path(path))
+		return xstrdup(path);
+
+	full = xstrfmt("%s/%s", old_cwd, path);
+	ret = xstrdup(remove_leading_path(full, new_cwd));
+	free(full);
+
+	return ret;
+}
diff --git a/chdir-notify.h b/chdir-notify.h
new file mode 100644
index 0000000000..366e4c1ee9
--- /dev/null
+++ b/chdir-notify.h
@@ -0,0 +1,73 @@
+#ifndef CHDIR_NOTIFY_H
+#define CHDIR_NOTIFY_H
+
+/*
+ * An API to let code "subscribe" to changes to the current working directory.
+ * The general idea is that some code asks to be notified when the working
+ * directory changes, and other code that calls chdir uses a special wrapper
+ * that notifies everyone.
+ */
+
+/*
+ * Callers who need to know about changes can do:
+ *
+ *   void foo(const char *old_path, const char *new_path, void *data)
+ *   {
+ *	warning("switched from %s to %s!", old_path, new_path);
+ *   }
+ *   ...
+ *   chdir_notify_register("description", foo, data);
+ *
+ * In practice most callers will want to move a relative path to the new root;
+ * they can use the reparent_relative_path() helper for that. If that's all
+ * you're doing, you can also use the convenience function:
+ *
+ *   chdir_notify_reparent("description", &my_path);
+ *
+ * Whenever a chdir event occurs, that will update my_path (if it's relative)
+ * to adjust for the new cwd by freeing any existing string and allocating a
+ * new one.
+ *
+ * Registered functions are called in the order in which they were added. Note
+ * that there's currently no way to remove a function, so make sure that the
+ * data parameter remains valid for the rest of the program.
+ *
+ * The "name" argument is used only for printing trace output from
+ * $GIT_TRACE_SETUP. It may be NULL, but if non-NULL should point to
+ * storage which lasts as long as the registration is active.
+ */
+typedef void (*chdir_notify_callback)(const char *name,
+				      const char *old_cwd,
+				      const char *new_cwd,
+				      void *data);
+void chdir_notify_register(const char *name, chdir_notify_callback cb, void *data);
+void chdir_notify_reparent(const char *name, char **path);
+
+/*
+ *
+ * Callers that want to chdir:
+ *
+ *   chdir_notify(new_path);
+ *
+ * to switch to the new path and notify any callbacks.
+ *
+ * Note that you don't need to chdir_notify() if you're just temporarily moving
+ * to a directory and back, as long as you don't call any subscribed code in
+ * between (but it should be safe to do so if you're unsure).
+ */
+int chdir_notify(const char *new_cwd);
+
+/*
+ * Reparent a relative path from old_root to new_root. For example:
+ *
+ *   reparent_relative_path("/a", "/a/b", "b/rel");
+ *
+ * would return the (newly allocated) string "rel". Note that we may return an
+ * absolute path in some cases (e.g., if the resulting path is not inside
+ * new_cwd).
+ */
+char *reparent_relative_path(const char *old_cwd,
+			     const char *new_cwd,
+			     const char *path);
+
+#endif /* CHDIR_NOTIFY_H */
diff --git a/environment.c b/environment.c
index b72598a2d9..b2355b21d7 100644
--- a/environment.c
+++ b/environment.c
@@ -15,6 +15,7 @@
 #include "commit.h"
 #include "argv-array.h"
 #include "object-store.h"
+#include "chdir-notify.h"
 
 int trust_executable_bit = 1;
 int trust_ctime = 1;
@@ -325,12 +326,31 @@ char *get_graft_file(void)
 	return the_repository->graft_file;
 }
 
-int set_git_dir(const char *path)
+static void set_git_dir_1(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);
 	setup_git_env(path);
-	return 0;
+}
+
+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)
diff --git a/refs/files-backend.c b/refs/files-backend.c
index bec8e30e9e..a92a2aa821 100644
--- a/refs/files-backend.c
+++ b/refs/files-backend.c
@@ -9,6 +9,7 @@
 #include "../lockfile.h"
 #include "../object.h"
 #include "../dir.h"
+#include "../chdir-notify.h"
 
 /*
  * This backend uses the following flags in `ref_update::flags` for
@@ -106,6 +107,11 @@ static struct ref_store *files_ref_store_create(const char *gitdir,
 	refs->packed_ref_store = packed_ref_store_create(sb.buf, flags);
 	strbuf_release(&sb);
 
+	chdir_notify_reparent("files-backend $GIT_DIR",
+			      &refs->gitdir);
+	chdir_notify_reparent("files-backend $GIT_COMMONDIR",
+			      &refs->gitcommondir);
+
 	return ref_store;
 }
 
diff --git a/refs/packed-backend.c b/refs/packed-backend.c
index 65288c6472..369c34f886 100644
--- a/refs/packed-backend.c
+++ b/refs/packed-backend.c
@@ -5,6 +5,7 @@
 #include "packed-backend.h"
 #include "../iterator.h"
 #include "../lockfile.h"
+#include "../chdir-notify.h"
 
 enum mmap_strategy {
 	/*
@@ -202,6 +203,8 @@ struct ref_store *packed_ref_store_create(const char *path,
 	refs->store_flags = store_flags;
 
 	refs->path = xstrdup(path);
+	chdir_notify_reparent("packed-refs", &refs->path);
+
 	return ref_store;
 }
 
diff --git a/setup.c b/setup.c
index 664453fcef..3e03d442b6 100644
--- a/setup.c
+++ b/setup.c
@@ -3,6 +3,7 @@
 #include "config.h"
 #include "dir.h"
 #include "string-list.h"
+#include "chdir-notify.h"
 
 static int inside_git_dir = -1;
 static int inside_work_tree = -1;
@@ -378,7 +379,7 @@ int is_inside_work_tree(void)
 
 void setup_work_tree(void)
 {
-	const char *work_tree, *git_dir;
+	const char *work_tree;
 	static int initialized = 0;
 
 	if (initialized)
@@ -388,10 +389,7 @@ void setup_work_tree(void)
 		die(_("unable to set up work tree using invalid config"));
 
 	work_tree = get_git_work_tree();
-	git_dir = get_git_dir();
-	if (!is_absolute_path(git_dir))
-		git_dir = real_path(get_git_dir());
-	if (!work_tree || chdir(work_tree))
+	if (!work_tree || chdir_notify(work_tree))
 		die(_("this operation must be run in a work tree"));
 
 	/*
@@ -401,7 +399,6 @@ void setup_work_tree(void)
 	if (getenv(GIT_WORK_TREE_ENVIRONMENT))
 		setenv(GIT_WORK_TREE_ENVIRONMENT, ".", 1);
 
-	set_git_dir(remove_leading_path(git_dir, work_tree));
 	initialized = 1;
 }
 
diff --git a/t/t1501-work-tree.sh b/t/t1501-work-tree.sh
index 02cf2013fc..9c0bc65250 100755
--- a/t/t1501-work-tree.sh
+++ b/t/t1501-work-tree.sh
@@ -431,4 +431,16 @@ test_expect_success 'error out gracefully on invalid $GIT_WORK_TREE' '
 	)
 '
 
+test_expect_success 'refs work with relative gitdir and work tree' '
+	git init relative &&
+	git -C relative commit --allow-empty -m one &&
+	git -C relative commit --allow-empty -m two &&
+
+	GIT_DIR=relative/.git GIT_WORK_TREE=relative git reset HEAD^ &&
+
+	git -C relative log -1 --format=%s >actual &&
+	echo one >expect &&
+	test_cmp expect actual
+'
+
 test_done
diff --git a/trace.c b/trace.c
index 7f3b08e148..fc623e91fd 100644
--- a/trace.c
+++ b/trace.c
@@ -26,6 +26,7 @@
 
 struct trace_key trace_default_key = { "GIT_TRACE", 0, 0, 0 };
 struct trace_key trace_perf_key = TRACE_KEY_INIT(PERFORMANCE);
+struct trace_key trace_setup_key = TRACE_KEY_INIT(SETUP);
 
 /* Get a trace file descriptor from "key" env variable. */
 static int get_trace_fd(struct trace_key *key)
@@ -300,11 +301,10 @@ static const char *quote_crnl(const char *path)
 /* FIXME: move prefix to startup_info struct and get rid of this arg */
 void trace_repo_setup(const char *prefix)
 {
-	static struct trace_key key = TRACE_KEY_INIT(SETUP);
 	const char *git_work_tree;
 	char *cwd;
 
-	if (!trace_want(&key))
+	if (!trace_want(&trace_setup_key))
 		return;
 
 	cwd = xgetcwd();
@@ -315,11 +315,11 @@ void trace_repo_setup(const char *prefix)
 	if (!prefix)
 		prefix = "(null)";
 
-	trace_printf_key(&key, "setup: git_dir: %s\n", quote_crnl(get_git_dir()));
-	trace_printf_key(&key, "setup: git_common_dir: %s\n", quote_crnl(get_git_common_dir()));
-	trace_printf_key(&key, "setup: worktree: %s\n", quote_crnl(git_work_tree));
-	trace_printf_key(&key, "setup: cwd: %s\n", quote_crnl(cwd));
-	trace_printf_key(&key, "setup: prefix: %s\n", quote_crnl(prefix));
+	trace_printf_key(&trace_setup_key, "setup: git_dir: %s\n", quote_crnl(get_git_dir()));
+	trace_printf_key(&trace_setup_key, "setup: git_common_dir: %s\n", quote_crnl(get_git_common_dir()));
+	trace_printf_key(&trace_setup_key, "setup: worktree: %s\n", quote_crnl(git_work_tree));
+	trace_printf_key(&trace_setup_key, "setup: cwd: %s\n", quote_crnl(cwd));
+	trace_printf_key(&trace_setup_key, "setup: prefix: %s\n", quote_crnl(prefix));
 
 	free(cwd);
 }
diff --git a/trace.h b/trace.h
index 88055abef7..2b6a1bc17c 100644
--- a/trace.h
+++ b/trace.h
@@ -15,6 +15,7 @@ extern struct trace_key trace_default_key;
 
 #define TRACE_KEY_INIT(name) { "GIT_TRACE_" #name, 0, 0, 0 }
 extern struct trace_key trace_perf_key;
+extern struct trace_key trace_setup_key;
 
 extern void trace_repo_setup(const char *prefix);
 extern int trace_want(struct trace_key *key);