Re: [PATCH v11 31/41] refs.c: make prune_ref use a transaction to delete the ref

[Jonathan Nieder <jrnieder@gmail.com>]

Ronnie Sahlberg wrote:

> Change prune_ref to delete the ref using a ref transaction. To do this we also
> need to add a new flag REF_ISPRUNING that will tell the transaction that we
> do not want to delete this ref from the packed refs.

s/from the packed refs/from packed-refs, nor delete the ref's reflog/

[...]
> --- a/refs.h
> +++ b/refs.h
> @@ -235,6 +235,11 @@ struct ref_transaction *ref_transaction_begin(struct strbuf *err);
>   * The following functions add a reference check or update to a
>   * ref_transaction.  In all of them, refname is the name of the
>   * reference to be affected.  The functions make internal copies of
>   * refname, so the caller retains ownership of the parameter.  flags
>   * can be REF_NODEREF; it is passed to update_ref_lock().
>   */
>  
>  /*
> + * ref_transaction_update ref_transaction_create and ref_transaction_delete
> + * all take a flag argument. Currently the only public flag is REF_NODEREF.
> + * Flag values >= 0x100 are reserved for internal use.
> + */
> +/*
>   * Add a reference update to transaction.  new_sha1 is the value that

The comment right before here already tries to explain the flag argument,
though it isn't in an obvious place so it's easy to miss.  Maybe the flag
argument should be explained in the overview documentation for the
ref_transaction API near the top of the file (but I haven't thought that
through, so leaving it alone).

How about this as a way to make the reserved flag values easier to
find when adding new flags?

diff --git i/refs.h w/refs.h
index 25ac4a9..dee7c8f 100644
--- i/refs.h
+++ w/refs.h
@@ -171,8 +171,17 @@ extern int ref_exists(const char *);
  */
 extern int peel_ref(const char *refname, unsigned char *sha1);
 
-/** Locks any ref (for 'HEAD' type refs). */
+/*
+ * Flags controlling lock_any_ref_for_update(), ref_transaction_update(),
+ * ref_transaction_create(), etc.
+ * REF_NODEREF: act on the ref directly, instead of dereferencing
+ *              symbolic references.
+ *
+ * Flags >= 0x100 are reserved for internal use.
+ */
 #define REF_NODEREF	0x01
+
+/** Locks any ref (for 'HEAD' type refs). */
 extern struct ref_lock *lock_any_ref_for_update(const char *refname,
 						const unsigned char *old_sha1,
 						int flags, int *type_p);
@@ -265,11 +274,6 @@ struct ref_transaction *ref_transaction_begin(struct strbuf *err);
  */
 
 /*
- * ref_transaction_update ref_transaction_create and ref_transaction_delete
- * all take a flag argument. Currently the only public flag is REF_NODEREF.
- * Flag values >= 0x100 are reserved for internal use.
- */
-/*
  * Add a reference update to transaction.  new_sha1 is the value that
  * the reference should have after the update, or zeros if it should
  * be deleted.  If have_old is true, then old_sha1 holds the value