Re: [PATCH] add a reflog_exists and delete_reflog abstraction

From: Ronnie Sahlberg <sahlberg@google.com>
Date: May 7, 2014, 20:47 UTC
Message-ID: <CAL=YDW=dhNR3qy9mym9naS9QrHhghSZmoQ=2L1nA47CtmY90=g@mail.gmail.com>
In-Reply-To: <5368FE5E.7020009@alum.mit.edu>

On Tue, May 6, 2014 at 8:23 AM, Michael Haggerty <mhagger@alum.mit.edu> wrote:

Show 27 quoted lines

> On 05/06/2014 12:57 AM, Ronnie Sahlberg wrote:
>> This is a single patch that adds two new functions to try to hide the reflog
>> implementation details from the callers in checkout.c and reflog.c.
>> It adds new functions to test if a reflog exists and to delete it, thus
>> allowing checkout.c to perform this if-test-then-delete operation without
>> having to know the internal implementation of reflogs (i.e. that they are files
>> that live unde r.git/logs)
>>
>> It also changes checkout.c to use ref_exists when checking for whether a ref
>> exists or not instead of checking if the loose ref file exists or not.
>> Using ref_exists instead both hides the reflog implementation details from
>> checkout.c as well as making the code more robust against future changes.
>> It currently has a hard assumption that the loose ref file must exist at this
>> stage or else it would end up deleting the reflog which is true today, as far
>> as I can tell, but would break if git would change such that we could have
>> a branch checked out without having a loose ref file for that branch.
>
> For single patches, people usually don't send a separate cover letter.
> Any comments that you want to make that are not suitable for the commit
> message can go between the "---" line and the diffstat of the patch email.
>
> Regarding this change, I think before too long we will also need an API
> to turn reflogs on for a reference.  We might want to add a flag to ref
> transaction entries that cause the reflog to be created if it doesn't
> already exist.  Reflogs can currently be created for a reference via the
> config setting "core.logAllRefUpdates" or (for branches) by "git branch
> --create-reflog" (maybe there are others).

Yes. I agree.

I think we need a flag to ref_transaction_create and ref_transaction_update to create a new reflog.

Then I also think we need to add a new transaction command, ref_transaction_reflog() that can be used to either append or overwrite the reflog. Builtin/reflog.c will need an API to create/overwrite the reflog with new text.

And finally we also need an api to create symrefs. ref_transaction_symref() or something.

But this will wait until my current patch series is merged.

>
> Several tests in the test suite currently create reflog files by hand.
> Thus we might also need a way to create reflogs at the command line by
> the time we implement pluggable reference backends.

I can add this once we have an api.

Show 7 quoted lines

>
> Michael
>
> --
> Michael Haggerty
> mhagger@alum.mit.edu
> http://softwareswirl.blogspot.com/