odb: move logic to disable ref updates into repo

Our object database sources have a field `disable_ref_updates`. This
field can obviously be set to disable reference updates, but it is
somewhat curious that this logic is hosted by the object database.

The reason for this is that it was primarily added to keep us from
accidentally updating references while an ODB transaction is ongoing.
Any objects part of the transaction have not yet been committed to disk,
so new references that point to them might get corrupted in case we
never end up committing the transaction. As such, whenever we create a
new transaction we set up a new temporary ODB source and mark it as
disabling reference updates.

This has one (and only one?) upside: once we have committed the
transaction, the temporary source will be dropped and thus we clean up
the disabled reference updates automatically. But other than that, it's
somewhat misdesigned:

  - We can have multiple ODB sources, but only the currently active
    source inhibits reference updates.

  - We're mixing concerns of the refdb with the ODB.

Arguably, the decision of whether we can update references or not should
be handled by the refdb. But that wouldn't be a great fit either, as
there can be one refdb per worktree. So we'd again have the same problem
that a "global" intent becomes localized to a specific instance.

Instead, move the setting into the repository. While at it, convert it
into a boolean.

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 29cf6496c5e50aff2e67b0cc81d08b97ae0b613f..ccc6e999e7ae25d3d0d36379b94115cfdc731053 100644 (file)
--- a/odb.c
+++ b/odb.c
@@ -360,7 +360,7 @@ struct odb_source *odb_set_temporary_primary_source(struct object_database *odb,
         * Disable ref updates while a temporary odb is active, since
         * the objects in the database may roll back.
         */
-       source->disable_ref_updates = 1;
+       odb->repo->disable_ref_updates = true;
        source->will_destroy = will_destroy;
        source->next = odb->sources;
        odb->sources = source;
@@ -387,6 +387,7 @@ void odb_restore_primary_source(struct object_database *odb,
        if (cur_source->next != restore_source)
                BUG("we expect the old primary object store to be the first alternate");
 
+       odb->repo->disable_ref_updates = false;
        odb->sources = restore_source;
        odb_source_free(cur_source);
 }

diff --git a/odb.h b/odb.h
index 77b313b784cad38303ee4aecc0f2f6f1d3a9d7b7..99c4d4897294597873461bbfd20bf9fcbb24dfa8 100644 (file)
--- a/odb.h
+++ b/odb.h
@@ -66,13 +66,6 @@ struct odb_source {
         */
        bool local;
 
-       /*
-        * This is a temporary object store created by the tmp_objdir
-        * facility. Disable ref updates since the objects in the store
-        * might be discarded on rollback.
-        */
-       int disable_ref_updates;
-
        /*
         * This object store is ephemeral, so there is no need to fsync.
         */

diff --git a/refs.c b/refs.c
index 965381367e0e53914040d77bea12975354c8b854..6c7283d9eb2aa89c354dd86497bf983f30994ed3 100644 (file)
--- a/refs.c
+++ b/refs.c
@@ -2491,7 +2491,7 @@ int ref_transaction_prepare(struct ref_transaction *transaction,
                break;
        }
 
-       if (refs->repo->objects->sources->disable_ref_updates) {
+       if (refs->repo->disable_ref_updates) {
                strbuf_addstr(err,
                              _("ref updates forbidden inside quarantine environment"));
                return -1;

diff --git a/repository.c b/repository.c
index 3c8b3813b00af00c30d1954ffd656744cfe5cfbb..455c2d279fb8ab7cf0edb498f65576be4f95a4e0 100644 (file)
--- a/repository.c
+++ b/repository.c
@@ -179,7 +179,7 @@ void repo_set_gitdir(struct repository *repo,
                repo->objects->sources->path = objects_path;
        }
 
-       repo->objects->sources->disable_ref_updates = o->disable_ref_updates;
+       repo->disable_ref_updates = o->disable_ref_updates;
 
        free(repo->objects->alternate_db);
        repo->objects->alternate_db = xstrdup_or_null(o->alternate_db);

diff --git a/repository.h b/repository.h
index 5808a5d610846a0e42233f66e56dcbcebbd3ecd0..614649413b68bc6d665c893cdc2dcf9a284e2321 100644 (file)
--- a/repository.h
+++ b/repository.h
@@ -71,6 +71,13 @@ struct repository {
         */
        struct ref_store *refs_private;
 
+       /*
+        * Disable ref updates. This is especially used in contexts where
+        * transactions may still be rolled back so that we don't start to
+        * reference objects that may vanish.
+        */
+       bool disable_ref_updates;
+
        /*
         * A strmap of ref_stores, stored by submodule name, accessible via
         * `repo_get_submodule_ref_store()`.
@@ -187,7 +194,7 @@ struct set_gitdir_args {
        const char *graft_file;
        const char *index_file;
        const char *alternate_db;
-       int disable_ref_updates;
+       bool disable_ref_updates;
 };
 
 void repo_set_gitdir(struct repository *repo, const char *root,

diff --git a/setup.c b/setup.c
index 8bf52df71663a350071a79f2e3ac60f9e94d663c..a752e9fc8476a05e3550f4b69b5f70fd1488748d 100644 (file)
--- a/setup.c
+++ b/setup.c
@@ -1682,7 +1682,7 @@ void setup_git_env(const char *git_dir)
        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 = 1;
+               args.disable_ref_updates = true;
        }
 
        repo_set_gitdir(the_repository, git_dir, &args);