Re: [PATCH v2 05/17] refs.c: add transaction support for replacing a reflog
- From
Junio C Hamano <gitster@pobox.com>
- Date
- Nov 3, 2014, 21:06 UTC
- Message-ID
- <xmqqk33cq9i5.fsf@gitster.dls.corp.google.com>
- In-Reply-To
- <1415041339-18450-6-git-send-email-sahlberg@google.com>
Ronnie Sahlberg <sahlberg@google.com> writes:
> Add a new transaction function transaction_replace_reflog. > This function takes a blob and replaces the new or existing reflog with the > content of this blob. This will be used by rename_ref where we basically want > to copy the existing blob as is.
Hmph, I find that "let's give backends an interface to tell them that we want to use the whole reflog for ref A for ref B by renaming" a sensible higher level abstraction, but forcing all the backends to have the reflog data as a series of reflog entries as series of text lines does not look like a sensible abstraction at all to me. Imagine a backend that uses a reflog_<refname> table (in a database) to store reflog entries for that named ref---should it give you a textual dump of the table and accept from you another textual dump? Wouldn't you rather tell it to rename A to B and have it decide the best way to do so (namely, rename table reflog_A to reflog_B in the case of such a hypothetical implementation)?
Show 125 quoted lines
>
> Signed-off-by: Ronnie Sahlberg <sahlberg@google.com>
> ---
> refs.c | 38 +++++++++++++++++++++++++++++++++++++-
> refs.h | 16 ++++++++++++++++
> 2 files changed, 53 insertions(+), 1 deletion(-)
>
> diff --git a/refs.c b/refs.c
> index e9e321e..3d13624 100644
> --- a/refs.c
> +++ b/refs.c
> @@ -35,6 +35,11 @@ static unsigned char refname_disposition[256] = {
> * just use the lock taken by the first update.
> */
> #define UPDATE_REFLOG_NOLOCK 0x0200
> +/*
> + * This update is used to replace a new or existing reflog with new content
> + * held in update->new_reflog.
> + */
> +#define REFLOG_REPLACE 0x0400
>
> /*
> * Try to read one refname component from the front of refname.
> @@ -3561,6 +3566,7 @@ struct ref_update {
> struct lock_file *reflog_lock;
> char *committer;
> struct ref_update *orig_update; /* For UPDATE_REFLOG_NOLOCK */
> + struct strbuf new_reflog;
>
> const char refname[FLEX_ARRAY];
> };
> @@ -3607,6 +3613,7 @@ void transaction_free(struct transaction *transaction)
> return;
>
> for (i = 0; i < transaction->nr; i++) {
> + strbuf_release(&transaction->updates[i]->new_reflog);
> free(transaction->updates[i]->msg);
> free(transaction->updates[i]->committer);
> free(transaction->updates[i]);
> @@ -3622,6 +3629,7 @@ static struct ref_update *add_update(struct transaction *transaction,
> size_t len = strlen(refname);
> struct ref_update *update = xcalloc(1, sizeof(*update) + len + 1);
>
> + strbuf_init(&update->new_reflog, 0);
> strcpy((char *)update->refname, refname);
> update->update_type = update_type;
> ALLOC_GROW(transaction->updates, transaction->nr + 1, transaction->alloc);
> @@ -3681,6 +3689,24 @@ int transaction_update_reflog(struct transaction *transaction,
> return 0;
> }
>
> +int transaction_replace_reflog(struct transaction *transaction,
> + const char *refname,
> + struct strbuf *buf,
> + struct strbuf *err)
> +{
> + struct ref_update *update;
> +
> + if (transaction->state != TRANSACTION_OPEN)
> + die("BUG: replace_reflog called for transaction that is "
> + "not open");
> +
> + update = add_update(transaction, refname, UPDATE_LOG);
> + update->flags = REFLOG_REPLACE;
> + strbuf_swap(&update->new_reflog, buf);
> + update->reflog_lock = xcalloc(1, sizeof(struct lock_file));
> + return 0;
> +}
> +
> int transaction_update_ref(struct transaction *transaction,
> const char *refname,
> const unsigned char *new_sha1,
> @@ -4012,7 +4038,7 @@ int transaction_commit(struct transaction *transaction,
> continue;
> if (update->reflog_fd == -1)
> continue;
> - if (update->flags & REFLOG_TRUNCATE)
> + if (update->flags & (REFLOG_TRUNCATE|REFLOG_REPLACE))
> if (lseek(update->reflog_fd, 0, SEEK_SET) < 0 ||
> ftruncate(update->reflog_fd, 0)) {
> error("Could not truncate reflog: %s. %s",
> @@ -4030,6 +4056,16 @@ int transaction_commit(struct transaction *transaction,
> rollback_lock_file(update->reflog_lock);
> update->reflog_fd = -1;
> }
> + if (update->flags & REFLOG_REPLACE)
> + if (write_in_full(update->reflog_fd,
> + update->new_reflog.buf,
> + update->new_reflog.len) !=
> + update->new_reflog.len) {
> + error("Could write to reflog: %s. %s",
> + update->refname, strerror(errno));
> + rollback_lock_file(update->reflog_lock);
> + update->reflog_fd = -1;
> + }
> }
>
> /* Commit all reflog files */
> diff --git a/refs.h b/refs.h
> index d174380..ec4965f 100644
> --- a/refs.h
> +++ b/refs.h
> @@ -354,6 +354,22 @@ int transaction_update_reflog(struct transaction *transaction,
> struct strbuf *err);
>
> /*
> + * Replace the reflog with the content of buf.
> + * This function can be used when replacing the whole content of the reflog
> + * during for example a rename operation. For these cases we do not want
> + * to have to spend time processing and marshaling each entry individually
> + * if instead we can just pass one single blob to the transaction system and
> + * say "write this blob, it is the new reflog".
> + *
> + * When successful, this function will take over ownership of buf.
> + * Thus meaning that buf will become cleared from the callers view.
> + */
> +int transaction_replace_reflog(struct transaction *transaction,
> + const char *refname,
> + struct strbuf *buf,
> + struct strbuf *err);
> +
> +/*
> * Commit all of the changes that have been queued in transaction, as
> * atomically as possible.
> *