Re: [PATCH 05/27] object-store: move packed_git and packed_git_mru to object store

[Stefan Beller <sbeller@google.com>]

>>  2. Applying the semantic patch
>>     contrib/coccinelle/refactoring/packed_git.cocci to adjust callers.
>>     This semantic patch is placed in a sub directory of the coccinelle
>>     contrib dir, as this semantic patch is not expected to be of general
>>     usefulness; it is only useful during developing this series and
>>     merging it with other topics in flight. At a later date, just
>>     delete that semantic patch.
>
> Can the semantic patch go in the commit message instead?  It is very
> brief.

done

>
> Actually, I don't see this semantic patch in the diffstat.  Is the
> commit message stale?
>
>>  3. Applying line wrapping fixes from "make style" to break the
>>     resulting long lines.
>>
>>  4. Adding missing #includes of repository.h and object-store.h
>>     where needed.
>
> Is there a way to automate this step?  (I'm asking for my own
> reference when writing future patches, not because of any concern
> about the correctness of this one.)

no, for several reasons.
(a) I don't know how to write it in coccinelle, because
(b) I have not figured out the order in which we include headers, apart from
  "cache.h goes first", the rest of the includes sometimes looks like a random
  order, because different patches add new includes at different places.
  I have the impression, that (1) some add the include after all other
  existing includes or (2) try to figure out where to add the include to make
  sense in the existing include order or (3) sort alphabetically or (4) put it
  randomly, so the chance for a merge conflict with other series in flight
  decreases.
(c) I did it in a semi automated fashion:
    while make fails:
        add another include

The mess with including repository or object-store comes from the fact that
I had v2 based on object-store, not the repository and cherry-picked this patch
over to v3. Fixed all of the includes now.

>> @@ -59,10 +83,25 @@ struct raw_object_store {
>>        */
>>       char *objectdir;
>>
>> +     struct packed_git *packed_git;
>> +     /*
>> +      * A most-recently-used ordered version of the packed_git list, which can
>> +      * be iterated instead of packed_git (and marked via mru_mark).
>> +      */
>> +     struct list_head packed_git_mru;
>
> I don't understand the new part of the comment.  Can you explain here,
> for me?

cherrypicking error, fixed.

>> -#define RAW_OBJECT_STORE_INIT { NULL, NULL, NULL }
>> +
>> +/*
>> + * The mru list_head is supposed to be initialized using
>> + * the LIST_HEAD macro, assigning prev/next to itself.
>> + * However this doesn't work in this case as some compilers dislike
>> + * that macro on member variables. Use NULL instead as that is defined
>> + * and accepted, deferring the real init to prepare_packed_git_mru(). */
>
> style nit: '*/' should be on its own line.
>
> More importantly, we can avoid such an issue as described by Junio. :)

See reply to Junio, I am not quite sure I like that.