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

["Ævar Arnfjörð Bjarmason" <avarab@gmail.com>]

On Sun, Nov 14, 2010 at 18:23, Jonathan Nieder <jrnieder@gmail.com> wrote:
> Æ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.

>> 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.

> 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.