Re: [PATCH] pack-objects: move builtin-only code to its own header

Ævar Arnfjörð Bjarmason  [off-list ref] writes:

Move the code that's only used in builtin/pack-objects.c to a new
builtin/pack-objects.h header and out of pack-objects.h.

I've amended the early part of the proposed log message in
preparation for merging it to 'next' and then later down to
'master'.

Here is what the result looks like.

Thanks.


Author: Ævar Arnfjörð Bjarmason [off-list ref]
Date:   Thu May 27 02:52:51 2021 +0200

    pack-objects: move static inline from a header to its sole consumer
    
    Move the code that is only used in builtin/pack-objects.c out of
    pack-objects.h.
    
    This fixes an issue where Solaris's SunCC hasn't been able to compile
    git since 483fa7f42d9 (t/helper/test-bitmap.c: initial commit,
    2021-03-31).
    
    The real origin of that issue is that in 898eba5e630 (pack-objects:
    refer to delta objects by index instead of pointer, 2018-04-14)
    utility functions only needed by builtin/pack-objects.c were added to
    pack-objects.h. Since then the header has been used in a few other
    places, but 483fa7f42d9 was the first time it was used by test helper.
    
    Since Solaris is stricter about linking and the oe_get_size_slow()
    function lives in builtin/pack-objects.c the build started failing
    with:
    
        Undefined                       first referenced
         symbol                             in file
        oe_get_size_slow                    t/helper/test-bitmap.o
        ld: fatal: symbol referencing errors. No output written to t/helper/test-tool
    
    On other platforms this is presumably OK because the compiler and/or
    linker detects that the "static inline" functions that reference
    oe_get_size_slow() aren't used.
    
    Let's solve this by moving the relevant code from pack-objects.h to
    builtin/pack-objects.c. This is almost entirely a code-only move, but
    because of the early macro definitions in that file referencing some
    of these inline functions we need to move the definition of "static
    struct packing_data to_pack" earlier, and declare these inline
    functions above the macros.
    
    Signed-off-by: Ævar Arnfjörð Bjarmason [off-list ref]
    Signed-off-by: Junio C Hamano [off-list ref]

On Thu, May 27 2021, Junio C Hamano wrote:

Ævar Arnfjörð Bjarmason  [off-list ref] writes:

quoted

Move the code that's only used in builtin/pack-objects.c to a new
builtin/pack-objects.h header and out of pack-objects.h.

I've amended the early part of the proposed log message in
preparation for merging it to 'next' and then later down to
'master'.

Here is what the result looks like.

Thanks.


Author: Ævar Arnfjörð Bjarmason [off-list ref]
Date:   Thu May 27 02:52:51 2021 +0200

    pack-objects: move static inline from a header to its sole consumer
    
    Move the code that is only used in builtin/pack-objects.c out of
    pack-objects.h.
    
    This fixes an issue where Solaris's SunCC hasn't been able to compile
    git since 483fa7f42d9 (t/helper/test-bitmap.c: initial commit,
    2021-03-31).
    
    The real origin of that issue is that in 898eba5e630 (pack-objects:
    refer to delta objects by index instead of pointer, 2018-04-14)
    utility functions only needed by builtin/pack-objects.c were added to
    pack-objects.h. Since then the header has been used in a few other
    places, but 483fa7f42d9 was the first time it was used by test helper.
    
    Since Solaris is stricter about linking and the oe_get_size_slow()
    function lives in builtin/pack-objects.c the build started failing
    with:
    
        Undefined                       first referenced
         symbol                             in file
        oe_get_size_slow                    t/helper/test-bitmap.o
        ld: fatal: symbol referencing errors. No output written to t/helper/test-tool
    
    On other platforms this is presumably OK because the compiler and/or
    linker detects that the "static inline" functions that reference
    oe_get_size_slow() aren't used.
    
    Let's solve this by moving the relevant code from pack-objects.h to
    builtin/pack-objects.c. This is almost entirely a code-only move, but
    because of the early macro definitions in that file referencing some
    of these inline functions we need to move the definition of "static
    struct packing_data to_pack" earlier, and declare these inline
    functions above the macros.
    
    Signed-off-by: Ævar Arnfjörð Bjarmason [off-list ref]
    Signed-off-by: Junio C Hamano [off-list ref]

Looks good to me for what it's worth, and I see you merged this down
already, thanks! Git builds again on the Solaris boxes now.