On 26/06/24 11:35AM, Junio C Hamano wrote:

Justin Tobler [off-list ref] writes:

quoted

The "files" transaction backend may encounter errors related to managing
the temporary directory used to stage objects, but silently ignores
these errors. Instead return errors encountered in the
`odb_transaction_files_{prepare,begin,commit}()` interfaces to allow
callers to handle as needed.

"handle them as needed", perhaps.

Will fix, thanks

[snip]

The caller of this function does react to a failure of it, ...

quoted

@@ -1670,27 +1678,34 @@ int read_loose_object(struct repository *repo,
 	return ret;
 }
 
-static void odb_transaction_files_commit(struct odb_transaction *base)
+static int odb_transaction_files_commit(struct odb_transaction *base)
 {
 	struct odb_transaction_files *transaction =
 		container_of(base, struct odb_transaction_files, base);
 
-	flush_loose_object_transaction(transaction);
+	if (flush_loose_object_transaction(transaction))
+		return -1;
 	flush_packfile_transaction(transaction);
+
+	return 0;
 }

... like this, which is good.  Do we need an explicit "abort-transaction",
or is that implicit?

So this is currently handled implicitly via
`tmp-objdir.c:remove_tmp_objdir()` which gets registered as an atexit()
handler. As long as the tmp_objdir global remains set, it will
automatically get cleaned up.

In a subsequent series, I do plan to add `odb_transaction_abort()` to
the transaction interface. It may make sense to also use that here to
make the cleanup a bit more explicit though.

-Justin