Subject: Re: [PATCH 07/31] refs.c: add a flag to allow reflog updates to truncate the log

From: Junio C Hamano <[email-masked]>
Date: Fri, 16 May 2014 14:20:58 -0700

Ronnie Sahlberg <[email-masked]> writes:

> Add a flag that allows us to truncate the reflog before we write the update.
>
> Signed-off-by: Ronnie Sahlberg <[email-masked]>
> ---

Until we read the callers it is hard to see how such a feature is
useful, though.

(style) The two multi-line comments are formatted differently.

>  refs.c | 12 ++++++++++--
>  refs.h |  4 +++-
>  2 files changed, 13 insertions(+), 3 deletions(-)
>
> diff --git a/refs.c b/refs.c
> index d8a1568..a8b583a 100644
> --- a/refs.c
> +++ b/refs.c
> @@ -3608,7 +3608,11 @@ int transaction_commit(struct ref_transaction *transaction,
>  		}
>  	}
>  
> -	/* Update all reflog files */
> +	/* Update all reflog files
> +	   We have already done all ref updates and deletes.
> +	   There is not much we can do here if there are any reflog
> +	   update errors, other than complain.
> +	*/
>  	for (i = 0; i < n; i++) {
>  		struct ref_update *update = updates[i];
>  
> @@ -3616,7 +3620,11 @@ int transaction_commit(struct ref_transaction *transaction,
>  			continue;
>  		if (update->reflog_fd == -1)
>  			continue;
> -
> +		if (update->flags & REFLOG_TRUNCATE)
> +			if (lseek(update->reflog_fd, 0, SEEK_SET) < 0 ||
> +			    ftruncate(update->reflog_fd, 0)) {
> +				error("Could not truncate reflog");
> +			}

The {} looks somewhat curious here.  If you seeked to the beginning,
and failed to truncate, do you still want to go on without "return"
in front of the error()?  Would that overwrite existing entries?

>  		if (log_ref_write_fd(update->reflog_fd, update->old_sha1,
>  				     update->new_sha1,
>  				     update->committer, update->msg)) {
> diff --git a/refs.h b/refs.h
> index 2e22a26..b1b97e7 100644
> --- a/refs.h
> +++ b/refs.h
> @@ -269,8 +269,10 @@ int transaction_delete_sha1(struct ref_transaction *transaction,
>  			    const unsigned char *old_sha1,
>  			    int flags, int have_old, const char *msg);
>  
> +#define REFLOG_TRUNCATE 0x01
>  /*
> - * Append a reflog entry for refname.
> + * Append a reflog entry for refname. If the REFLOG_TRUNCATE flag is set
> + * this update will first truncate the reflog before writing the entry.
>   */
>  int transaction_update_reflog(struct ref_transaction *transaction,
>  			      const char *refname,