Patrick Steinhardt <ps@xxxxxx> writes:
> On Mon, Dec 09, 2024 at 12:07:19PM +0100, Karthik Nayak wrote:
>> diff --git a/refs.c b/refs.c
>> index 732c236a3fd0cf324cc172b48d3d54f6dbadf4a4..602a65873181a90751def525608a7fa7bea59562 100644
>> --- a/refs.c
>> +++ b/refs.c
>> @@ -1160,13 +1160,15 @@ void ref_transaction_free(struct ref_transaction *transaction)
>> free(transaction);
>> }
>>
>> -struct ref_update *ref_transaction_add_update(
>> - struct ref_transaction *transaction,
>> - const char *refname, unsigned int flags,
>> - const struct object_id *new_oid,
>> - const struct object_id *old_oid,
>> - const char *new_target, const char *old_target,
>> - const char *msg)
>> +struct ref_update *ref_transaction_add_update(struct ref_transaction *transaction,
>> + const char *refname,
>> + unsigned int flags,
>> + const struct object_id *new_oid,
>> + const struct object_id *old_oid,
>> + const char *new_target,
>> + const char *old_target,
>> + const char *committer_info,
>> + const char *msg)
>> {
>> struct ref_update *update;
>>
>
> I'd personally avoid reindenting this block. It's somewhat-common
> practice to not align all arguments with the opening brace when the line
> would become too long. The reindents also distract a bit from the actual
> changes done in other places further down.
>
Makes sense, I'll undo that.
>> @@ -1190,8 +1192,15 @@ struct ref_update *ref_transaction_add_update(
>> oidcpy(&update->new_oid, new_oid);
>> if ((flags & REF_HAVE_OLD) && old_oid)
>> oidcpy(&update->old_oid, old_oid);
>> - if (!(flags & REF_SKIP_CREATE_REFLOG))
>> + if (!(flags & REF_SKIP_CREATE_REFLOG)) {
>> + if (committer_info) {
>> + struct strbuf sb = STRBUF_INIT;
>> + strbuf_addstr(&sb, committer_info);
>> + update->committer_info = strbuf_detach(&sb, NULL);
>
> Can't we simplify this via `xstrdup()`?
>
Yup, Christian suggested the same too, will fix it up.
>> @@ -3080,10 +3081,12 @@ static int files_transaction_finish_initial(struct files_ref_store *refs,
>> }
>>
>> /*
>> - * packed-refs don't support symbolic refs and root refs, so we
>> - * have to queue these references via the loose transaction.
>> + * packed-refs don't support symbolic refs, root refs and reflogs,
>> + * so we have to queue these references via the loose transaction.
>> */
>> - if (update->new_target || is_root_ref(update->refname)) {
>> + if (update->new_target ||
>> + is_root_ref(update->refname) ||
>> + (update->flags & REF_LOG_ONLY)) {
>> if (!loose_transaction) {
>> loose_transaction = ref_store_transaction_begin(&refs->base, 0, err);
>> if (!loose_transaction) {
>
> Makes sense. While we already had REF_LOG_ONLY beforehand, it was only
> used in very specific cases and thus the support implemented by the
> backends is lacking. And given that the packed-ref backend does not
> support reflogs we have to queue these up via the loose backend.
>
> Patrick
Attachment:
signature.asc
Description: PGP signature
