build: deduplicate makefiles #5478

kmk3 · 2022-11-21T20:51:56Z

Main commits:

makefiles: include config.mk directly
makefiles: equalize object dependencies in program targets
makefiles: rename H_FILE_LIST and C_FILE_LIST
makefiles: expand HDRS, SRCS and OBJS immediately
makefiles: move extra deps into new MOD vars
makefiles: deduplicate main target name into new PROG var
makefiles: deduplicate many makefiles into common.mk
makefiles: deduplicate main target name into new SO var
makefiles: deduplicate lib makefiles into so.mk
makefiles: rename common.mk to prog.mk
makefiles: organize CFLAGS

This is kind of a continuation of #5140 and #5219.

Added on commit a627071 ("intrusion detection system", 2021-07-28).

It is unclear what its intended purpose would be. Example: $ cat Makefile OBJS = a b c BINOBJS = $(foreach file, $(OBJS), $file) all: printf '"%s"\n' "$(BINOBJS)" $ make printf '"%s"\n' " ile ile ile" " ile ile ile" Added on commit 1379851 ("Baseline firejail 0.9.28", 2015-08-08).

The "all" target is usually intended to be the default one and when running make, the first target on a makefile is the one that gets built if no target is specified (such as when running `make` with no arguments). Also, note that unlike config.mk, src/common.mk may define its own targets, so move the "all" target to before the include of src/common.mk, to ensure that "all" keeps being the default target regardless of what is defined in src/common.mk. Note: If the "all" target is defined as depending directly on `$(OBJS)` while it is empty (that is, before src/common.mk is included), running `make` (or `make all`) will result in make always concluding that there is nothing to be done and exiting. So make "all" depend on an intermediary phony "lib" target instead, which in turn depends on `$(OBJS)` (and is declared after `$(OBJS)` is populated).

This should make it more consistent with the other makefiles (especially considering the subsequent deduplication commits on this branch) and enables it to depend on the variables in question (as variables in dependencies are immediately expanded, at least by default).

Instead of including it through src/common.mk. This allows each makefile to directly override any value defined in config.mk.

Compared to the objects that are actually used in a given recipe, some program targets are missing object dependencies, while others appear to have unused object dependencies. Make each of those targets depend on the objects that are actually used when linking. Note: No check was done for extraneous/missing objects when linking; this commit only makes the object dependencies equal to the objects that are linked.

To HDRS and SRCS, respectively. To be more consistent with the OBJS variable. Misc: These names also appear to be more common from the makefiles that I've seen. Commands used to search and replace: git grep -IFlz -e H_FILE_LIST -e C_FILE_LIST -- src | xargs -0 -I '{}' sh -c "printf '%s\n' \"\$(sed \ -e 's/^H_FILE_LIST *=/HDRS =/' \ -e 's/\$(H_FILE_LIST)/\$(HDRS)/g' \ -e 's/^C_FILE_LIST *=/SRCS =/' \ -e 's/\$(C_FILE_LIST:/\$(SRCS:/g' \ '{}')\" >'{}'"

Use immediate expansion of the right-hand side (with `:=`) to set the variables to the output of the commands rather than to the (text of the) commands themselves. This should prevent deferred/lazy evaluation, which is something that might potentially result in the relevant files being looked up each time that HDRS and SRCS are evaluated. Commands used to search and replace: git grep -Ilz '^SRCS' -- src | xargs -0 -I '{}' \ sh -c "printf '%s\n' \"\$(sed \ -e 's/^HDRS =/HDRS :=/' \ -e 's/^SRCS =/SRCS :=/' \ -e 's/^OBJS =/OBJS :=/' '{}')\" >'{}'"

To make the makefiles more similar. That is, add the following new variables: * MOD_HDRS * MOD_SRCS * MOD_OBJS And move existing module-specific header and object dependencies into `MOD_HDRS` and `MOD_OBJS`, respectively. `MOD_SRCS` is added mostly for symmetry/consistency. Note: Use `MOD_` as a prefix instead of `EXTRA_` to avoid confusion, as the latter is currently used for global variables (such as `EXTRA_CFLAGS`), as opposed to module-specific variables. Note2: Add them directly into the HDRS/SRCS/OBJS variables to avoid cluttering the existing recipes with an extra variables unnecessarily. This also allows, for example, referencing all of the object dependencies with `$<` if `$(OBJS)` is the first dependency (at least in GNU make). Note3: Since HDRS/SRCS/OBJS use simple assignment (through `:=`), the MOD variables should appear before including src/common.mk (or src/so.mk).

For increased readability, list one item per line on lines that are currently longer than 80 characters.

Put the main target name into a new PROG variable, put PROG into a new TARGET variable, make "all" depend on `$(TARGET)` and replace every other occurrence of the main target name with `$(PROG)`. On the makefiles that build non-shared objects, to make them more similar. With this commit, all of their targets are identical (except for the extra "lib" target on src/lib/Makefile).

The makefiles that both build C programs and include src/common.mk are nearly identical, save for the main target name and for any extra headers and objects that they might use. So move all of their (duplicated) code into src/common.mk, which (other than the "lib" target on src/lib/Makefile) leaves only variables and the includes of config.mk and src/common.mk in place.

Put the main target name into a new SO variable, put SO into a new TARGET variable, make "all" depend on `$(TARGET)` and replace every other occurrence of the main target name with `$(SO)`. On the makefiles that build shared objects, to make them more similar. With this commit, all of their targets are identical.

The following makefiles are nearly identical, except for the main target name and for any extra headers that they might use: * src/libpostexecseccomp/Makefile * src/libtrace/Makefile * src/libtracelog/Makefile So move all of their (duplicated) code into a new src/so.mk file, and add an include of src/so.mk, which leaves only variables, and the includes of config.mk and src/so.mk in place. With this commit, CFLAGS and LDFLAGS are only defined/changed in the following files: * config.mk.in * src/common.mk * src/so.mk

For clarity, as it is included by the Makefiles that create programs and non-shared-objects, but not by the ones that create shared objects (see src/so.mk). Commands used to move and search and replace: $ git mv src/common.mk src/prog.mk $ git grep -IFlz 'common.mk' -- src | xargs -0 -I '{}' sh -c \ "printf '%s\n' \"\$(sed 's/common.mk/prog.mk/' '{}')\" >'{}'"

So that includers of src/prog.mk or src/so.mk can just define anything extra that needs to be cleaned without having to override the "clean" target (or having to declare a "distclean" target). Example usage: TOCLEAN += foo TODISTCLEAN += bar

On src/prog.mk and src/so.mk.

Line-wrap them and make the order of the flags more similar across src/prog.mk and src/so.mk. This should make it easier to see the differences in CFLAGS between both files.

kmk3 · 2022-11-21T20:56:01Z

@reinerh commented on Nov 2, 2021:

We currently also have a lot of redundancy in our Makefile.in files (and
some are not even in sync with others).

This PR should help with that.

Note: Some other planned improvements will probably depend on adding some
duplication back, though it should not be nearly as much as in the current
state (that is, without this PR).

Note2: There is more that can be done with regards to deduplication, but this
PR is mostly focused on mechanical changes and on refactoring the code only.

kmk3 · 2022-11-21T21:31:40Z

@reinerh commented on Nov 2, 2021:

And they are "buggy", as the object targets miss stuff like CPPFLAGS, the
linking targets miss CFLAGS etc... common.mk.in is also a bit of a mess.

After this PR, the idea is to do things like that. For example:

Add missing CPPFLAGS and CFLAGS on targets (as you mentioned)
Deduplicate CFLAGS and LDFLAGS by moving them into config.mk (from prog.mk
and so.mk)
Use different variables for project-specific values, to allow the
user/packager to override CFLAGS/LDFLAGS directly or with environment
variables (like in /etc/makepkg.conf on Arch Linux)

For details on the latter item, see the description of CFLAGS (the relevant
explanation starts on the third paragraph) from the manual of GNU Autoconf
(version 2.69):

 -- Variable: CFLAGS
     Debugging and optimization options for the C compiler.  If it is
     not set in the environment when 'configure' runs, the default value
     is set when you call 'AC_PROG_CC' (or empty if you don't).
     'configure' uses this variable when compiling or linking programs
     to test for C features.

     If a compiler option affects only the behavior of the preprocessor
     (e.g., '-DNAME'), it should be put into 'CPPFLAGS' instead.  If it
     affects only the linker (e.g., '-LDIRECTORY'), it should be put
     into 'LDFLAGS' instead.  If it affects only the compiler proper,
     'CFLAGS' is the natural home for it.  If an option affects multiple
     phases of the compiler, though, matters get tricky.  One approach
     to put such options directly into 'CC', e.g., 'CC='gcc -m64''.
     Another is to put them into both 'CPPFLAGS' and 'LDFLAGS', but not
     into 'CFLAGS'.

     However, remember that some 'Makefile' variables are reserved by
     the GNU Coding Standards for the use of the "user"--the person
     building the package.  For instance, 'CFLAGS' is one such variable.

     Sometimes package developers are tempted to set user variables such
     as 'CFLAGS' because it appears to make their job easier.  However,
     the package itself should never set a user variable, particularly
     not to include switches that are required for proper compilation of
     the package.  Since these variables are documented as being for the
     package builder, that person rightfully expects to be able to
     override any of these variables at build time.  If the package
     developer needs to add switches without interfering with the user,
     the proper way to do that is to introduce an additional variable.
     Automake makes this easy by introducing 'AM_CFLAGS' (*note
     (automake)Flag Variables Ordering::), but the concept is the same
     even if Automake is not used.

kmk3 · 2022-11-22T00:44:01Z

Notes regarding extensibility/flexibility in the makefiles with this PR:

Any variable used on config.mk can be extended/overridden by the includer.

Any variable used on src/prog.mk or src/so.mk can be extended by the includer.

Any variable used on src/prog.mk or src/so.mk other than the following
variables can be overridden by the includer:

HDRS
SCRS
OBJS
CFLAGS
LDFLAGS

Note: I think that it would be unlikely for this to be an issue, but it should
eventually be possible to override CFLAGS and LDFLAGS if they are later
moved from src/prog.mk and src/so.mk into config.mk.

For example, if a module in src/foo needs to define extra
compile-time/link-time flags, it can do so by adding them to EXTRA_CFLAGS /
EXTRA_LDFLAGS before including src/prog.mk. If it produces additional/custom
files that should be cleaned up, it can set TOCLEAN and/or TODISTCLEAN:

src/foo/Makefile:

ROOT = ../..
-include $(ROOT)/config.mk

PROG = foo
TARGET = $(PROG)

EXTRA_CFLAGS += -O1
EXTRA_LDFLAGS += -lm

TOCLEAN += foo.foo
TODISTCLEAN += foo.bar

include $(ROOT)/src/prog.mk

Also, if a module differs too much from what is used on src/prog.mk and
src/so.mk, instead of including them, the module's Makefile could include only
config.mk and manually define its own custom variables and targets, similarly
to what is currently done without this PR.

netblue30 · 2022-11-25T16:24:24Z

merged, thanks!

Relates to #5429 #5478 #5481.

It is unused and is unlikely to be used. Added on commit f5b1cca ("makefiles: move extra deps into new MOD vars", 2022-05-07) / PR netblue30#5478.

To make them less confusing, as they are extra dependencies, not files that are specific to the module. Commands used to search and replace: $ git grep -IFlz -e 'MOD_HDRS' -e 'MOD_OBJS' -- src | xargs -0 -I '{}' sh -c "printf '%s\n' \"\$(sed \ -e 's/MOD_HDRS/EXTRA_HDRS/g' \ -e 's/MOD_OBJS/EXTRA_OBJS/g' '{}')\" >'{}'" Added on commit f5b1cca ("makefiles: move extra deps into new MOD vars", 2022-05-07) / PR netblue30#5478.

To CLEANFILES and DISTCLEANFILES, respectively. This matches what GNU automake uses. Commands used to search and replace: $ git grep -IFlz -e TOCLEAN -e TODISTCLEAN | xargs -0 -I '{}' sh -c "printf '%s\n' \"\$(sed \ -e 's/TOCLEAN/CLEANFILES/g' \ -e 's/TODISTCLEAN/DISTCLEANFILES/g' '{}')\" >'{}'" Added on commit cbdee65 ("makefiles: add TOCLEAN and TODISTCLEAN variables", 2022-07-15) / PR netblue30#5478.

kmk3 added 18 commits November 20, 2022 17:01

fids/Makefile: remove code comment

c208939

Added on commit a627071 ("intrusion detection system", 2021-07-28).

makefiles: include config.mk directly

72345b7

Instead of including it through src/common.mk. This allows each makefile to directly override any value defined in config.mk.

makefiles: line-wrap MOD_HDRS and MOD_OBJS

350755c

For increased readability, list one item per line on lines that are currently longer than 80 characters.

makefiles: mention variables intended to be used by includers

f938149

On src/prog.mk and src/so.mk.

makefiles: organize CFLAGS

e425d84

Line-wrap them and make the order of the flags more similar across src/prog.mk and src/so.mk. This should make it easier to see the differences in CFLAGS between both files.

netblue30 merged commit 3e3a872 into netblue30:master Nov 25, 2022

kmk3 deleted the build-dedup-makefiles branch November 25, 2022 17:08

kmk3 added a commit that referenced this pull request Nov 25, 2022

RELNOTES: add build and ci items

8b7e343

Relates to #5429 #5478 #5481.

kmk3 mentioned this pull request Dec 6, 2022

build: actually set LDFLAGS/LIBS & stop overriding CFLAGS/LDFLAGS #5504

Merged

kmk3 added a commit to kmk3/firejail that referenced this pull request Jun 25, 2023

build: remove MOD_SRCS variable

862dff0

It is unused and is unlikely to be used. Added on commit f5b1cca ("makefiles: move extra deps into new MOD vars", 2022-05-07) / PR netblue30#5478.

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

build: deduplicate makefiles #5478

build: deduplicate makefiles #5478

kmk3 commented Nov 21, 2022 •

edited

Loading

kmk3 commented Nov 21, 2022 •

edited

Loading

kmk3 commented Nov 21, 2022

kmk3 commented Nov 22, 2022 •

edited

Loading

netblue30 commented Nov 25, 2022

build: deduplicate makefiles #5478

build: deduplicate makefiles #5478

Conversation

kmk3 commented Nov 21, 2022 • edited Loading

kmk3 commented Nov 21, 2022 • edited Loading

kmk3 commented Nov 21, 2022

kmk3 commented Nov 22, 2022 • edited Loading

netblue30 commented Nov 25, 2022

kmk3 commented Nov 21, 2022 •

edited

Loading

kmk3 commented Nov 21, 2022 •

edited

Loading

kmk3 commented Nov 22, 2022 •

edited

Loading