Re: [PATCH v11 31/41] refs.c: make prune_ref use a transaction to delete the ref
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_NODEREF0x01 + +/** 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 -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v11 31/41] refs.c: make prune_ref use a transaction to delete the ref
On Wed, May 28, 2014 at 2:51 PM, Jonathan Nieder jrnie...@gmail.com wrote: 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_NODEREF0x01 + +/** 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 Thanks. Changed. -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH v11 31/41] refs.c: make prune_ref use a transaction to delete the ref
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. This flag is private to refs.c and not exposed to external callers. Signed-off-by: Ronnie Sahlberg sahlb...@google.com --- refs.c | 28 +--- refs.h | 5 + 2 files changed, 26 insertions(+), 7 deletions(-) diff --git a/refs.c b/refs.c index 4ca84f7..1819434 100644 --- a/refs.c +++ b/refs.c @@ -30,6 +30,12 @@ static inline int bad_ref_char(int ch) } /* + * Used as a flag to ref_transaction_delete when a loose ref is being + * pruned. + */ +#define REF_ISPRUNING 0x0100 + +/* * Try to read one refname component from the front of refname. Return * the length of the component found, or -1 if the component is not * legal. @@ -2328,17 +2334,24 @@ static void try_remove_empty_parents(char *name) /* make sure nobody touched the ref, and unlink */ static void prune_ref(struct ref_to_prune *r) { - struct ref_lock *lock; + struct ref_transaction *transaction; + struct strbuf err = STRBUF_INIT; if (check_refname_format(r-name + 5, 0)) return; - lock = lock_ref_sha1_basic(r-name, r-sha1, 0, NULL); - if (lock) { - unlink_or_warn(git_path(%s, r-name)); - unlock_ref(lock); - try_remove_empty_parents(r-name); + transaction = ref_transaction_begin(err); + if (!transaction || + ref_transaction_delete(transaction, r-name, r-sha1, + REF_ISPRUNING, 1, err) || + ref_transaction_commit(transaction, NULL, err)) { + ref_transaction_free(transaction); + error(%s, err.buf); + strbuf_release(err); + return; } + ref_transaction_free(transaction); + try_remove_empty_parents(r-name); } static void prune_refs(struct ref_to_prune *r) @@ -3546,9 +3559,10 @@ int ref_transaction_commit(struct ref_transaction *transaction, struct ref_update *update = updates[i]; if (update-lock) { - delnames[delnum++] = update-lock-ref_name; ret |= delete_ref_loose(update-lock, update-type, err); + if (!(update-flags REF_ISPRUNING)) + delnames[delnum++] = update-lock-ref_name; } } diff --git a/refs.h b/refs.h index 1e25e1c..39c97f8 100644 --- a/refs.h +++ b/refs.h @@ -235,6 +235,11 @@ 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 -- 2.0.0.rc3.474.g0203784 -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html