Re: [PATCH 07/31] refs.c: add a flag to allow reflog updates to truncate the log
On Fri, May 16, 2014 at 2:20 PM, Junio C Hamano wrote:
> Ronnie Sahlberg writes:
>
>> Add a flag that allows us to truncate the reflog before we write the update.
>>
>> Signed-off-by: Ronnie Sahlberg
>> ---
>
> Until we read the callers it is hard to see how such a feature is
> useful, though.
Shortly later in the series comes the update to builtin/reflog.c which
uses this feature.
Even later in the series I also use this in builtin/reflog.c to both
do the ref update as well as the corresponding reflog updates all in
one single transaction.
>
> (style) The two multi-line comments are formatted differently.
Fixed. Thanks.
>
>> 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?
I have changed it to do this :
+ if (update->flags & REFLOG_TRUNCATE)
+ if (lseek(update->reflog_fd, 0, SEEK_SET) < 0 ||
+ ftruncate(update->reflog_fd, 0)) {
+ error("Could not truncate reflog: %s",
+ update->refname);
+ rollback_lock_file(&update->reflog_lock);
+ update->reflog_fd = -1;
+ }
At this point we have already committed all the changes to refs and
also performed all unlinks for refs we are deleting. So if things
start to go wrong here during the reflog updates it is too late to do
much more than complain.
>
>> 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,
--
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 07/31] refs.c: add a flag to allow reflog updates to truncate the log
Ronnie Sahlberg writes:
> Add a flag that allows us to truncate the reflog before we write the update.
>
> Signed-off-by: Ronnie Sahlberg
> ---
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,
--
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 07/31] refs.c: add a flag to allow reflog updates to truncate the log
Add a flag that allows us to truncate the reflog before we write the update.
Signed-off-by: Ronnie Sahlberg
---
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");
+ }
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,
--
2.0.0.rc3.506.g3739a35
--
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
