Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

build: use full paths on compile/link targets #6158

Merged
merged 2 commits into from
Jan 20, 2024

Conversation

kmk3
Copy link
Collaborator

@kmk3 kmk3 commented Jan 16, 2024

This makes the compile commands clearer when building in parallel (with
make -j) and ensures that __FILE__ includes the full build-time path
(relative to the root of the repository) whenever it is referenced, such
as in failed assert() messages (currently the full path is only shown in
errExit() messages). Example:

Before:

firejail: main.c:100: main: Assertion `1 == 2' failed.
Error src/firecfg/main.c:100: main: malloc: Cannot allocate memory

After:

firejail: ../../src/firejail/main.c:100: main: Assertion `1 == 2' failed.
Error ../../src/firecfg/main.c:100: main: malloc: Cannot allocate memory

Commands used to search and replace:

$ git grep -Ilz '^MOD_DIR =' -- '*Makefile' | xargs -0 -I '{}' \
  sh -c "printf '%s\n' \"\$(sed -E \
    -e 's|^MOD_DIR = src/(.*)|MOD = \\1\\nMOD_DIR = \$(ROOT)/src/\$(MOD)|' \
    -e 's:^(PROG|SO) = [^.]+(\.so)?$:\\1 = \$(MOD_DIR)/\$(MOD)\2:' \
    '{}')\" >'{}'"
$ git grep -Ilz '^HDRS :=' -- '*.mk' | xargs -0 -I '{}' \
  sh -c "printf '%s\n' \"\$(sed -E \
    -e 's|wildcard (\*\..)|wildcard \$(MOD_DIR)/\\1|' '{}')\" >'{}'"

Note: config.mk.in, src/fnettrace/Makefile and src/include/common.h were
edited manually.

This is a follow-up to #5871.

Make it more similar to the assert() message format for consistency.
Example:

Before:

    firejail: main.c:100: main: Assertion `1 == 2' failed.
    Error src/firecfg/main.c:100 main(): malloc: Cannot allocate memory

After:

    firejail: main.c:100: main: Assertion `1 == 2' failed.
    Error src/firecfg/main.c:100: main: malloc: Cannot allocate memory

This amends commit b963fe4 ("Improve errExit error messages",
2023-06-16) / PR netblue30#5871.
This makes the compile commands clearer when building in parallel (with
`make -j`) and ensures that `__FILE__` includes the full build-time path
(relative to the root of the repository) whenever it is referenced, such
as in failed assert() messages (currently the full path is only shown in
errExit() messages).  Example:

Before:

    firejail: main.c:100: main: Assertion `1 == 2' failed.
    Error src/firecfg/main.c:100: main: malloc: Cannot allocate memory

After:

    firejail: ../../src/firejail/main.c:100: main: Assertion `1 == 2' failed.
    Error ../../src/firecfg/main.c:100: main: malloc: Cannot allocate memory

Commands used to search and replace:

    $ git grep -Ilz '^MOD_DIR =' -- '*Makefile' | xargs -0 -I '{}' \
      sh -c "printf '%s\n' \"\$(sed -E \
        -e 's|^MOD_DIR = src/(.*)|MOD = \\1\\nMOD_DIR = \$(ROOT)/src/\$(MOD)|' \
        -e 's:^(PROG|SO) = [^.]+(\.so)?$:\\1 = \$(MOD_DIR)/\$(MOD)\2:' \
        '{}')\" >'{}'"
    $ git grep -Ilz '^HDRS :=' -- '*.mk' | xargs -0 -I '{}' \
      sh -c "printf '%s\n' \"\$(sed -E \
        -e 's|wildcard (\*\..)|wildcard \$(MOD_DIR)/\\1|' '{}')\" >'{}'"

Note: config.mk.in, src/fnettrace/Makefile and src/include/common.h were
edited manually.

This is a follow-up to netblue30#5871.
@kmk3 kmk3 merged commit 4f13411 into netblue30:master Jan 20, 2024
13 checks passed
@kmk3 kmk3 deleted the build-use-full-paths branch January 20, 2024 21:15
kmk3 added a commit that referenced this pull request Jan 20, 2024
kmk3 added a commit to kmk3/firejail that referenced this pull request Jan 22, 2024
Instead of manually specifying which source files depend on which
headers, use compiler flags to automatically generate depfiles (.d),
which declare the correct header (make) dependencies for each source
file (.c).

Use `-MMD` (which ignores system headers) to generate the dependencies
and `-MP` to prevent make from complaining when a header file is removed
while it is listed as a dependency in a depfile.

If depfiles exist, just include them.  If not, make each object file
(.o) unconditionally depend on all header files in its source directory
and in src/include, to ensure that rebuilds are done when needed.  The
latter case applies for the first build after `make clean` (which would
build everything anyway) and when the compiler does not support
generating depfiles.

Note that both gcc and clang have supported these options for a long
time.

Misc: This depends on the changes from commit 5b1bd33 ("build: use
full paths on compile/link targets", 2023-07-02) / PR netblue30#6158 to avoid
issues with make dependency tracking.
kmk3 added a commit to kmk3/firejail that referenced this pull request Jan 22, 2024
Instead of manually specifying which source files depend on which
headers, use compiler flags to automatically generate depfiles (.d),
which declare the correct header (make) dependencies for each source
file (.c).

Use `-MMD` (which ignores system headers) to generate the dependencies
and `-MP` to prevent make from complaining when a header file is removed
while it is listed as a dependency in a depfile.

If depfiles exist, just include them.  If not, make each object file
(.o) unconditionally depend on all header files in its source directory
and in src/include, to ensure that rebuilds are done when needed.  The
latter case applies to the first build after `make clean` (which would
build everything anyway) and when the compiler does not support
generating depfiles.

Note that both gcc and clang have supported these options for a long
time.

Misc: This depends on the changes from commit 5b1bd33 ("build: use
full paths on compile/link targets", 2023-07-02) / PR netblue30#6158 to avoid
issues with make dependency tracking.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
Status: Done (on RELNOTES)
Development

Successfully merging this pull request may close these issues.

1 participant