Re: [PATCH v2 7/7] odb/transaction: add transaction interface to write packfiles

Justin Tobler <[email protected]> writes:

> .... Note that a packfile
> written via git-index-pack(1) is kept in place by a ".keep" lockfile
> that must be retained until references are updated. To faciliate this in
> an ODB backend agnostic manner, the "files" transaction backend takes
> ownership of these lockfiles and removes them post-commit through its
> release callback.

The above is confusing and I am lost.  Care to explain a bit more?

> +		status = start_command(&child);
> +		if (status) {
> +			strbuf_addstr(err_msg, "index-pack fork failed");
> +			return -1;
> +		}
> +
> +		lockfile = index_pack_lockfile(repo, child.out, NULL);
> +		if (lockfile) {
> +			ALLOC_GROW(transaction->pack_lockfiles,
> +				   transaction->pack_lockfiles_nr + 1,
> +				   transaction->pack_lockfiles_alloc);
> +			transaction->pack_lockfiles[transaction->pack_lockfiles_nr++] =
> +				register_tempfile(lockfile);
> +			free(lockfile);
> +		}

Here we add the .keep file to the list of lockfiles.  We have
finalization step laer in odb_transaction_files_finalize() that
deletes the tempfiles when we are done, which comes after
the transaction is committed.

But isn't the odb_transaction_files_commit() where the migration of
tmp_objdir_migrate() happens?  Everything in the quarantine directory
including these .keep files are "migrated" (either link-to-the-new
followed by unlink-of-the-old, or rename-old-to-new) there.

And then ...

> +static int odb_transaction_files_finalize(struct odb_transaction *base)
> +{
> +	struct odb_transaction_files *transaction =
> +		container_of(base, struct odb_transaction_files, base);
> +	int ret = 0;
> +
> +	for (size_t i = 0; i < transaction->pack_lockfiles_nr; i++)
> +		ret |= delete_tempfile(&transaction->pack_lockfiles[i]);
> +
> +	free(transaction->pack_lockfiles);
> +
> +	return ret;
> +}

... we do the deletion of tempfile but has anybody migrated the path
to these files recorded in the lockfile structure?  How are we
removing the .keep files that were "migrated" when the transaction
was committed?

Puzzled and confused...