odb: handle changing a repository's commondir

The function `repo_set_gitdir()` is called in two situations:

  - To initialize the repository with its discovered location. As part
    of this we also set up the new object database.

  - To update the repository's discovered location in case the process
    changes its working directory so that we update relative paths. This
    means we also have to update any relative paths that are potentially
    used in the object database.

In the context of the object database we ideally wouldn't ever have to
worry about the second case: if all paths used by our object database
sources were absolute, then we wouldn't have to update them. But
unfortunately, the paths aren't only used to locate files owned by the
given source, but we also use them for reporting purposes. One such
example is `repo_get_object_directory()`, where we cannot just change
semantics to always return absolute paths, as that is likely to break
tooling out there.

One solution to this would be to have both a "display path" and an
"internal path". This would allow us to use internal paths for all
internal matters, but continue to use the potentially-relative display
paths so that we don't break compatibility. But converting the codebase
to honor this split is quite a messy endeavour, and it wouldn't even
help us with the goal to get rid of the need to update the display path
on chdir(3p).

Another solution would be to rework "setup.c" so that we never have to
update paths in the first place. In that case, we'd only initialize the
repository once we have figured out final locations for all directories.
This would be a significant simplification of that subsystem indeed, but
the current logic is so messy that it would take significant investments
to get there.

Meanwhile though, while object sources may still use relative paths, the
best thing we can do is to handle the reparenting of the object source
paths in the object database itself. This can be done by registering one
callback for each object database so that we get notified whenever the
current working directory changes, and we then perform the reparenting
ourselves.

Ideally, this wouldn't even happen on the object database level, but
instead handled by each object database source. But we don't yet have
proper pluggable object database sources, so this will need to be
handled at a later point in time.

The logic itself is rather simple:

  - We register the callback when creating the object database.

  - We unregister the callback when releasing it again.

  - We split up `set_git_dir_1()` so that it becomes possible to skip
    recreating the object database. This is required because the
    function is called both when the current working directory changes,
    but also when we set up the repository. Calling this function
    without skipping creation of the ODB will result in a bug in case
    it's already created.

Signed-off-by: Patrick Steinhardt <ps@pks.im>
Signed-off-by: Junio C Hamano <gitster@pobox.com>

diff --git a/odb.c b/odb.c
index 88b40c81c0060c022c30ce1c19672dfa5779a0f2..70665fb7f48f0ff4662365f5744d5118e70c84a5 100644 (file)
--- a/odb.c
+++ b/odb.c
@@ -1,5 +1,6 @@
 #include "git-compat-util.h"
 #include "abspath.h"
+#include "chdir-notify.h"
 #include "commit-graph.h"
 #include "config.h"
 #include "dir.h"
@@ -142,9 +143,9 @@ static void read_info_alternates(struct object_database *odb,
                                 const char *relative_base,
                                 int depth);
 
-struct odb_source *odb_source_new(struct object_database *odb,
-                                 const char *path,
-                                 bool local)
+static struct odb_source *odb_source_new(struct object_database *odb,
+                                        const char *path,
+                                        bool local)
 {
        struct odb_source *source;
 
@@ -1034,6 +1035,32 @@ int odb_write_object_stream(struct object_database *odb,
        return odb_source_loose_write_stream(odb->sources, stream, len, oid);
 }
 
+static void odb_update_commondir(const char *name UNUSED,
+                                const char *old_cwd,
+                                const char *new_cwd,
+                                void *cb_data)
+{
+       struct object_database *odb = cb_data;
+       struct odb_source *source;
+
+       /*
+        * In theory, we only have to do this for the primary object source, as
+        * alternates' paths are always resolved to an absolute path.
+        */
+       for (source = odb->sources; source; source = source->next) {
+               char *path;
+
+               if (is_absolute_path(source->path))
+                       continue;
+
+               path = reparent_relative_path(old_cwd, new_cwd,
+                                             source->path);
+
+               free(source->path);
+               source->path = path;
+       }
+}
+
 struct object_database *odb_new(struct repository *repo,
                                const char *primary_source,
                                const char *secondary_sources)
@@ -1055,6 +1082,8 @@ struct object_database *odb_new(struct repository *repo,
 
        free(to_free);
 
+       chdir_notify_register(NULL, odb_update_commondir, o);
+
        return o;
 }
 
@@ -1106,6 +1135,8 @@ void odb_free(struct object_database *o)
        packfile_store_free(o->packfiles);
        string_list_clear(&o->submodule_source_paths, 0);
 
+       chdir_notify_unregister(NULL, odb_update_commondir, o);
+
        free(o);
 }
 

diff --git a/odb.h b/odb.h
index 41b3c03027f4d833278592e1b34040f59d055971..014cd9585a2f6efe7367e300afd465906f4a1e3a 100644 (file)
--- a/odb.h
+++ b/odb.h
@@ -78,10 +78,6 @@ struct odb_source {
        char *path;
 };
 
-struct odb_source *odb_source_new(struct object_database *odb,
-                                 const char *path,
-                                 bool local);
-
 struct packed_git;
 struct packfile_store;
 struct cached_object_entry;

diff --git a/repository.c b/repository.c
index 5975c8f341c8cb86fecf4533122f987b10ab5d5f..863f24411b7bf9ac31ee6b2fde1af1b7848d6099 100644 (file)
--- a/repository.c
+++ b/repository.c
@@ -165,17 +165,10 @@ void repo_set_gitdir(struct repository *repo,
 
        repo_set_commondir(repo, o->commondir);
 
-       if (!repo->objects) {
+       if (!repo->objects)
                repo->objects = odb_new(repo, o->object_dir, o->alternate_db);
-       } else {
-               char *objects_path = NULL;
-               expand_base_dir(&objects_path, o->object_dir,
-                               repo->commondir, "objects");
-               free(repo->objects->sources->path);
-               repo->objects->sources->path = objects_path;
-               free(repo->objects->alternate_db);
-               repo->objects->alternate_db = xstrdup_or_null(o->alternate_db);
-       }
+       else if (!o->skip_initializing_odb)
+               BUG("cannot reinitialize an already-initialized object directory");
 
        repo->disable_ref_updates = o->disable_ref_updates;
 

diff --git a/repository.h b/repository.h
index 614649413b68bc6d665c893cdc2dcf9a284e2321..6063c4b846d031d657827f8b16d65af8c09e5b29 100644 (file)
--- a/repository.h
+++ b/repository.h
@@ -195,6 +195,7 @@ struct set_gitdir_args {
        const char *index_file;
        const char *alternate_db;
        bool disable_ref_updates;
+       bool skip_initializing_odb;
 };
 
 void repo_set_gitdir(struct repository *repo, const char *root,

diff --git a/setup.c b/setup.c
index a752e9fc8476a05e3550f4b69b5f70fd1488748d..a625f9fbc8b1b7fb470ec652a332a8da638f3dbd 100644 (file)
--- a/setup.c
+++ b/setup.c
@@ -1002,10 +1002,51 @@ const char *read_gitfile_gently(const char *path, int *return_error_code)
        return error_code ? NULL : path;
 }
 
-static void set_git_dir_1(const char *path)
+static void setup_git_env_internal(const char *git_dir,
+                                  bool skip_initializing_odb)
+{
+       char *git_replace_ref_base;
+       const char *shallow_file;
+       const char *replace_ref_base;
+       struct set_gitdir_args args = { NULL };
+       struct strvec to_free = STRVEC_INIT;
+
+       args.commondir = getenv_safe(&to_free, GIT_COMMON_DIR_ENVIRONMENT);
+       args.object_dir = getenv_safe(&to_free, DB_ENVIRONMENT);
+       args.graft_file = getenv_safe(&to_free, GRAFT_ENVIRONMENT);
+       args.index_file = getenv_safe(&to_free, INDEX_ENVIRONMENT);
+       args.alternate_db = getenv_safe(&to_free, ALTERNATE_DB_ENVIRONMENT);
+       if (getenv(GIT_QUARANTINE_ENVIRONMENT))
+               args.disable_ref_updates = true;
+       args.skip_initializing_odb = skip_initializing_odb;
+
+       repo_set_gitdir(the_repository, git_dir, &args);
+       strvec_clear(&to_free);
+
+       if (getenv(NO_REPLACE_OBJECTS_ENVIRONMENT))
+               disable_replace_refs();
+       replace_ref_base = getenv(GIT_REPLACE_REF_BASE_ENVIRONMENT);
+       git_replace_ref_base = xstrdup(replace_ref_base ? replace_ref_base
+                                                         : "refs/replace/");
+       update_ref_namespace(NAMESPACE_REPLACE, git_replace_ref_base);
+
+       shallow_file = getenv(GIT_SHALLOW_FILE_ENVIRONMENT);
+       if (shallow_file)
+               set_alternate_shallow_file(the_repository, shallow_file, 0);
+
+       if (git_env_bool(NO_LAZY_FETCH_ENVIRONMENT, 0))
+               fetch_if_missing = 0;
+}
+
+void setup_git_env(const char *git_dir)
+{
+       setup_git_env_internal(git_dir, false);
+}
+
+static void set_git_dir_1(const char *path, bool skip_initializing_odb)
 {
        xsetenv(GIT_DIR_ENVIRONMENT, path, 1);
-       setup_git_env(path);
+       setup_git_env_internal(path, skip_initializing_odb);
 }
 
 static void update_relative_gitdir(const char *name UNUSED,
@@ -1020,7 +1061,7 @@ static void update_relative_gitdir(const char *name UNUSED,
        trace_printf_key(&trace_setup_key,
                         "setup: move $GIT_DIR to '%s'",
                         path);
-       set_git_dir_1(path);
+       set_git_dir_1(path, true);
        if (tmp_objdir)
                tmp_objdir_reapply_primary_odb(tmp_objdir, old_cwd, new_cwd);
        free(path);
@@ -1035,7 +1076,7 @@ static void set_git_dir(const char *path, int make_realpath)
                path = realpath.buf;
        }
 
-       set_git_dir_1(path);
+       set_git_dir_1(path, false);
        if (!is_absolute_path(path))
                chdir_notify_register(NULL, update_relative_gitdir, NULL);
 
@@ -1668,41 +1709,6 @@ enum discovery_result discover_git_directory_reason(struct strbuf *commondir,
        return result;
 }
 
-void setup_git_env(const char *git_dir)
-{
-       char *git_replace_ref_base;
-       const char *shallow_file;
-       const char *replace_ref_base;
-       struct set_gitdir_args args = { NULL };
-       struct strvec to_free = STRVEC_INIT;
-
-       args.commondir = getenv_safe(&to_free, GIT_COMMON_DIR_ENVIRONMENT);
-       args.object_dir = getenv_safe(&to_free, DB_ENVIRONMENT);
-       args.graft_file = getenv_safe(&to_free, GRAFT_ENVIRONMENT);
-       args.index_file = getenv_safe(&to_free, INDEX_ENVIRONMENT);
-       args.alternate_db = getenv_safe(&to_free, ALTERNATE_DB_ENVIRONMENT);
-       if (getenv(GIT_QUARANTINE_ENVIRONMENT)) {
-               args.disable_ref_updates = true;
-       }
-
-       repo_set_gitdir(the_repository, git_dir, &args);
-       strvec_clear(&to_free);
-
-       if (getenv(NO_REPLACE_OBJECTS_ENVIRONMENT))
-               disable_replace_refs();
-       replace_ref_base = getenv(GIT_REPLACE_REF_BASE_ENVIRONMENT);
-       git_replace_ref_base = xstrdup(replace_ref_base ? replace_ref_base
-                                                         : "refs/replace/");
-       update_ref_namespace(NAMESPACE_REPLACE, git_replace_ref_base);
-
-       shallow_file = getenv(GIT_SHALLOW_FILE_ENVIRONMENT);
-       if (shallow_file)
-               set_alternate_shallow_file(the_repository, shallow_file, 0);
-
-       if (git_env_bool(NO_LAZY_FETCH_ENVIRONMENT, 0))
-               fetch_if_missing = 0;
-}
-
 const char *enter_repo(const char *path, unsigned flags)
 {
        static struct strbuf validated_path = STRBUF_INIT;