build: deduplicate configure-time vars into new config files #5140
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
|
@reinerh Could you test whether |
|
@kmk3 I don't use the |
kmk3
force-pushed
the
build-dedup-config-vars
branch
from
9ee1186 to
498edba
Compare
kmk3
force-pushed
the
build-dedup-config-vars
branch
from
498edba to
da98193
Compare
Good catch; with #5142 this should not be an issue anymore. Considering #5142, #5154 and contrib/fj-mkdeb.py, changing mkdeb.sh.in is more |
kmk3
changed the title
Actually, I now realized that contrib/fj-mkdeb.py does more than a work around; |
kmk3
force-pushed
the
build-dedup-config-vars
branch
from
da98193 to
c7d25b8
Compare
kmk3
changed the title
|
So, the next PR is the one that does the key changes and I want to reference it Since this was opened over a month ago (though admittedly it stayed most of the |
An output message and some whitespace were changed on commit 9903aaa ("rel 0.9.68rc1 testing", 2022-01-18). Environment: autoconf 2.69 (with the runstatedir patch) on Artix Linux
To make it easier to read and edit them and to make the diffs clearer.
vim commands used to search and replace:
:0/AC_CONFIG_FILES/1 | ,+3s/ \\// | -3,+1s/ /\r/g
They are being double-quoted twice, as in `""$(DISTFILES)""`, which is equivalent to not using quotes at all, as each double-quote pair gets expanded into nothing, leaving only `$(DISTFILES)`. Note that DISTFILES and DISTFILES_TEST are the only variables defined with quoted values and that make does not work with filenames that contain whitespace anyway. Added on commit da19d2d ("Simplify dist target and add missing test/sysutils to tarball", 2016-07-25) / PR netblue30#646.
kmk3
added a commit
to kmk3/firejail
that referenced
this pull request
Jun 25, 2022
Configure summary: autoconf essentially only parses configure.ac and generates the configure script (that is, the "./configure" shell script). The latter is what actually checks what is available on the system and internally sets the value of the output variables. It then, for every filename foo in AC_CONFIG_FILES (and for every output variable name BAR in AC_SUBST), reads foo.in, replaces every occurrence of @bar@ with the value of the shell variable $BAR and generates the file foo from the result. After this, configure is finished and `make` could be executed to start the build. Now that (as of netblue30#5140) all output variables are only defined on config.mk.in and on config.sh.in, there is no need to generate any makefile nor any other mkfile or shell script at configure time. So rename every "Makefile.in" to "Makefile", mkdeb.sh.in to mkdeb.sh, src/common.mk.in to src/common.mk and leave just config.mk and config.sh as the files to be generated at configure time. This allows editing and committing all makefiles directly, without potentially having to run ./configure in between. Commands used to rename the makefiles: $ git ls-files -z -- '*Makefile.in' | xargs -0 -I '{}' sh -c \ "git mv '{}' \"\$(dirname '{}')/Makefile\"" Additionally, from my (rudimentary) testing, this commit reduces the time it takes to run ./configure by about 20~25% (using ext4 on an HDD) compared to commit 72ece92 ("Transmission fixes: drop private-lib (netblue30#5213)", 2022-06-22). Commands used for benchmarking each commit: $ : >time_configure && ./configure && make distclean && for i in $(seq 1 10); do { time -p ./configure; } 2>>time_configure; done $ grep real time_configure | awk '{ total += $2 } END { print total/NR }'
kmk3
added a commit
to kmk3/firejail
that referenced
this pull request
Jun 25, 2022
This allows running `make clean` and `make distclean` (and possibly
others) without having to run ./configure beforehand.
Note that some packaging-related targets still depend on the existence
of generated files. For example:
* dist: config.mk
* deb: config.sh
Commands used to search and replace:
$ git grep -Elz 'include *([^ ]*/)?config.mk' | xargs -0 -I '{}' \
sh -c "printf '%s\n' \
\"\$(sed -E 's|^include *(([^ ]*/)?config.mk)|-include \1|' '{}')\" >'{}'"
Relates to netblue30#5140.
kmk3
added a commit
to kmk3/firejail
that referenced
this pull request
Jun 25, 2022
With the previous commit ("makefiles: stop failing when config.mk does
not exist", 2022-06-23), make will not immediately fail when trying to
build a target without having the proper compile-time flags (which are
defined on common.mk).
For example, when running the command below:
make distclean && make
It will throw an error only after (mis-)compiling multiple objects.
So add a dependency on config.mk on every target that uses output
variables (such as @name@ / $(NAME)) on its recipe. And add a
dependency on config.sh on targets that call shell scripts that use
output variables (such as @name@ / $NAME). Also, add a recipe for
config.mk / config.sh telling to run ./configure, to make it a bit more
obvious just in case.
With this commit, make will abort earlier, by detecting that the
config.mk / config.sh dependency does not exist. This happens before
trying to execute the recipe.
This also makes the dependencies more accurate, since if config.mk
(which defines some CFLAGS) is changed, the CFLAGS may also have
changed, so a target that uses CFLAGS should probably be considered out
of date in this case anyway.
Relates to netblue30#5140.
kmk3
added a commit
to kmk3/firejail
that referenced
this pull request
Jun 25, 2022
Configure summary: autoconf essentially only parses configure.ac and generates the configure script (that is, the "./configure" shell script). The latter is what actually checks what is available on the system and internally sets the value of the output variables. It then, for every filename foo in AC_CONFIG_FILES (and for every output variable name BAR in AC_SUBST), reads foo.in, replaces every occurrence of `@BAR@` with the value of the shell variable `$BAR` and generates the file foo from the result. After this, configure is finished and `make` could be executed to start the build. Now that (as of netblue30#5140) all output variables are only defined on config.mk.in and on config.sh.in, there is no need to generate any makefile nor any other mkfile or shell script at configure time. So rename every "Makefile.in" to "Makefile", mkdeb.sh.in to mkdeb.sh, src/common.mk.in to src/common.mk and leave just config.mk and config.sh as the files to be generated at configure time. This allows editing and committing all makefiles directly, without potentially having to run ./configure in between. Commands used to rename the makefiles: $ git ls-files -z -- '*Makefile.in' | xargs -0 -I '{}' sh -c \ "git mv '{}' \"\$(dirname '{}')/Makefile\"" Additionally, from my (rudimentary) testing, this commit reduces the time it takes to run ./configure by about 20~25% (using ext4 on an HDD) compared to commit 72ece92 ("Transmission fixes: drop private-lib (netblue30#5213)", 2022-06-22). Commands used for benchmarking each commit: $ : >time_configure && ./configure && make distclean && for i in $(seq 1 10); do { time -p ./configure; } 2>>time_configure; done $ grep real time_configure | awk '{ total += $2 } END { print total/NR }'
kmk3
added a commit
to kmk3/firejail
that referenced
this pull request
Jun 25, 2022
This allows running `make clean` and `make distclean` (and possibly
others) without having to run ./configure beforehand.
Note that some packaging-related targets still depend on the existence
of generated files. For example:
* dist: config.mk
* deb: config.sh
Commands used to search and replace:
$ git grep -Elz 'include *([^ ]*/)?config.mk' | xargs -0 -I '{}' \
sh -c "printf '%s\n' \
\"\$(sed -E 's|^include *(([^ ]*/)?config.mk)|-include \1|' '{}')\" >'{}'"
Relates to netblue30#5140.
kmk3
added a commit
to kmk3/firejail
that referenced
this pull request
Jun 25, 2022
With the previous commit ("makefiles: stop failing when config.mk does
not exist", 2022-06-23), make will not immediately fail when trying to
build a target without having the proper compile-time flags (which are
defined on common.mk).
For example, when running the command below:
make distclean && make
It will throw an error only after (mis-)compiling multiple objects.
So add a dependency on config.mk on every target that uses output
variables (such as @name@ / $(NAME)) on its recipe. And add a
dependency on config.sh on targets that call shell scripts that use
output variables (such as @name@ / $NAME). Also, add a recipe for
config.mk / config.sh telling to run ./configure, to make it a bit more
obvious just in case.
With this commit, make will abort earlier, by detecting that the
config.mk / config.sh dependency does not exist. This happens before
trying to execute the recipe.
This also makes the dependencies more accurate, since if config.mk
(which defines some CFLAGS) is changed, the CFLAGS may also have
changed, so a target that uses CFLAGS should probably be considered out
of date in this case anyway.
Relates to netblue30#5140.
kmk3
added a commit
to kmk3/firejail
that referenced
this pull request
Jun 30, 2022
Configure summary: autoconf essentially only parses configure.ac and generates the configure script (that is, the "./configure" shell script). The latter is what actually checks what is available on the system and internally sets the value of the output variables. It then, for every filename foo in AC_CONFIG_FILES (and for every output variable name BAR in AC_SUBST), reads foo.in, replaces every occurrence of `@BAR@` with the value of the shell variable `$BAR` and generates the file foo from the result. After this, configure is finished and `make` could be executed to start the build. Now that (as of netblue30#5140) all output variables are only defined on config.mk.in and on config.sh.in, there is no need to generate any makefile nor any other mkfile or shell script at configure time. So rename every "Makefile.in" to "Makefile", mkdeb.sh.in to mkdeb.sh, src/common.mk.in to src/common.mk and leave just config.mk and config.sh as the files to be generated at configure time. This allows editing and committing all makefiles directly, without potentially having to run ./configure in between. Commands used to rename the makefiles: $ git ls-files -z -- '*Makefile.in' | xargs -0 -I '{}' sh -c \ "git mv '{}' \"\$(dirname '{}')/Makefile\"" Additionally, from my (rudimentary) testing, this commit reduces the time it takes to run ./configure by about 20~25% compared to commit 72ece92 ("Transmission fixes: drop private-lib (netblue30#5213)", 2022-06-22). Environment: dash 0.5.11.5-1, gcc 12.1.0-2, Artix Linux, ext4 on an HDD. Commands used for benchmarking each commit: $ : >time_configure && ./configure && make distclean && for i in $(seq 1 10); do { time -p ./configure; } 2>>time_configure; done $ grep real time_configure | awk '{ total += $2 } END { print total/NR }'
kmk3
added a commit
to kmk3/firejail
that referenced
this pull request
Jun 30, 2022
This allows running `make clean` and `make distclean` (and possibly
others) without having to run ./configure beforehand.
Note that some packaging-related targets still depend on the existence
of generated files. For example:
* dist: config.mk
* deb: config.sh
Commands used to search and replace:
$ git grep -Elz 'include *([^ ]*/)?config.mk' | xargs -0 -I '{}' \
sh -c "printf '%s\n' \
\"\$(sed -E 's|^include *(([^ ]*/)?config.mk)|-include \1|' '{}')\" >'{}'"
Relates to netblue30#5140.
kmk3
added a commit
to kmk3/firejail
that referenced
this pull request
Jun 30, 2022
With the previous commit ("makefiles: stop failing when config.mk does
not exist", 2022-06-23), make will not immediately fail when trying to
build a target without having the proper compile-time flags (which are
defined on common.mk).
For example, when running the command below:
make distclean && make
It will throw an error only after (mis-)compiling multiple objects.
So add a dependency on config.mk on every target that uses output
variables (such as @name@ / $(NAME)) on its recipe. And add a
dependency on config.sh on targets that call shell scripts that use
output variables (such as @name@ / $NAME). Also, add a recipe for
config.mk / config.sh telling to run ./configure, to make it a bit more
obvious just in case.
With this commit, make will abort earlier, by detecting that the
config.mk / config.sh dependency does not exist. This happens before
trying to execute the recipe.
This also makes the dependencies more accurate, since if config.mk
(which defines some CFLAGS) is changed, the CFLAGS may also have
changed, so a target that uses CFLAGS should probably be considered out
of date in this case anyway.
Relates to netblue30#5140.
kmk3
added a commit
to kmk3/firejail
that referenced
this pull request
Jul 12, 2022
The following leverages the fact that when using a normal merge (as
opposed to "rebase and merge" or "squash and merge") on GitHub, the pull
request number is put in the commit message title and the title of the
PR is added to the commit message body.
Commands used to find and print the items for the RELNOTES:
$ git log --grep='^build:' --merges --reverse --pretty='%s %b' 0.9.70.. |
sed -E -n 's/Merge pull request (#[0-9]+) from [^ ]+ (.*)/ * \2 (\1)/p'
* build: deduplicate configure-time vars into new config files (netblue30#5140)
* build: fix file mode of shell scripts (644 -> 755) (netblue30#5206)
* build: reduce autoconf input files from 32 to 2 (netblue30#5219)
Commands used to generate the message below:
$ git log --grep='^build:' --merges --reverse --pretty='%s %b' 0.9.70.. |
sed -E -n 's/Merge pull request (#[0-9]+).*/\1/p' | sort | tr '\n' ' ' |
sed -E 's/^(.*) /Relates to \1./'
Relates to netblue30#5140 netblue30#5206 netblue30#5219.
Relates to netblue30#5140 netblue30#5206 netblue30#5219.
kmk3
added a commit
that referenced
this pull request
Jul 12, 2022
The following leverages the fact that when using a normal merge (as
opposed to "rebase and merge" or "squash and merge") on GitHub, the pull
request number is put in the commit message title and the title of the
PR is added to the commit message body.
Commands used to find and print the items for the RELNOTES:
$ git log --grep='^build:' --merges --reverse --pretty='%s %b' 0.9.70.. |
sed -E -n 's/Merge pull request (#[0-9]+) from [^ ]+ (.*)/ * \2 (\1)/p'
* build: deduplicate configure-time vars into new config files (#5140)
* build: fix file mode of shell scripts (644 -> 755) (#5206)
* build: reduce autoconf input files from 32 to 2 (#5219)
Commands used to generate the message below:
$ git log --grep='^build:' --merges --reverse --pretty='%s %b' 0.9.70.. |
sed -E -n 's/Merge pull request (#[0-9]+).*/\1/p' | sort | tr '\n' ' ' |
sed -E 's/^(.*) /Relates to \1./'
Relates to #5140 #5206 #5219.
Relates to #5140 #5206 #5219.
kmk3
added a commit
to kmk3/firejail
that referenced
this pull request
Jul 16, 2022
To note on the output files that they are generated and to clarify how they are generated. From the manual of GNU Autoconf (version 2.69): > -- Variable: configure_input > A comment saying that the file was generated automatically by > 'configure' and giving the name of the input file. 'AC_OUTPUT' > adds a comment line containing this variable to the top of every > makefile it creates. For other files, you should reference this > variable in a comment at the top of each input file. For > example, an input shell script should begin like this: > > #!/bin/sh > # @configure_input@ > > The presence of that line also reminds people editing the file > that it needs to be processed by 'configure' in order to be used. Resulting output on config.mk: > # config.mk. Generated from config.mk.in by configure. Relates to netblue30#5140.
kmk3
added a commit
to kmk3/firejail
that referenced
this pull request
Jul 16, 2022
To note on the output files that they are generated and to clarify how
they are generated.
From the manual of GNU Autoconf (version 2.69):
> -- Variable: configure_input
> A comment saying that the file was generated automatically by
> 'configure' and giving the name of the input file. 'AC_OUTPUT'
> adds a comment line containing this variable to the top of every
> makefile it creates. For other files, you should reference this
> variable in a comment at the top of each input file. For
> example, an input shell script should begin like this:
>
> #!/bin/sh
> # @configure_input@
>
> The presence of that line also reminds people editing the file
> that it needs to be processed by 'configure' in order to be used.
Resulting output on config.mk:
# config.mk. Generated from config.mk.in by configure.
Relates to netblue30#5140.
kmk3
added a commit
to kmk3/firejail
that referenced
this pull request
Aug 1, 2022
Output variables in general may contain values with spaces in them. Example: `CC=gcc -foo`. Relates to netblue30#5140.
kmk3
added a commit
to kmk3/firejail
that referenced
this pull request
Aug 1, 2022
Fix the following error and warnings:
$ shellcheck --version | grep ^version:
version: 0.8.0
$ shellcheck config.sh.in
In config.sh.in line 1:
# @configure_input@
^-- SC2148 (error): Tips depend on target shell and yours is unknown. Add a shebang or a 'shell' directive.
In config.sh.in line 3:
NAME=@PACKAGE_NAME@
^--^ SC2034 (warning): NAME appears unused. Verify use (or export if used externally).
In config.sh.in line 4:
VERSION=@PACKAGE_VERSION@
^-----^ SC2034 (warning): VERSION appears unused. Verify use (or export if used externally).
For more information:
https://www.shellcheck.net/wiki/SC2148 -- Tips depend on target shell and y...
https://www.shellcheck.net/wiki/SC2034 -- NAME appears unused. Verify use (...
Relates to netblue30#5140.
ChrysoliteAzalea
pushed a commit
to ChrysoliteAzalea/firejail
that referenced
this pull request
Aug 2, 2022
To note on the output files that they are generated and to clarify how
they are generated.
From the manual of GNU Autoconf (version 2.69):
> -- Variable: configure_input
> A comment saying that the file was generated automatically by
> 'configure' and giving the name of the input file. 'AC_OUTPUT'
> adds a comment line containing this variable to the top of every
> makefile it creates. For other files, you should reference this
> variable in a comment at the top of each input file. For
> example, an input shell script should begin like this:
>
> #!/bin/sh
> # @configure_input@
>
> The presence of that line also reminds people editing the file
> that it needs to be processed by 'configure' in order to be used.
Resulting output on config.mk:
# config.mk. Generated from config.mk.in by configure.
Relates to netblue30#5140.
kmk3
added a commit
to kmk3/firejail
that referenced
this pull request
Jul 24, 2023
Similarly to mkdeb.sh. Relates to netblue30#5140.
kmk3
added a commit
to kmk3/firejail
that referenced
this pull request
Jul 27, 2023
Similarly to mkdeb.sh. Relates to netblue30#5140.
kmk3
added a commit
to kmk3/firejail
that referenced
this pull request
Jul 28, 2023
This removes the need to manually pass variables such as `$(TARNAME)` and `$(VERSION)` to shell scripts in the root Makefile. Relates to netblue30#5140.
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
makefiles: deduplicate configure-time vars into new config.mk.in
Currently, the configure-time variables (that is, the ones that assign
to placeholders, such as "@HAVE_MAN@", which are set/replaced at
configure-time) are defined on multiple files (such as on Makefile.in
and on common.mk.in).
To avoid duplication, centralize these variables on a single file
(config.mk.in) and replace all of the other definitions of them with an
include of config.mk.
mkdeb.sh.in: move configure-time vars into new config.sh.in
For better organization and so that they can be used by other shell
scripts by just sourcing config.sh.
Relates to #646.