Re: [PATCH 0/8] Re: git complains packed-refs is not a directory when used with GIT_DIR and GIT_WORK_TREE envvars.
- From
Duy Nguyen <pclouds@gmail.com>
- Date
- Mar 29, 2018, 14:57 UTC
- Message-ID
- <20180329145726.GA10253@duynguyen.home>
- In-Reply-To
- <20180328181932.GB16565@sigill.intra.peff.net>
On Wed, Mar 28, 2018 at 02:19:32PM -0400, Jeff King wrote:
Show 7 quoted lines
> > > I will probably rework on top of your chdir-notify instead (and let > > yours to be merged earlier) > > Thanks. I like some of the related changes you made, like including this > in the tracing output. That should be easy to do on top of mine, I > think.
Yeah. But is it possible to sneak something like this in your series (I assume you will reroll anyway)? I could do it separately, but it looks nicer if it's split out and merged in individual patches that add new chdir-notify call site.
diff --git a/chdir-notify.c b/chdir-notify.c index 7034e98d71..a2a33443f8 100644 --- a/chdir-notify.c +++ b/chdir-notify.c @@ -4,21 +4,26 @@ #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(chdir_notify_callback cb, void *data) +void chdir_notify_register(const char *name, + chdir_notify_callback cb, + void *data) { struct chdir_notify_entry *e = xmalloc(sizeof(*e)); e->cb = cb; e->data = data; + e->name = name; list_add_tail(&e->list, &chdir_notify_entries); } -static void reparent_cb(const char *old_cwd, +static void reparent_cb(const char *name, + const char *old_cwd, const char *new_cwd, void *data) { @@ -28,12 +33,16 @@ static void reparent_cb(const char *old_cwd, if (!tmp || !is_absolute_path(tmp)) return; *path = reparent_relative_path(old_cwd, new_cwd, tmp); + if (name) + trace_printf_key(&trace_setup_key, + "setup: reparent %s to '%s'", + name, *path); free(tmp); } -void chdir_notify_reparent(char **path) +void chdir_notify_reparent(const char *name, char **path) { - chdir_notify_register(reparent_cb, path); + chdir_notify_register(name, reparent_cb, path); } int chdir_notify(const char *new_cwd) @@ -45,11 +54,12 @@ int chdir_notify(const char *new_cwd) return -1; if (chdir(new_cwd) < 0) return -1; + trace_printf_key(&trace_setup_key, "setup: chdir to '%s'", new_cwd); list_for_each(pos, &chdir_notify_entries) { struct chdir_notify_entry *e = list_entry(pos, struct chdir_notify_entry, list); - e->cb(old_cwd.buf, new_cwd, e->data); + e->cb(e->name, old_cwd.buf, new_cwd, e->data); } strbuf_release(&old_cwd); diff --git a/chdir-notify.h b/chdir-notify.h index c3bd818a85..b9be1b54ac 100644 --- a/chdir-notify.h +++ b/chdir-notify.h @@ -16,23 +16,29 @@ * warning("switched from %s to %s!", old_path, new_path); * } * ... - * chdir_notify_register(foo, data); + * 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(&my_path); + * chdir_notify_reparent("description", &my_path); * * 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 first argument is used for tracing purpose only. when my_path is updated + * by chdir_notify_reparent() it will print a message if $GIT_TRACE_SETUP is set. + * This argument could be NULL. */ -typedef void (*chdir_notify_callback)(const char *old_cwd, +typedef void (*chdir_notify_callback)(const char *name, + const char *old_cwd, const char *new_cwd, void *data); -void chdir_notify_register(chdir_notify_callback cb, void *data); -void chdir_notify_reparent(char **path); +void chdir_notify_register(const char *name, chdir_notify_callback cb, + void *data); +void chdir_notify_reparent(const char *name, char **path); /* * diff --git a/environment.c b/environment.c index 794a75a717..92701cbc0c 100644 --- a/environment.c +++ b/environment.c @@ -330,11 +330,15 @@ static void set_git_dir_1(const char *path) setup_git_env(path); } -static void update_relative_gitdir(const char *old_cwd, +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 $GIR_DIR and others to '%s'", + path); set_git_dir_1(path); free(path); } @@ -343,7 +347,7 @@ void set_git_dir(const char *path) { set_git_dir_1(path); if (!is_absolute_path(path)) - chdir_notify_register(update_relative_gitdir, NULL); + 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 ab3e00ffa0..861072c0dc 100644 --- a/refs/files-backend.c +++ b/refs/files-backend.c @@ -107,8 +107,8 @@ 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(&refs->gitdir); - chdir_notify_reparent(&refs->gitcommondir); + chdir_notify_reparent("files-backend $GIT_DIR", &refs->gitdir); + chdir_notify_reparent("files-backedn $GIT_COMMONDIR", &refs->gitcommondir); return ref_store; } diff --git a/refs/packed-backend.c b/refs/packed-backend.c index 6725742f00..369c34f886 100644 --- a/refs/packed-backend.c +++ b/refs/packed-backend.c @@ -203,7 +203,7 @@ struct ref_store *packed_ref_store_create(const char *path, refs->store_flags = store_flags; refs->path = xstrdup(path); - chdir_notify_reparent(&refs->path); + chdir_notify_reparent("packed-refs", &refs->path); return ref_store; } 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);