diff --git a/branch.c b/branch.c index 44977ad0aa..ebaa870c01 100644 --- a/branch.c +++ b/branch.c @@ -627,7 +627,7 @@ void create_branch(struct repository *r, else msg = xstrfmt("branch: Created from %s", start_name); transaction = ref_store_transaction_begin(get_main_ref_store(the_repository), - &err); + 0, &err); if (!transaction || ref_transaction_update(transaction, ref.buf, &oid, forcing ? NULL : null_oid(), diff --git a/builtin/clone.c b/builtin/clone.c index d3022a185a..21721db28a 100644 --- a/builtin/clone.c +++ b/builtin/clone.c @@ -574,7 +574,7 @@ static void write_remote_refs(const struct ref *local_refs) struct strbuf err = STRBUF_INIT; t = ref_store_transaction_begin(get_main_ref_store(the_repository), - &err); + REF_TRANSACTION_FLAG_INITIAL, &err); if (!t) die("%s", err.buf); @@ -586,7 +586,7 @@ static void write_remote_refs(const struct ref *local_refs) die("%s", err.buf); } - if (initial_ref_transaction_commit(t, &err)) + if (ref_transaction_commit(t, &err)) die("%s", err.buf); strbuf_release(&err); diff --git a/builtin/fast-import.c b/builtin/fast-import.c index 51c8228cb7..457cdb40cc 100644 --- a/builtin/fast-import.c +++ b/builtin/fast-import.c @@ -1647,7 +1647,7 @@ static int update_branch(struct branch *b) } } transaction = ref_store_transaction_begin(get_main_ref_store(the_repository), - &err); + 0, &err); if (!transaction || ref_transaction_update(transaction, b->name, &b->oid, &old_oid, NULL, NULL, 0, msg, &err) || @@ -1682,7 +1682,7 @@ static void dump_tags(void) struct ref_transaction *transaction; transaction = ref_store_transaction_begin(get_main_ref_store(the_repository), - &err); + 0, &err); if (!transaction) { failure |= error("%s", err.buf); goto cleanup; diff --git a/builtin/fetch.c b/builtin/fetch.c index 06b4611958..335083eb10 100644 --- a/builtin/fetch.c +++ b/builtin/fetch.c @@ -665,7 +665,7 @@ static int s_update_ref(const char *action, */ if (!transaction) { transaction = our_transaction = ref_store_transaction_begin(get_main_ref_store(the_repository), - &err); + 0, &err); if (!transaction) { ret = STORE_REF_ERROR_OTHER; goto out; @@ -1667,7 +1667,7 @@ static int do_fetch(struct transport *transport, if (atomic_fetch) { transaction = ref_store_transaction_begin(get_main_ref_store(the_repository), - &err); + 0, &err); if (!transaction) { retcode = -1; goto cleanup; diff --git a/builtin/receive-pack.c b/builtin/receive-pack.c index ab5b20e39c..9d2c07f68d 100644 --- a/builtin/receive-pack.c +++ b/builtin/receive-pack.c @@ -1849,7 +1849,7 @@ static void execute_commands_non_atomic(struct command *commands, continue; transaction = ref_store_transaction_begin(get_main_ref_store(the_repository), - &err); + 0, &err); if (!transaction) { rp_error("%s", err.buf); strbuf_reset(&err); @@ -1878,7 +1878,7 @@ static void execute_commands_atomic(struct command *commands, const char *reported_error = "atomic push failure"; transaction = ref_store_transaction_begin(get_main_ref_store(the_repository), - &err); + 0, &err); if (!transaction) { rp_error("%s", err.buf); strbuf_reset(&err); diff --git a/builtin/replace.c b/builtin/replace.c index a44f4e7ea9..a4eaadff91 100644 --- a/builtin/replace.c +++ b/builtin/replace.c @@ -201,7 +201,7 @@ static int replace_object_oid(const char *object_ref, } transaction = ref_store_transaction_begin(get_main_ref_store(the_repository), - &err); + 0, &err); if (!transaction || ref_transaction_update(transaction, ref.buf, repl, &prev, NULL, NULL, 0, NULL, &err) || diff --git a/builtin/tag.c b/builtin/tag.c index c37c0a68fd..5e1f904d35 100644 --- a/builtin/tag.c +++ b/builtin/tag.c @@ -681,7 +681,7 @@ int cmd_tag(int argc, } transaction = ref_store_transaction_begin(get_main_ref_store(the_repository), - &err); + 0, &err); if (!transaction || ref_transaction_update(transaction, ref.buf, &object, &prev, NULL, NULL, diff --git a/builtin/update-ref.c b/builtin/update-ref.c index 8a98615dc8..670e7812d6 100644 --- a/builtin/update-ref.c +++ b/builtin/update-ref.c @@ -612,7 +612,7 @@ static void update_refs_stdin(void) int i, j; transaction = ref_store_transaction_begin(get_main_ref_store(the_repository), - &err); + 0, &err); if (!transaction) die("%s", err.buf); @@ -680,7 +680,7 @@ static void update_refs_stdin(void) */ state = cmd->state; transaction = ref_store_transaction_begin(get_main_ref_store(the_repository), - &err); + 0, &err); if (!transaction) die("%s", err.buf); diff --git a/refs.c b/refs.c index 7866cd7378..b5657cb6eb 100644 --- a/refs.c +++ b/refs.c @@ -918,7 +918,7 @@ int refs_delete_ref(struct ref_store *refs, const char *msg, struct ref_transaction *transaction; struct strbuf err = STRBUF_INIT; - transaction = ref_store_transaction_begin(refs, &err); + transaction = ref_store_transaction_begin(refs, 0, &err); if (!transaction || ref_transaction_delete(transaction, refname, old_oid, NULL, flags, msg, &err) || @@ -1116,6 +1116,7 @@ int read_ref_at(struct ref_store *refs, const char *refname, } struct ref_transaction *ref_store_transaction_begin(struct ref_store *refs, + unsigned int flags, struct strbuf *err) { struct ref_transaction *tr; @@ -1123,6 +1124,7 @@ struct ref_transaction *ref_store_transaction_begin(struct ref_store *refs, CALLOC_ARRAY(tr, 1); tr->ref_store = refs; + tr->flags = flags; return tr; } @@ -1186,8 +1188,9 @@ struct ref_update *ref_transaction_add_update( oidcpy(&update->new_oid, new_oid); if ((flags & REF_HAVE_OLD) && old_oid) oidcpy(&update->old_oid, old_oid); + if (!(flags & REF_SKIP_CREATE_REFLOG)) + update->msg = normalize_reflog_message(msg); - update->msg = normalize_reflog_message(msg); return update; } @@ -1309,7 +1312,7 @@ int refs_update_ref(struct ref_store *refs, const char *msg, struct strbuf err = STRBUF_INIT; int ret = 0; - t = ref_store_transaction_begin(refs, &err); + t = ref_store_transaction_begin(refs, 0, &err); if (!t || ref_transaction_update(t, refname, new_oid, old_oid, NULL, NULL, flags, msg, &err) || @@ -2120,7 +2123,7 @@ int refs_update_symref(struct ref_store *refs, const char *ref, struct strbuf err = STRBUF_INIT; int ret = 0; - transaction = ref_store_transaction_begin(refs, &err); + transaction = ref_store_transaction_begin(refs, 0, &err); if (!transaction || ref_transaction_update(transaction, ref, NULL, NULL, target, NULL, REF_NO_DEREF, @@ -2316,7 +2319,7 @@ int ref_transaction_commit(struct ref_transaction *transaction, } ret = refs->be->transaction_finish(refs, transaction, err); - if (!ret) + if (!ret && !(transaction->flags & REF_TRANSACTION_FLAG_INITIAL)) run_transaction_hook(transaction, "committed"); return ret; } @@ -2325,6 +2328,7 @@ int refs_verify_refname_available(struct ref_store *refs, const char *refname, const struct string_list *extras, const struct string_list *skip, + unsigned int initial_transaction, struct strbuf *err) { const char *slash; @@ -2333,8 +2337,6 @@ int refs_verify_refname_available(struct ref_store *refs, struct strbuf referent = STRBUF_INIT; struct object_id oid; unsigned int type; - struct ref_iterator *iter; - int ok; int ret = -1; /* @@ -2364,7 +2366,8 @@ int refs_verify_refname_available(struct ref_store *refs, if (skip && string_list_has_string(skip, dirname.buf)) continue; - if (!refs_read_raw_ref(refs, dirname.buf, &oid, &referent, + if (!initial_transaction && + !refs_read_raw_ref(refs, dirname.buf, &oid, &referent, &type, &ignore_errno)) { strbuf_addf(err, _("'%s' exists; cannot create '%s'"), dirname.buf, refname); @@ -2389,22 +2392,27 @@ int refs_verify_refname_available(struct ref_store *refs, strbuf_addstr(&dirname, refname + dirname.len); strbuf_addch(&dirname, '/'); - iter = refs_ref_iterator_begin(refs, dirname.buf, NULL, 0, - DO_FOR_EACH_INCLUDE_BROKEN); - while ((ok = ref_iterator_advance(iter)) == ITER_OK) { - if (skip && - string_list_has_string(skip, iter->refname)) - continue; + if (!initial_transaction) { + struct ref_iterator *iter; + int ok; - strbuf_addf(err, _("'%s' exists; cannot create '%s'"), - iter->refname, refname); - ref_iterator_abort(iter); - goto cleanup; + iter = refs_ref_iterator_begin(refs, dirname.buf, NULL, 0, + DO_FOR_EACH_INCLUDE_BROKEN); + while ((ok = ref_iterator_advance(iter)) == ITER_OK) { + if (skip && + string_list_has_string(skip, iter->refname)) + continue; + + strbuf_addf(err, _("'%s' exists; cannot create '%s'"), + iter->refname, refname); + ref_iterator_abort(iter); + goto cleanup; + } + + if (ok != ITER_DONE) + BUG("error while iterating over references"); } - if (ok != ITER_DONE) - BUG("error while iterating over references"); - extra_refname = find_descendant_ref(dirname.buf, extras, skip); if (extra_refname) strbuf_addf(err, _("cannot process '%s' and '%s' at the same time"), @@ -2487,14 +2495,6 @@ int refs_reflog_expire(struct ref_store *refs, cleanup_fn, policy_cb_data); } -int initial_ref_transaction_commit(struct ref_transaction *transaction, - struct strbuf *err) -{ - struct ref_store *refs = transaction->ref_store; - - return refs->be->initial_transaction_commit(refs, transaction, err); -} - void ref_transaction_for_each_queued_update(struct ref_transaction *transaction, ref_transaction_for_each_queued_update_fn cb, void *cb_data) @@ -2530,7 +2530,7 @@ int refs_delete_refs(struct ref_store *refs, const char *logmsg, * individual updates can't fail, so we can pack all of the * updates into a single transaction. */ - transaction = ref_store_transaction_begin(refs, &err); + transaction = ref_store_transaction_begin(refs, 0, &err); if (!transaction) { ret = error("%s", err.buf); goto out; @@ -2836,7 +2836,8 @@ int repo_migrate_ref_storage_format(struct repository *repo, if (ret < 0) goto done; - transaction = ref_store_transaction_begin(new_refs, errbuf); + transaction = ref_store_transaction_begin(new_refs, REF_TRANSACTION_FLAG_INITIAL, + errbuf); if (!transaction) goto done; @@ -2861,13 +2862,6 @@ int repo_migrate_ref_storage_format(struct repository *repo, if (ret < 0) goto done; - /* - * TODO: we might want to migrate to `initial_ref_transaction_commit()` - * here, which is more efficient for the files backend because it would - * write new refs into the packed-refs file directly. At this point, - * the files backend doesn't handle pseudo-refs and symrefs correctly - * though, so this requires some more work. - */ ret = ref_transaction_commit(transaction, errbuf); if (ret < 0) goto done; diff --git a/refs.h b/refs.h index 108dfc93b3..95baf194ba 100644 --- a/refs.h +++ b/refs.h @@ -101,13 +101,16 @@ int refs_read_symbolic_ref(struct ref_store *ref_store, const char *refname, * both "foo" and with "foo/bar/baz" but not with "foo/bar" or * "foo/barbados". * + * If `initial_transaction` is truish, then all collision checks with + * preexisting refs are skipped. + * * extras and skip must be sorted. */ - int refs_verify_refname_available(struct ref_store *refs, const char *refname, const struct string_list *extras, const struct string_list *skip, + unsigned int initial_transaction, struct strbuf *err); int refs_ref_exists(struct ref_store *refs, const char *refname); @@ -214,11 +217,9 @@ char *repo_default_branch_name(struct repository *r, int quiet); * * Or * - * - Call `initial_ref_transaction_commit()` if the ref database is - * known to be empty and have no other writers (e.g. during - * clone). This is likely to be much faster than - * `ref_transaction_commit()`. `ref_transaction_prepare()` should - * *not* be called before `initial_ref_transaction_commit()`. + * - Call `ref_transaction_begin()` with REF_TRANSACTION_FLAG_INITIAL if the + * ref database is known to be empty and have no other writers (e.g. during + * clone). This is likely to be much faster than without the flag. * * - Then finally, call `ref_transaction_free()` to free the * `ref_transaction` data structure. @@ -234,7 +235,7 @@ char *repo_default_branch_name(struct repository *r, int quiet); * struct strbuf err = STRBUF_INIT; * int ret = 0; * - * transaction = ref_store_transaction_begin(refs, &err); + * transaction = ref_store_transaction_begin(refs, 0, &err); * if (!transaction || * ref_transaction_update(...) || * ref_transaction_create(...) || @@ -579,11 +580,27 @@ enum action_on_err { UPDATE_REFS_QUIET_ON_ERR }; +enum ref_transaction_flag { + /* + * The ref transaction is part of the initial creation of the ref store + * and can thus assume that the ref store is completely empty. This + * allows the backend to perform the transaction more efficiently by + * skipping certain checks. + * + * It is a bug to set this flag when there might be other processes + * accessing the repository or if there are existing references that + * might conflict with the ones being created. All old_oid values must + * either be absent or null_oid. + */ + REF_TRANSACTION_FLAG_INITIAL = (1 << 0), +}; + /* * Begin a reference transaction. The reference transaction must * be freed by calling ref_transaction_free(). */ struct ref_transaction *ref_store_transaction_begin(struct ref_store *refs, + unsigned int flags, struct strbuf *err); /* @@ -797,20 +814,6 @@ int ref_transaction_commit(struct ref_transaction *transaction, int ref_transaction_abort(struct ref_transaction *transaction, struct strbuf *err); -/* - * Like ref_transaction_commit(), but optimized for creating - * references when originally initializing a repository (e.g., by "git - * clone"). It writes the new references directly to packed-refs - * without locking the individual references. - * - * It is a bug to call this function when there might be other - * processes accessing the repository or if there are existing - * references that might conflict with the ones being created. All - * old_oid values must either be absent or null_oid. - */ -int initial_ref_transaction_commit(struct ref_transaction *transaction, - struct strbuf *err); - /* * Execute the given callback function for each of the reference updates which * have been queued in the given transaction. `old_oid` and `new_oid` may be diff --git a/refs/debug.c b/refs/debug.c index 45e2e784a0..cbac62c8a4 100644 --- a/refs/debug.c +++ b/refs/debug.c @@ -118,18 +118,6 @@ static int debug_transaction_abort(struct ref_store *refs, return res; } -static int debug_initial_transaction_commit(struct ref_store *refs, - struct ref_transaction *transaction, - struct strbuf *err) -{ - struct debug_ref_store *drefs = (struct debug_ref_store *)refs; - int res; - transaction->ref_store = drefs->refs; - res = drefs->refs->be->initial_transaction_commit(drefs->refs, - transaction, err); - return res; -} - static int debug_pack_refs(struct ref_store *ref_store, struct pack_refs_opts *opts) { struct debug_ref_store *drefs = (struct debug_ref_store *)ref_store; @@ -443,7 +431,6 @@ struct ref_storage_be refs_be_debug = { .transaction_prepare = debug_transaction_prepare, .transaction_finish = debug_transaction_finish, .transaction_abort = debug_transaction_abort, - .initial_transaction_commit = debug_initial_transaction_commit, .pack_refs = debug_pack_refs, .rename_ref = debug_rename_ref, diff --git a/refs/files-backend.c b/refs/files-backend.c index 0824c0b8a9..d27806c02c 100644 --- a/refs/files-backend.c +++ b/refs/files-backend.c @@ -706,7 +706,7 @@ retry: * reason to expect this error to be transitory. */ if (refs_verify_refname_available(&refs->base, refname, - extras, NULL, err)) { + extras, NULL, 0, err)) { if (mustexist) { /* * To the user the relevant error is @@ -813,7 +813,7 @@ retry: REMOVE_DIR_EMPTY_ONLY)) { if (refs_verify_refname_available( &refs->base, refname, - extras, NULL, err)) { + extras, NULL, 0, err)) { /* * The error message set by * verify_refname_available() is OK. @@ -850,7 +850,7 @@ retry: */ if (refs_verify_refname_available( refs->packed_ref_store, refname, - extras, NULL, err)) { + extras, NULL, 0, err)) { ret = TRANSACTION_NAME_CONFLICT; goto error_return; } @@ -1159,7 +1159,7 @@ static struct ref_lock *lock_ref_oid_basic(struct files_ref_store *refs, */ if (is_null_oid(&lock->old_oid) && refs_verify_refname_available(refs->packed_ref_store, refname, - NULL, NULL, err)) + NULL, NULL, 0, err)) goto error_return; lock->ref_name = xstrdup(refname); @@ -1252,7 +1252,7 @@ static void prune_ref(struct files_ref_store *refs, struct ref_to_prune *r) if (check_refname_format(r->name, 0)) return; - transaction = ref_store_transaction_begin(&refs->base, &err); + transaction = ref_store_transaction_begin(&refs->base, 0, &err); if (!transaction) goto cleanup; ref_transaction_add_update( @@ -1396,7 +1396,8 @@ static int files_pack_refs(struct ref_store *ref_store, if (!should_pack_refs(refs, opts)) return 0; - transaction = ref_store_transaction_begin(refs->packed_ref_store, &err); + transaction = ref_store_transaction_begin(refs->packed_ref_store, + 0, &err); if (!transaction) return -1; @@ -1537,7 +1538,7 @@ static int refs_rename_ref_available(struct ref_store *refs, string_list_insert(&skip, old_refname); ok = !refs_verify_refname_available(refs, new_refname, - NULL, &skip, &err); + NULL, &skip, 0, &err); if (!ok) error("%s", err.buf); @@ -2780,6 +2781,8 @@ static int files_transaction_prepare(struct ref_store *ref_store, assert(err); + if (transaction->flags & REF_TRANSACTION_FLAG_INITIAL) + goto cleanup; if (!transaction->nr) goto cleanup; @@ -2867,7 +2870,8 @@ static int files_transaction_prepare(struct ref_store *ref_store, */ if (!packed_transaction) { packed_transaction = ref_store_transaction_begin( - refs->packed_ref_store, err); + refs->packed_ref_store, + transaction->flags, err); if (!packed_transaction) { ret = TRANSACTION_GENERIC_ERROR; goto cleanup; @@ -2973,6 +2977,127 @@ static int parse_and_write_reflog(struct files_ref_store *refs, return 0; } +static int ref_present(const char *refname, const char *referent UNUSED, + const struct object_id *oid UNUSED, + int flags UNUSED, + void *cb_data) +{ + struct string_list *affected_refnames = cb_data; + + return string_list_has_string(affected_refnames, refname); +} + +static int files_transaction_finish_initial(struct files_ref_store *refs, + struct ref_transaction *transaction, + struct strbuf *err) +{ + size_t i; + int ret = 0; + struct string_list affected_refnames = STRING_LIST_INIT_NODUP; + struct ref_transaction *packed_transaction = NULL; + struct ref_transaction *loose_transaction = NULL; + + assert(err); + + if (transaction->state != REF_TRANSACTION_PREPARED) + BUG("commit called for transaction that is not prepared"); + + /* Fail if a refname appears more than once in the transaction: */ + for (i = 0; i < transaction->nr; i++) + string_list_append(&affected_refnames, + transaction->updates[i]->refname); + string_list_sort(&affected_refnames); + if (ref_update_reject_duplicates(&affected_refnames, err)) { + ret = TRANSACTION_GENERIC_ERROR; + goto cleanup; + } + + /* + * It's really undefined to call this function in an active + * repository or when there are existing references: we are + * only locking and changing packed-refs, so (1) any + * simultaneous processes might try to change a reference at + * the same time we do, and (2) any existing loose versions of + * the references that we are setting would have precedence + * over our values. But some remote helpers create the remote + * "HEAD" and "master" branches before calling this function, + * so here we really only check that none of the references + * that we are creating already exists. + */ + if (refs_for_each_rawref(&refs->base, ref_present, + &affected_refnames)) + BUG("initial ref transaction called with existing refs"); + + packed_transaction = ref_store_transaction_begin(refs->packed_ref_store, + transaction->flags, err); + if (!packed_transaction) { + ret = TRANSACTION_GENERIC_ERROR; + goto cleanup; + } + + for (i = 0; i < transaction->nr; i++) { + struct ref_update *update = transaction->updates[i]; + + if ((update->flags & REF_HAVE_OLD) && + !is_null_oid(&update->old_oid)) + BUG("initial ref transaction with old_sha1 set"); + + if (refs_verify_refname_available(&refs->base, update->refname, + &affected_refnames, NULL, 1, err)) { + ret = TRANSACTION_NAME_CONFLICT; + goto cleanup; + } + + /* + * packed-refs don't support symbolic refs and root refs, so we + * have to queue these references via the loose transaction. + */ + if (update->new_target || is_root_ref(update->refname)) { + if (!loose_transaction) { + loose_transaction = ref_store_transaction_begin(&refs->base, 0, err); + if (!loose_transaction) { + ret = TRANSACTION_GENERIC_ERROR; + goto cleanup; + } + } + + ref_transaction_add_update(loose_transaction, update->refname, + update->flags & ~REF_HAVE_OLD, + update->new_target ? NULL : &update->new_oid, NULL, + update->new_target, NULL, NULL); + } else { + ref_transaction_add_update(packed_transaction, update->refname, + update->flags & ~REF_HAVE_OLD, + &update->new_oid, &update->old_oid, + NULL, NULL, NULL); + } + } + + if (packed_refs_lock(refs->packed_ref_store, 0, err) || + ref_transaction_commit(packed_transaction, err)) { + ret = TRANSACTION_GENERIC_ERROR; + goto cleanup; + } + packed_refs_unlock(refs->packed_ref_store); + + if (loose_transaction) { + if (ref_transaction_prepare(loose_transaction, err) || + ref_transaction_commit(loose_transaction, err)) { + ret = TRANSACTION_GENERIC_ERROR; + goto cleanup; + } + } + +cleanup: + if (loose_transaction) + ref_transaction_free(loose_transaction); + if (packed_transaction) + ref_transaction_free(packed_transaction); + transaction->state = REF_TRANSACTION_CLOSED; + string_list_clear(&affected_refnames, 0); + return ret; +} + static int files_transaction_finish(struct ref_store *ref_store, struct ref_transaction *transaction, struct strbuf *err) @@ -2988,6 +3113,8 @@ static int files_transaction_finish(struct ref_store *ref_store, assert(err); + if (transaction->flags & REF_TRANSACTION_FLAG_INITIAL) + return files_transaction_finish_initial(refs, transaction, err); if (!transaction->nr) { transaction->state = REF_TRANSACTION_CLOSED; return 0; @@ -3121,106 +3248,6 @@ static int files_transaction_abort(struct ref_store *ref_store, return 0; } -static int ref_present(const char *refname, const char *referent UNUSED, - const struct object_id *oid UNUSED, - int flags UNUSED, - void *cb_data) -{ - struct string_list *affected_refnames = cb_data; - - return string_list_has_string(affected_refnames, refname); -} - -static int files_initial_transaction_commit(struct ref_store *ref_store, - struct ref_transaction *transaction, - struct strbuf *err) -{ - struct files_ref_store *refs = - files_downcast(ref_store, REF_STORE_WRITE, - "initial_ref_transaction_commit"); - size_t i; - int ret = 0; - struct string_list affected_refnames = STRING_LIST_INIT_NODUP; - struct ref_transaction *packed_transaction = NULL; - - assert(err); - - if (transaction->state != REF_TRANSACTION_OPEN) - BUG("commit called for transaction that is not open"); - - /* Fail if a refname appears more than once in the transaction: */ - for (i = 0; i < transaction->nr; i++) - string_list_append(&affected_refnames, - transaction->updates[i]->refname); - string_list_sort(&affected_refnames); - if (ref_update_reject_duplicates(&affected_refnames, err)) { - ret = TRANSACTION_GENERIC_ERROR; - goto cleanup; - } - - /* - * It's really undefined to call this function in an active - * repository or when there are existing references: we are - * only locking and changing packed-refs, so (1) any - * simultaneous processes might try to change a reference at - * the same time we do, and (2) any existing loose versions of - * the references that we are setting would have precedence - * over our values. But some remote helpers create the remote - * "HEAD" and "master" branches before calling this function, - * so here we really only check that none of the references - * that we are creating already exists. - */ - if (refs_for_each_rawref(&refs->base, ref_present, - &affected_refnames)) - BUG("initial ref transaction called with existing refs"); - - packed_transaction = ref_store_transaction_begin(refs->packed_ref_store, err); - if (!packed_transaction) { - ret = TRANSACTION_GENERIC_ERROR; - goto cleanup; - } - - for (i = 0; i < transaction->nr; i++) { - struct ref_update *update = transaction->updates[i]; - - if ((update->flags & REF_HAVE_OLD) && - !is_null_oid(&update->old_oid)) - BUG("initial ref transaction with old_sha1 set"); - if (refs_verify_refname_available(&refs->base, update->refname, - &affected_refnames, NULL, - err)) { - ret = TRANSACTION_NAME_CONFLICT; - goto cleanup; - } - - /* - * Add a reference creation for this reference to the - * packed-refs transaction: - */ - ref_transaction_add_update(packed_transaction, update->refname, - update->flags & ~REF_HAVE_OLD, - &update->new_oid, &update->old_oid, - NULL, NULL, NULL); - } - - if (packed_refs_lock(refs->packed_ref_store, 0, err)) { - ret = TRANSACTION_GENERIC_ERROR; - goto cleanup; - } - - if (initial_ref_transaction_commit(packed_transaction, err)) { - ret = TRANSACTION_GENERIC_ERROR; - } - - packed_refs_unlock(refs->packed_ref_store); -cleanup: - if (packed_transaction) - ref_transaction_free(packed_transaction); - transaction->state = REF_TRANSACTION_CLOSED; - string_list_clear(&affected_refnames, 0); - return ret; -} - struct expire_reflog_cb { reflog_expiry_should_prune_fn *should_prune_fn; void *policy_cb; @@ -3614,7 +3641,6 @@ struct ref_storage_be refs_be_files = { .transaction_prepare = files_transaction_prepare, .transaction_finish = files_transaction_finish, .transaction_abort = files_transaction_abort, - .initial_transaction_commit = files_initial_transaction_commit, .pack_refs = files_pack_refs, .rename_ref = files_rename_ref, diff --git a/refs/packed-backend.c b/refs/packed-backend.c index 07c57fd541..794471de60 100644 --- a/refs/packed-backend.c +++ b/refs/packed-backend.c @@ -1730,13 +1730,6 @@ cleanup: return ret; } -static int packed_initial_transaction_commit(struct ref_store *ref_store UNUSED, - struct ref_transaction *transaction, - struct strbuf *err) -{ - return ref_transaction_commit(transaction, err); -} - static int packed_pack_refs(struct ref_store *ref_store UNUSED, struct pack_refs_opts *pack_opts UNUSED) { @@ -1769,7 +1762,6 @@ struct ref_storage_be refs_be_packed = { .transaction_prepare = packed_transaction_prepare, .transaction_finish = packed_transaction_finish, .transaction_abort = packed_transaction_abort, - .initial_transaction_commit = packed_initial_transaction_commit, .pack_refs = packed_pack_refs, .rename_ref = NULL, diff --git a/refs/refs-internal.h b/refs/refs-internal.h index 2313c830d8..33335d54c9 100644 --- a/refs/refs-internal.h +++ b/refs/refs-internal.h @@ -193,6 +193,7 @@ struct ref_transaction { size_t nr; enum ref_transaction_state state; void *backend_data; + unsigned int flags; }; /* @@ -665,7 +666,6 @@ struct ref_storage_be { ref_transaction_prepare_fn *transaction_prepare; ref_transaction_finish_fn *transaction_finish; ref_transaction_abort_fn *transaction_abort; - ref_transaction_commit_fn *initial_transaction_commit; pack_refs_fn *pack_refs; rename_ref_fn *rename_ref; diff --git a/refs/reftable-backend.c b/refs/reftable-backend.c index 38eb14d591..bbc779ab41 100644 --- a/refs/reftable-backend.c +++ b/refs/reftable-backend.c @@ -1097,7 +1097,9 @@ static int reftable_be_transaction_prepare(struct ref_store *ref_store, * at a later point. */ ret = refs_verify_refname_available(ref_store, u->refname, - &affected_refnames, NULL, err); + &affected_refnames, NULL, + transaction->flags & REF_TRANSACTION_FLAG_INITIAL, + err); if (ret < 0) goto done; @@ -1490,13 +1492,6 @@ done: return ret; } -static int reftable_be_initial_transaction_commit(struct ref_store *ref_store UNUSED, - struct ref_transaction *transaction, - struct strbuf *err) -{ - return ref_transaction_commit(transaction, err); -} - static int reftable_be_pack_refs(struct ref_store *ref_store, struct pack_refs_opts *opts) { @@ -1591,7 +1586,7 @@ static int write_copy_table(struct reftable_writer *writer, void *cb_data) if (arg->delete_old) string_list_insert(&skip, arg->oldname); ret = refs_verify_refname_available(&arg->refs->base, arg->newname, - NULL, &skip, &errbuf); + NULL, &skip, 0, &errbuf); if (ret < 0) { error("%s", errbuf.buf); goto done; @@ -2490,7 +2485,6 @@ struct ref_storage_be refs_be_reftable = { .transaction_prepare = reftable_be_transaction_prepare, .transaction_finish = reftable_be_transaction_finish, .transaction_abort = reftable_be_transaction_abort, - .initial_transaction_commit = reftable_be_initial_transaction_commit, .pack_refs = reftable_be_pack_refs, .rename_ref = reftable_be_rename_ref, diff --git a/reftable/block.c b/reftable/block.c index f5b432566a..0198078485 100644 --- a/reftable/block.c +++ b/reftable/block.c @@ -70,14 +70,14 @@ static int block_writer_register_restart(struct block_writer *w, int n, return 0; } -int block_writer_init(struct block_writer *bw, uint8_t typ, uint8_t *buf, +int block_writer_init(struct block_writer *bw, uint8_t typ, uint8_t *block, uint32_t block_size, uint32_t header_off, int hash_size) { - bw->buf = buf; + bw->block = block; bw->hash_size = hash_size; bw->block_size = block_size; bw->header_off = header_off; - bw->buf[header_off] = typ; + bw->block[header_off] = typ; bw->next = header_off + 4; bw->restart_interval = 16; bw->entries = 0; @@ -95,7 +95,7 @@ int block_writer_init(struct block_writer *bw, uint8_t typ, uint8_t *buf, uint8_t block_writer_type(struct block_writer *bw) { - return bw->buf[bw->header_off]; + return bw->block[bw->header_off]; } /* Adds the reftable_record to the block. Returns -1 if it does not fit, 0 on @@ -107,27 +107,24 @@ int block_writer_add(struct block_writer *w, struct reftable_record *rec) struct reftable_buf last = w->entries % w->restart_interval == 0 ? empty : w->last_key; struct string_view out = { - .buf = w->buf + w->next, + .buf = w->block + w->next, .len = w->block_size - w->next, }; - struct string_view start = out; - int is_restart = 0; - struct reftable_buf key = REFTABLE_BUF_INIT; int n = 0; int err; - err = reftable_record_key(rec, &key); + err = reftable_record_key(rec, &w->scratch); if (err < 0) goto done; - if (!key.len) { + if (!w->scratch.len) { err = REFTABLE_API_ERROR; goto done; } - n = reftable_encode_key(&is_restart, out, last, key, + n = reftable_encode_key(&is_restart, out, last, w->scratch, reftable_record_val_type(rec)); if (n < 0) { err = -1; @@ -143,9 +140,8 @@ int block_writer_add(struct block_writer *w, struct reftable_record *rec) string_view_consume(&out, n); err = block_writer_register_restart(w, start.len - out.len, is_restart, - &key); + &w->scratch); done: - reftable_buf_release(&key); return err; } @@ -153,13 +149,13 @@ int block_writer_finish(struct block_writer *w) { int i; for (i = 0; i < w->restart_len; i++) { - put_be24(w->buf + w->next, w->restarts[i]); + put_be24(w->block + w->next, w->restarts[i]); w->next += 3; } - put_be16(w->buf + w->next, w->restart_len); + put_be16(w->block + w->next, w->restart_len); w->next += 2; - put_be24(w->buf + 1 + w->header_off, w->next); + put_be24(w->block + 1 + w->header_off, w->next); /* * Log records are stored zlib-compressed. Note that the compression @@ -188,7 +184,7 @@ int block_writer_finish(struct block_writer *w) w->zstream->next_out = w->compressed; w->zstream->avail_out = compressed_len; - w->zstream->next_in = w->buf + block_header_skip; + w->zstream->next_in = w->block + block_header_skip; w->zstream->avail_in = src_len; /* @@ -206,7 +202,7 @@ int block_writer_finish(struct block_writer *w) * adjust the `next` pointer to point right after the * compressed data. */ - memcpy(w->buf + block_header_skip, w->compressed, + memcpy(w->block + block_header_skip, w->compressed, w->zstream->total_out); w->next = w->zstream->total_out + block_header_skip; } @@ -569,6 +565,7 @@ void block_writer_release(struct block_writer *bw) REFTABLE_FREE_AND_NULL(bw->zstream); REFTABLE_FREE_AND_NULL(bw->restarts); REFTABLE_FREE_AND_NULL(bw->compressed); + reftable_buf_release(&bw->scratch); reftable_buf_release(&bw->last_key); /* the block is not owned. */ } diff --git a/reftable/block.h b/reftable/block.h index 9a3effa513..0431e8591f 100644 --- a/reftable/block.h +++ b/reftable/block.h @@ -22,7 +22,7 @@ struct block_writer { unsigned char *compressed; size_t compressed_cap; - uint8_t *buf; + uint8_t *block; uint32_t block_size; /* Offset of the global header. Nonzero in the first block only. */ @@ -39,13 +39,15 @@ struct block_writer { uint32_t restart_cap; struct reftable_buf last_key; + /* Scratch buffer used to avoid allocations. */ + struct reftable_buf scratch; int entries; }; /* - * initializes the blockwriter to write `typ` entries, using `buf` as temporary - * storage. `buf` is not owned by the block_writer. */ -int block_writer_init(struct block_writer *bw, uint8_t typ, uint8_t *buf, + * initializes the blockwriter to write `typ` entries, using `block` as temporary + * storage. `block` is not owned by the block_writer. */ +int block_writer_init(struct block_writer *bw, uint8_t typ, uint8_t *block, uint32_t block_size, uint32_t header_off, int hash_size); /* returns the block type (eg. 'r' for ref records. */ diff --git a/reftable/writer.c b/reftable/writer.c index fd136794d5..be0fae6cb2 100644 --- a/reftable/writer.c +++ b/reftable/writer.c @@ -148,6 +148,7 @@ int reftable_writer_new(struct reftable_writer **out, reftable_buf_init(&wp->block_writer_data.last_key); reftable_buf_init(&wp->last_key); + reftable_buf_init(&wp->scratch); REFTABLE_CALLOC_ARRAY(wp->block, opts.block_size); if (!wp->block) { reftable_free(wp); @@ -180,6 +181,7 @@ static void writer_release(struct reftable_writer *w) w->block_writer = NULL; writer_clear_index(w); reftable_buf_release(&w->last_key); + reftable_buf_release(&w->scratch); } } @@ -249,20 +251,19 @@ static int writer_index_hash(struct reftable_writer *w, struct reftable_buf *has static int writer_add_record(struct reftable_writer *w, struct reftable_record *rec) { - struct reftable_buf key = REFTABLE_BUF_INIT; int err; - err = reftable_record_key(rec, &key); + err = reftable_record_key(rec, &w->scratch); if (err < 0) goto done; - if (reftable_buf_cmp(&w->last_key, &key) >= 0) { + if (reftable_buf_cmp(&w->last_key, &w->scratch) >= 0) { err = REFTABLE_API_ERROR; goto done; } reftable_buf_reset(&w->last_key); - err = reftable_buf_add(&w->last_key, key.buf, key.len); + err = reftable_buf_add(&w->last_key, w->scratch.buf, w->scratch.len); if (err < 0) goto done; @@ -312,7 +313,6 @@ static int writer_add_record(struct reftable_writer *w, } done: - reftable_buf_release(&key); return err; } @@ -325,7 +325,6 @@ int reftable_writer_add_ref(struct reftable_writer *w, .ref = *ref }, }; - struct reftable_buf buf = REFTABLE_BUF_INIT; int err; if (!ref->refname || @@ -340,24 +339,25 @@ int reftable_writer_add_ref(struct reftable_writer *w, goto out; if (!w->opts.skip_index_objects && reftable_ref_record_val1(ref)) { - err = reftable_buf_add(&buf, (char *)reftable_ref_record_val1(ref), + reftable_buf_reset(&w->scratch); + err = reftable_buf_add(&w->scratch, (char *)reftable_ref_record_val1(ref), hash_size(w->opts.hash_id)); if (err < 0) goto out; - err = writer_index_hash(w, &buf); + err = writer_index_hash(w, &w->scratch); if (err < 0) goto out; } if (!w->opts.skip_index_objects && reftable_ref_record_val2(ref)) { - reftable_buf_reset(&buf); - err = reftable_buf_add(&buf, reftable_ref_record_val2(ref), + reftable_buf_reset(&w->scratch); + err = reftable_buf_add(&w->scratch, reftable_ref_record_val2(ref), hash_size(w->opts.hash_id)); if (err < 0) goto out; - err = writer_index_hash(w, &buf); + err = writer_index_hash(w, &w->scratch); if (err < 0) goto out; } @@ -365,7 +365,6 @@ int reftable_writer_add_ref(struct reftable_writer *w, err = 0; out: - reftable_buf_release(&buf); return err; } diff --git a/reftable/writer.h b/reftable/writer.h index e8a6fbb785..1f4788a430 100644 --- a/reftable/writer.h +++ b/reftable/writer.h @@ -20,6 +20,8 @@ struct reftable_writer { void *write_arg; int pending_padding; struct reftable_buf last_key; + /* Scratch buffer used to avoid allocations. */ + struct reftable_buf scratch; /* offset of next block to write. */ uint64_t next; diff --git a/sequencer.c b/sequencer.c index 353d804999..287f4e5e87 100644 --- a/sequencer.c +++ b/sequencer.c @@ -662,7 +662,7 @@ static int fast_forward_to(struct repository *r, strbuf_addf(&sb, "%s: fast-forward", action_name(opts)); transaction = ref_store_transaction_begin(get_main_ref_store(the_repository), - &err); + 0, &err); if (!transaction || ref_transaction_update(transaction, "HEAD", to, unborn && !is_rebase_i(opts) ? @@ -1297,7 +1297,7 @@ int update_head_with_reflog(const struct commit *old_head, } transaction = ref_store_transaction_begin(get_main_ref_store(the_repository), - err); + 0, err); if (!transaction || ref_transaction_update(transaction, "HEAD", new_head, old_head ? &old_head->object.oid : null_oid(), @@ -3890,7 +3890,7 @@ static int do_label(struct repository *r, const char *name, int len) strbuf_addf(&ref_name, "refs/rewritten/%.*s", len, name); strbuf_addf(&msg, "rebase (label) '%.*s'", len, name); - transaction = ref_store_transaction_begin(refs, &err); + transaction = ref_store_transaction_begin(refs, 0, &err); if (!transaction) { error("%s", err.buf); ret = -1; diff --git a/t/helper/test-ref-store.c b/t/helper/test-ref-store.c index 65346dee55..240f6fc29d 100644 --- a/t/helper/test-ref-store.c +++ b/t/helper/test-ref-store.c @@ -199,7 +199,7 @@ static int cmd_verify_ref(struct ref_store *refs, const char **argv) struct strbuf err = STRBUF_INIT; int ret; - ret = refs_verify_refname_available(refs, refname, NULL, NULL, &err); + ret = refs_verify_refname_available(refs, refname, NULL, NULL, 0, &err); if (err.len) puts(err.buf); return ret; diff --git a/t/t1460-refs-migrate.sh b/t/t1460-refs-migrate.sh index cf57cd66f2..1bfff3a7af 100755 --- a/t/t1460-refs-migrate.sh +++ b/t/t1460-refs-migrate.sh @@ -236,7 +236,7 @@ test_expect_success 'migrating from reftable format deletes backend files' ' test_path_is_missing repo/.git/reftable && echo "ref: refs/heads/main" >expect && test_cmp expect repo/.git/HEAD && - test_path_is_file repo/.git/refs/heads/main + test_path_is_file repo/.git/packed-refs ' test_done diff --git a/walker.c b/walker.c index 5ea7e5b392..7cc9dbea46 100644 --- a/walker.c +++ b/walker.c @@ -290,7 +290,7 @@ int walker_fetch(struct walker *walker, int targets, char **target, if (write_ref) { transaction = ref_store_transaction_begin(get_main_ref_store(the_repository), - &err); + 0, &err); if (!transaction) { error("%s", err.buf); goto done;