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

Re: [PATCH 1/4] Makefile: move "Platform specific tweaks" above LIB_{H,OBJS}

From
Ævar Arnfjörð Bjarmason <avarab@gmail.com>
Date
Nov 14, 2010, 17:51 UTC
Message-ID
<AANLkTinaDhXQjPC6s1eFb8HwEReW7P-ObmwRn2Hpb=O4@mail.gmail.com>
In-Reply-To
<20101114172331.GA26459@burratino>
On Sun, Nov 14, 2010 at 18:23, Jonathan Nieder <jrnieder@gmail.com> wrote:
Show 49 quoted lines
> Ævar Arnfjörð Bjarmason wrote:
>
>> Change the Makefile so that the "Platform specific tweaks" section
>> comes before the assignments to LIB_H and LIB_OBJS.
>
> Currently the Makefile is structured like this:
>
>        A. default target
>        B. basics
>           1. basic configuration section
>              a. -include GIT-VERSION-FILE and recipe to generate it
>              b. uname_S := $(shell uname -s) and similar variables
>              c. user-facing compilation variables: CFLAGS, LDFLAGS, STRIP
>              d. user-facing paths: prefix, bindir_relative, etc
>              e. program names: CC, AR, etc
>           2. basic cflags and ldflags (almost configurable)
>           3. main list of program targets:
>              SCRIPTS, PROGRAMS, TEST_PROGRAMS, BUILT_INS,
>              OTHER_PROGRAMS, BINDIR_PROGRAMS
>           4. defaults for SHELL_PATH, PERL_PATH, PYTHON_PATH
>           5. main list of library targets:
>              LIB_FILE, XDIFF_LIB, LIB_H, LIB_OBJS, BUILTIN_OBJS
>           6. GITLIBS, EXTLIBS for the linker command line
>           7. platform-specific tweaks
>           8. -include config.mak, config.mak.autogen
>        C. preparations
>           1. handling of the various NO_THIS_OR_THAT options.
>              This affects BASIC_CFLAGS, COMPAT_CFLAGS,
>              COMPAT_OBJS, PROGRAMS, EXTLIBS, LIB_OBJ, LIB_H, etc
>           2. machinery for non-noisy build
>           3. shell-quoted and C-quoted variables
>           4. ALL_CFLAGS, ALL_LDFLAGS
>        D. main build rules
>           1. all:: targets for the main build, subdirs
>           2. shell sanity check
>           3. building the git binary and built-ins
>           4. scripts and gitweb
>           5. autoconf
>           6. building objects:
>              a. %.o: %.c rule, header deps, dependency checking
>              b. target-specific -D flags
>           7. building non-builtins, remote-curl
>           8. libs
>           9. subdirs
>        E. GIT-CFLAGS, GIT-BUILD-OPTIONS, GIT-GUI-VARS
>        F. bin-wrappers
>        G. tests
>        H. installation rules
>        I. maintainer's dist rules, check-doc, coverage, etc

That listing should be in a comment at the start of the Makefile. Please submit a patch for that!

> This patch proposes moving A5 (main list of library targets) after
> A8 (end of configuration).
Right. Thanks for the helpful outline.
Show 14 quoted lines
>> In the ab/i18n series I only want to build gettext.o (by adding it to
>> LIB_OBJS) if NO_GETTEXT is unset. It's not possible to do that without
>> an ugly hack if we haven't applied our platform specific tweaks before
>> LIB_{H,OBJS} gets assigned to.
>>
>> See <201008140002.40587.j6t@kdbg.org> (subject: "[PATCH] Do not build
>> i18n on Windows.") for Johannes's original report, and my follow-up in
>> <AANLkTiku5R+idX-C8f0AcCikBLmfEb5ZEhdft+CSRzU0@mail.gmail.com> where I
>> suggested that the problem be solved in the manner of this patch.
>
> This doesn't motivate the patch all all to me.  Is changing the list
> of LIB_OBJS in section C1 really an ugly hack?  It is where
> configuration-specific things go and how BLK_SHA1, PROGRAM_OBJS, etc
> work already.

Yeah, because if it hadn't come before A8 it would have been *not* assigned to in the first place like we do everywhere else. Moving A8 before A5 enables us to do that.

Show 8 quoted lines
> That said, I can see another reason to move A3 and A5 lower down in
> the makefile.  Namely, they don't seem to have anything obvious to
> do with configuration.  Including the basic list of objects that
> high up may make the makefile easier to read straight through, but
> I don't think anyone is reading it straight through.
>
> So I wouldn't have anything against moving both A3 and A5 to right
> before C1, I just think it needs different motivation.

I was thinking about moving A3 & A5 further down. But since I just needed A5 now I only ended up doing that. Moving them both would clean things up though. I think we should do A8 as early as possible.

Previous: Jonathan NiederNext: Jonathan Nieder
Message 4 of 25 in “[PULL] ab/i18n-prereqs, prerequisites for ab/i18n”
  1. 0/4 [PULL] ab/i18n-prereqs, prerequisites for ab/i18nÆvar Arnfjörð Bjarmason, Nov 14, 2010
  2. 1/4 Makefile: move "Platform specific tweaks" above LIB_{H,OBJS}Ævar Arnfjörð Bjarmason, Nov 14, 2010
  3. Jonathan NiederNov 14, 2010
  4. Ævar Arnfjörð BjarmasonNov 14, 2010
  5. Jonathan NiederNov 14, 2010
  6. Junio C HamanoNov 15, 2010
  7. Jonathan NiederNov 15, 2010
  8. Ævar Arnfjörð BjarmasonNov 15, 2010
  9. Junio C HamanoNov 14, 2010
  10. Jonathan NiederNov 14, 2010
  11. Junio C HamanoNov 14, 2010
  12. 2/4 t7004-tag.sh: re-arrange git tag comment for clarityÆvar Arnfjörð Bjarmason, Nov 14, 2010
  13. Jonathan NiederNov 14, 2010
  14. Ævar Arnfjörð BjarmasonNov 14, 2010
  15. 3/4 tests: use test_cmp instead of piping to diff(1)Ævar Arnfjörð Bjarmason, Nov 14, 2010
  16. Jonathan NiederNov 14, 2010
  17. 4/4 builtin: use builtin.h for all builtin commandsÆvar Arnfjörð Bjarmason, Nov 14, 2010
  18. Sverre RabbelierNov 14, 2010
  19. Ævar Arnfjörð BjarmasonNov 14, 2010
  20. Junio C HamanoNov 14, 2010
  21. Jonathan NiederNov 14, 2010
  22. Junio C HamanoNov 17, 2010
  23. Ævar Arnfjörð BjarmasonNov 17, 2010
  24. Jonathan NiederNov 17, 2010
  25. Jonathan NiederNov 14, 2010

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.