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

> Justin Tobler <jltobler@gmail.com> 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?