On Wed, Mar 28, 2018 at 02:19:32PM -0400, Jeff King wrote:
>
> > 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);
