David Turner <[email protected]> writes:

> Since there is a manual check for that case, the code will fail at test
> time.

I see a difference between "We make it hard to write incorrect code"
and "We keep it easy to write incorrect code but a runtime check
will catch mistakes anyway", and I tend to prefer the former.

I do think the "manual check" needs to be kept in an adjusted form
even if we decide to take the suggested update.  A caller can pass
0x04 instead of REF_ISPRUNING after all---but that is exactly the
case where you have to work hard to write incorrect code.

One existing check that is not touched with 15/29 also needs to be
adjusted.  An incremental update relative to 15/29 would look like
this:

 refs.c               | 4 ++--
 refs/files-backend.c | 4 ++--
 refs/refs-internal.h | 6 ++++--
 3 files changed, 8 insertions(+), 6 deletions(-)

diff --git a/refs.c b/refs.c
index 5dc2473..bb81dfd 100644
--- a/refs.c
+++ b/refs.c
@@ -790,8 +790,8 @@ int ref_transaction_update(struct ref_transaction 
*transaction,
        if (transaction->state != REF_TRANSACTION_OPEN)
                die("BUG: update called for transaction that is not open");
 
-       if ((flags & REF_ISPRUNING) && !(flags & REF_NODEREF))
-               die("BUG: REF_ISPRUNING set without REF_NODEREF");
+       if ((flags & REF_ISPRUNING_) && !(flags & REF_NODEREF))
+               die("BUG: REF_ISPRUNING_ set without REF_NODEREF");
 
        if (new_sha1 && !is_null_sha1(new_sha1) &&
            check_refname_format(refname, REFNAME_ALLOW_ONELEVEL)) {
diff --git a/refs/files-backend.c b/refs/files-backend.c
index 8fcbd7d..012e3d0 100644
--- a/refs/files-backend.c
+++ b/refs/files-backend.c
@@ -2116,7 +2116,7 @@ static void prune_ref(struct ref_to_prune *r)
        transaction = ref_transaction_begin(&err);
        if (!transaction ||
            ref_transaction_delete(transaction, r->name, r->sha1,
-                                  REF_ISPRUNING | REF_NODEREF, NULL, &err) ||
+                                  REF_ISPRUNING, NULL, &err) ||
            ref_transaction_commit(transaction, &err)) {
                ref_transaction_free(transaction);
                error("%s", err.buf);
@@ -3178,7 +3178,7 @@ int ref_transaction_commit(struct ref_transaction 
*transaction,
                                goto cleanup;
                        }
 
-                       if (!(update->flags & REF_ISPRUNING))
+                       if (!(update->flags & REF_ISPRUNING_))
                                string_list_append(&refs_to_delete,
                                                   update->lock->ref_name);
                }
diff --git a/refs/refs-internal.h b/refs/refs-internal.h
index 37a1a37..0a2bf1f 100644
--- a/refs/refs-internal.h
+++ b/refs/refs-internal.h
@@ -15,9 +15,11 @@
 
 /*
  * Used as a flag in ref_update::flags when a loose ref is being
- * pruned.
+ * pruned.  As this must always be done with REF_NODEREF, we make
+ * the public version REF_ISPRUNING to contain REF_NODEREF.
  */
-#define REF_ISPRUNING  0x04
+#define REF_ISPRUNING_ 0x04
+#define REF_ISPRUNING  (REF_ISPRUNING_ | REF_NODEREF)
 
 /*
  * Used as a flag in ref_update::flags when the reference should be
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to [email protected]
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Reply via email to