Skip to content

Commit b67b2d9

Browse files
pks-tgitster
authored andcommitted
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>
1 parent f8bdf31 commit b67b2d9

6 files changed

Lines changed: 13 additions & 12 deletions

File tree

odb.c

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -360,7 +360,7 @@ struct odb_source *odb_set_temporary_primary_source(struct object_database *odb,
360360
* Disable ref updates while a temporary odb is active, since
361361
* the objects in the database may roll back.
362362
*/
363-
source->disable_ref_updates = 1;
363+
odb->repo->disable_ref_updates = true;
364364
source->will_destroy = will_destroy;
365365
source->next = odb->sources;
366366
odb->sources = source;
@@ -387,6 +387,7 @@ void odb_restore_primary_source(struct object_database *odb,
387387
if (cur_source->next != restore_source)
388388
BUG("we expect the old primary object store to be the first alternate");
389389

390+
odb->repo->disable_ref_updates = false;
390391
odb->sources = restore_source;
391392
odb_source_free(cur_source);
392393
}

odb.h

Lines changed: 0 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -66,13 +66,6 @@ struct odb_source {
6666
*/
6767
bool local;
6868

69-
/*
70-
* This is a temporary object store created by the tmp_objdir
71-
* facility. Disable ref updates since the objects in the store
72-
* might be discarded on rollback.
73-
*/
74-
int disable_ref_updates;
75-
7669
/*
7770
* This object store is ephemeral, so there is no need to fsync.
7871
*/

refs.c

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -2491,7 +2491,7 @@ int ref_transaction_prepare(struct ref_transaction *transaction,
24912491
break;
24922492
}
24932493

2494-
if (refs->repo->objects->sources->disable_ref_updates) {
2494+
if (refs->repo->disable_ref_updates) {
24952495
strbuf_addstr(err,
24962496
_("ref updates forbidden inside quarantine environment"));
24972497
return -1;

repository.c

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -179,7 +179,7 @@ void repo_set_gitdir(struct repository *repo,
179179
repo->objects->sources->path = objects_path;
180180
}
181181

182-
repo->objects->sources->disable_ref_updates = o->disable_ref_updates;
182+
repo->disable_ref_updates = o->disable_ref_updates;
183183

184184
free(repo->objects->alternate_db);
185185
repo->objects->alternate_db = xstrdup_or_null(o->alternate_db);

repository.h

Lines changed: 8 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -71,6 +71,13 @@ struct repository {
7171
*/
7272
struct ref_store *refs_private;
7373

74+
/*
75+
* Disable ref updates. This is especially used in contexts where
76+
* transactions may still be rolled back so that we don't start to
77+
* reference objects that may vanish.
78+
*/
79+
bool disable_ref_updates;
80+
7481
/*
7582
* A strmap of ref_stores, stored by submodule name, accessible via
7683
* `repo_get_submodule_ref_store()`.
@@ -187,7 +194,7 @@ struct set_gitdir_args {
187194
const char *graft_file;
188195
const char *index_file;
189196
const char *alternate_db;
190-
int disable_ref_updates;
197+
bool disable_ref_updates;
191198
};
192199

193200
void repo_set_gitdir(struct repository *repo, const char *root,

setup.c

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1682,7 +1682,7 @@ void setup_git_env(const char *git_dir)
16821682
args.index_file = getenv_safe(&to_free, INDEX_ENVIRONMENT);
16831683
args.alternate_db = getenv_safe(&to_free, ALTERNATE_DB_ENVIRONMENT);
16841684
if (getenv(GIT_QUARANTINE_ENVIRONMENT)) {
1685-
args.disable_ref_updates = 1;
1685+
args.disable_ref_updates = true;
16861686
}
16871687

16881688
repo_set_gitdir(the_repository, git_dir, &args);

0 commit comments

Comments
 (0)