git/list[1] front-page[2] threads[3] people[4] search[5] about
 

[PATCH v2 0/4] fixes related to `make hdr-check`

From
Denton Liu <liu.denton@gmail.com>
Date
Sep 25, 2019, 08:20 UTC
Message-ID
<cover.1569398897.git.liu.denton@gmail.com>
In-Reply-To
<cover.1569263631.git.liu.denton@gmail.com>

The first two patches fix errors causing `make hdr-check` to fail. The third is legacy from v1 but provides code cleanup so we leave it. Finally, the last patch is a patch which improves the portability and correctness of `hdr-check`.

Changes since v1:
* Reordered patches to put fixes first
* Took Peff's improvement suggestions for the $(HCO) target
* Added "pack-bitmap.h: fix unused variable warning"
Denton Liu (4):
  apply.h: include missing header
  promisor-remote.h: include missing header
  pack-bitmap.h: remove magic number
  Makefile: emulate compile in $(HCO) target better
 .gitignore        |  1 +
 Makefile          | 12 +++++++++---
 apply.h           |  1 +
 pack-bitmap.h     |  6 +++---
 promisor-remote.h |  2 ++
 5 files changed, 16 insertions(+), 6 deletions(-)
Range-diff against v1:
1:  0336d1345a < -:  ---------- Makefile: use $(ALL_CFLAGS) in $(HCO) target
2:  1fc6dfc5fa = 1:  74efb6c04c apply.h: include missing header
3:  8ccbd81673 = 2:  2befc450fb promisor-remote.h: include missing header
4:  a3a3357925 ! 3:  50e37c16f9 pack-bitmap.h: fix unused variable warning
    @@ Metadata
     Author: Denton Liu <liu.denton@gmail.com>
     
      ## Commit message ##
    -    pack-bitmap.h: fix unused variable warning
    +    pack-bitmap.h: remove magic number
     
    -    When we ran `make hdr-check`, we got the following warning on Arch Linux:
    +    When we ran `make hdr-check` with the following patch
    +
    +            diff --git a/Makefile b/Makefile
    +            index f879697ea3..d8df4e316b 100644
    +            --- a/Makefile
    +            +++ b/Makefile
    +            @@ -2773,7 +2773,7 @@ CHK_HDRS = $(filter-out $(EXCEPT_HDRS),$(patsubst ./%,%,$(LIB_H)))
    +            HCO = $(patsubst %.h,%.hco,$(CHK_HDRS))
    +
    +            $(HCO): %.hco: %.h FORCE
    +            -       $(QUIET_HDR)$(CC) -include git-compat-util.h -I. -o /dev/null -c -xc $<
    +            +       $(QUIET_HDR)$(CC) -include git-compat-util.h -I. -o /dev/null -c -xc $(ALL_CFLAGS) $<
    +
    +            .PHONY: hdr-check $(HCO)
    +            hdr-check: $(HCO)
    +
    +    and with `DEVELOPER=1`, we got the following warning on Arch Linux:
     
                 pack-bitmap.h:20:19: error: ‘BITMAP_IDX_SIGNATURE’ defined but not used [-Werror=unused-const-variable=]
                    20 | static const char BITMAP_IDX_SIGNATURE[] = {'B', 'I', 'T', 'M'};
    @@ Commit message
                 cc1: all warnings being treated as errors
     
         "Use" the BITMAP_IDX_SIGNATURE variable by making the size of
    -    bitmap_disk_header.magic equal to the size of BITMAP_IDX_SIGNATURE. An
    -    alternative was to simply add MAYBE_UNUSED. However, this design was
    -    chosen because we eliminate the magic number (4) in the process.
    +    bitmap_disk_header.magic equal to the size of BITMAP_IDX_SIGNATURE,
    +    thereby eliminating the magic number (4).
    +
    +    An alternative was to simply add MAYBE_UNUSED, however that does not
    +    eliminate the magic number.
    +
    +    Another alternative was to change the definition to
    +
    +            extern const char BITMAP_IDX_SIGNATURE[4];
    +
    +    However, this design was also not chosen as the static definition allows
    +    us to keep the declaration together for readability along with removing
    +    the magic number.
     
      ## pack-bitmap.h ##
     @@ pack-bitmap.h: struct commit;
-:  ---------- > 4:  14def72319 Makefile: emulate compile in $(HCO) target better
-- 
2.23.0.248.g3a9dd8fb08
Previous: Jeff KingNext: Denton Liu
Message 12 of 18 in “fixes related to `make hdr-check`”
  1. 0/3 fixes related to `make hdr-check`Denton Liu, Sep 23, 2019
  2. 1/3 Makefile: use $(ALL_CFLAGS) in $(HCO) targetDenton Liu, Sep 23, 2019
  3. Johannes SchindelinSep 26, 2019
  4. Denton LiuSep 26, 2019
  5. Johannes SchindelinSep 26, 2019
  6. Junio C HamanoSep 28, 2019
  7. 2/3 apply.h: include missing headerDenton Liu, Sep 23, 2019
  8. 3/3 promisor-remote.h: include missing headerDenton Liu, Sep 23, 2019
  9. 4/3 pack-bitmap.h: fix unused variable warningDenton Liu, Sep 24, 2019
  10. Jeff KingSep 24, 2019
  11. Jeff KingSep 24, 2019
  12. 0/4 fixes related to `make hdr-check`Denton Liu, Sep 25, 2019
  13. 1/4 apply.h: include missing headerDenton Liu, Sep 25, 2019
  14. 2/4 promisor-remote.h: include missing headerDenton Liu, Sep 25, 2019
  15. 3/4 pack-bitmap.h: remove magic numberDenton Liu, Sep 25, 2019
  16. 4/4 Makefile: emulate compile in $(HCO) target betterDenton Liu, Sep 25, 2019
  17. Jeff KingOct 2, 2019
  18. Johannes SchindelinSep 26, 2019

Read the whole thread, see it on lore, or plain text.

$ cat FOOTERMessages come from the public archive at lore.kernel.org/git, fetched every hour. The front page is chosen and written each morning by an AI editor and can be wrong; the threads themselves are the record. About and API. For agents: an MCP server at https://gitlist.dev/mcp, and any thread, story or person page as Markdown by adding .md to its URL (or sending Accept: text/markdown). Details in /llms.txt.