odb/transaction: propagate begin errors

When `odb_transaction_begin()` is invoked, the function returns the
transaction pointer directly. There is no way for the backend to
signal that it failed to set up its state, such as when creating the
temporary object directory backing the transaction.

In a subsequent commit, git-receive-pack(1) starts using ODB
transactions and needs to be able to report such failures rather
than silently ignore them. Refactor `odb_transaction_begin()` to
return an int error code and write the resulting transaction into an
out parameter. Also introduce `odb_transaction_begin_or_die()` as a
convenience for callsites that do not need to handle errors
explicitly.

Note that `odb_transaction_begin()` now returns an error when the ODB
already has an inflight transaction pending. ODB transaction call sites
that may encounter an inflight transaction are updated to explicitly
handle this case.

Signed-off-by: Justin Tobler <jltobler@gmail.com>
Signed-off-by: Junio C Hamano <gitster@pobox.com>

diff --git a/builtin/add.c b/builtin/add.c
index c859f665199efa91897472e6eab7e8d7ae983fc6..3d5d9cfdb9e30cc24d93d8e3d5ccb294a6fb1e88 100644 (file)
--- a/builtin/add.c
+++ b/builtin/add.c
@@ -581,7 +581,7 @@ int cmd_add(int argc,
                string_list_clear(&only_match_skip_worktree, 0);
        }
 
-       transaction = odb_transaction_begin(repo->objects);
+       odb_transaction_begin_or_die(repo->objects, &transaction);
 
        ps_matched = xcalloc(pathspec.nr, 1);
        if (add_renormalize)

diff --git a/builtin/unpack-objects.c b/builtin/unpack-objects.c
index 59e9b8711e3c2b3e4c0d90c45d57ec0925eb60c5..1a195bf0451c1a34bd313fb935b2e88104f33efa 100644 (file)
--- a/builtin/unpack-objects.c
+++ b/builtin/unpack-objects.c
@@ -596,7 +596,7 @@ static void unpack_all(void)
                progress = start_progress(the_repository,
                                          _("Unpacking objects"), nr_objects);
        CALLOC_ARRAY(obj_list, nr_objects);
-       transaction = odb_transaction_begin(the_repository->objects);
+       odb_transaction_begin_or_die(the_repository->objects, &transaction);
        for (i = 0; i < nr_objects; i++) {
                unpack_one(i);
                display_progress(progress, i + 1);

diff --git a/builtin/update-index.c b/builtin/update-index.c
index 3d6646c318b98e806a0aba09c3f034b133dc582c..17f3ea284cbb2cd725f93f53081c1f37cbf3a96e 100644 (file)
--- a/builtin/update-index.c
+++ b/builtin/update-index.c
@@ -1124,7 +1124,7 @@ int cmd_update_index(int argc,
         * Allow the object layer to optimize adding multiple objects in
         * a batch.
         */
-       transaction = odb_transaction_begin(the_repository->objects);
+       odb_transaction_begin_or_die(the_repository->objects, &transaction);
        while (ctx.argc) {
                if (parseopt_state != PARSE_OPT_DONE)
                        parseopt_state = parse_options_step(&ctx, options,

diff --git a/cache-tree.c b/cache-tree.c
index 184f7e2635b9f4bf0d2bf07878e6761da166d709..8eec1d4d5247e70533a0c4fde15f5d54d843f0c1 100644 (file)
--- a/cache-tree.c
+++ b/cache-tree.c
@@ -474,6 +474,7 @@ static int update_one(struct cache_tree *it,
 
 int cache_tree_update(struct index_state *istate, int flags)
 {
+       int inflight = !!the_repository->objects->transaction;
        struct odb_transaction *transaction;
        int skip, i;
 
@@ -490,10 +491,12 @@ int cache_tree_update(struct index_state *istate, int flags)
 
        trace_performance_enter();
        trace2_region_enter("cache_tree", "update", istate->repo);
-       transaction = odb_transaction_begin(the_repository->objects);
+       if (!inflight)
+               odb_transaction_begin_or_die(the_repository->objects, &transaction);
        i = update_one(istate->cache_tree, istate->cache, istate->cache_nr,
                       "", 0, &skip, flags);
-       odb_transaction_commit(transaction);
+       if (!inflight)
+               odb_transaction_commit(transaction);
        trace2_region_leave("cache_tree", "update", istate->repo);
        trace_performance_leave("cache_tree_update");
        if (i < 0)

diff --git a/object-file.c b/object-file.c
index 3d76b890b8d5e9f2cdbc74483f62df233fc6be3e..f1ec9f6a8bbf92540aa98c565c343ba26958a989 100644 (file)
--- a/object-file.c
+++ b/object-file.c
@@ -1352,13 +1352,17 @@ int index_fd(struct index_state *istate, struct object_id *oid,
 
                if (flags & INDEX_WRITE_OBJECT) {
                        struct object_database *odb = the_repository->objects;
-                       struct odb_transaction *transaction = odb_transaction_begin(odb);
+                       struct odb_transaction *transaction = odb->transaction;
+                       int inflight = !!transaction;
 
-                       ret = odb_transaction_write_object_stream(odb->transaction,
+                       if (!inflight)
+                               odb_transaction_begin_or_die(odb, &transaction);
+                       ret = odb_transaction_write_object_stream(transaction,
                                                                  &stream,
                                                                  xsize_t(st->st_size),
                                                                  oid);
-                       odb_transaction_commit(transaction);
+                       if (!inflight)
+                               odb_transaction_commit(transaction);
                } else {
                        ret = hash_blob_stream(&stream,
                                               the_repository->hash_algo, oid,

diff --git a/odb/transaction.c b/odb/transaction.c
index b16e07aebfc5ac3675f1296240a5f2565a67413e..b6da4a3942f8880412c741e4fec7faca4c1eec99 100644 (file)
--- a/odb/transaction.c
+++ b/odb/transaction.c
@@ -1,15 +1,21 @@
 #include "git-compat-util.h"
+#include "gettext.h"
 #include "odb/source.h"
 #include "odb/transaction.h"
 
-struct odb_transaction *odb_transaction_begin(struct object_database *odb)
+int odb_transaction_begin(struct object_database *odb,
+                         struct odb_transaction **out)
 {
+       int ret;
+
        if (odb->transaction)
-               return NULL;
+               return error(_("object database transaction already pending"));
 
-       odb_source_begin_transaction(odb->sources, &odb->transaction);
+       ret = odb_source_begin_transaction(odb->sources, out);
+       if (!ret)
+               odb->transaction = *out;
 
-       return odb->transaction;
+       return ret;
 }
 
 void odb_transaction_commit(struct odb_transaction *transaction)

diff --git a/odb/transaction.h b/odb/transaction.h
index d52f0533ce3ee8608198f2abcec710edaf2bfe76..f5c43187c94dd302445d529f3c20d90d90558de5 100644 (file)
--- a/odb/transaction.h
+++ b/odb/transaction.h
@@ -1,6 +1,7 @@
 #ifndef ODB_TRANSACTION_H
 #define ODB_TRANSACTION_H
 
+#include "gettext.h"
 #include "odb.h"
 #include "odb/source.h"
 
@@ -36,11 +37,21 @@ struct odb_transaction {
 };
 
 /*
- * Starts an ODB transaction. Subsequent objects are written to the transaction
- * and not committed until odb_transaction_commit() is invoked on the
- * transaction. If the ODB already has a pending transaction, NULL is returned.
+ * Starts an ODB transaction and returns it via `out`. Subsequent objects are
+ * written to the transaction and not committed until odb_transaction_commit()
+ * is invoked on the transaction. Returns 0 on success and a negative value on
+ * error. Note that it is considered an error to start a new transaction if the
+ * ODB already has an inflight transaction pending.
  */
-struct odb_transaction *odb_transaction_begin(struct object_database *odb);
+int odb_transaction_begin(struct object_database *odb,
+                         struct odb_transaction **out);
+
+static inline void odb_transaction_begin_or_die(struct object_database *odb,
+                                               struct odb_transaction **out)
+{
+       if (odb_transaction_begin(odb, out))
+               die(_("failed to start ODB transaction"));
+}
 
 /*
  * Commits an ODB transaction making the written objects visible. If the

diff --git a/read-cache.c b/read-cache.c
index 21829102ae275e124eb7b94550bc7ccf16993bac..bf25989956b5f4d2ed54b61fd5e37092e169194c 100644 (file)
--- a/read-cache.c
+++ b/read-cache.c
@@ -4012,6 +4012,7 @@ int add_files_to_cache(struct repository *repo, const char *prefix,
                       const struct pathspec *pathspec, char *ps_matched,
                       int include_sparse, int flags, int ignored_too )
 {
+       int inflight = !!repo->objects->transaction;
        struct odb_transaction *transaction;
        struct update_callback_data data;
        struct rev_info rev;
@@ -4042,9 +4043,11 @@ int add_files_to_cache(struct repository *repo, const char *prefix,
         * This function is invoked from commands other than 'add', which
         * may not have their own transaction active.
         */
-       transaction = odb_transaction_begin(repo->objects);
+       if (!inflight)
+               odb_transaction_begin_or_die(repo->objects, &transaction);
        run_diff_files(&rev, DIFF_RACY_IS_MODIFIED);
-       odb_transaction_commit(transaction);
+       if (!inflight)
+               odb_transaction_commit(transaction);
 
        release_revisions(&rev);
        return !!data.add_errors;