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.