Configure improvements2 #4712
Merged
Configure improvements2 #4712
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
For increased consistency and readability. This restores the spaces removed on commit bf81cd6 ("configure.ac: run autoupdate to fix autoconf warning") / PR netblue30#4316. Command used to check for the lack of whitespace: grep ',[^ ]' configure.ac
Command used to find them:
grep ' "$' configure.ac
Square brackets are used as quotes in autoconf. From Section 8.1.1, Active Characters of the Autoconf manual[1]: > To fully understand where proper quotation is important, you first > need to know what the special characters are in Autoconf: ‘#’ > introduces a comment inside which no macro expansion is performed, ‘,’ > separates arguments, ‘[’ and ‘]’ are the quotes themselves, ‘(’ and > ‘)’ (which M4 tries to match by pairs), and finally ‘$’ inside a macro > definition. [1] https://www.gnu.org/savannah-checkouts/gnu/autoconf/manual/autoconf-2.70/autoconf.html#Active-Characters
kmk3
requested review from
reinerh,
netblue30,
topimiettinen,
startx2017 and
rusty-snake
topimiettinen
approved these changes
Nov 24, 2021
For increased safety and consistency. In addition, this should make it clearer where each argument starts and ends. See also the following item from autoconf NEWS[1]: > * Noteworthy changes in release 2.70 (2020-12-08) [stable] [...] > *** Many macros have become pickier about argument quotation. > > If you get a shell syntax error from your generated configure > script, or seemingly impossible misbehavior (e.g. entire blocks of > the configure script not getting executed), check first that all > macro arguments are properly quoted. The “M4 Quotation” section of > the manual explains how to quote macro arguments properly. > > It is unfortunately not possible for autoupdate to correct > quotation errors. [1] https://git.savannah.gnu.org/gitweb/?p=autoconf.git;a=blob;f=NEWS;h=ba418d1af5da752de77a2c388f9af56f8f1bf6a4;hb=97fbc5c184acc6fa591ad094eae86917f03459fa
These macros should always be called regardless of the intended value of each variable, as even if e.g.: no --enable-apparmor flag is given, the configure script still has to substitute `@HAVE_APPARMOR@` with blank in the relevant files. Something similar is already being done for HAVE_OVERLAYFS since commit fb9f2a5 ("disabled overlayfs, fixes pending; added video channels to README* files", 2021-02-06). Note that each AC_SUBST is not immediately converted into search/replace code when generating the configure script. It appears that the variables are handled only after parsing all of configure.ac (or until a specific command is found), as all arguments passed to every AC_SUBST call are defined at once on the `ac_subst_vars` list. The actual substitutions are also done all at once (while iterating through the list) and that happens much later in the script (see both occurrences of `ac_subs_vars` on the current script).
Added on commit d1acb31 ("compile time: enable LTS", 2021-02-28). It only needs to be called once for each variable. See the configure script diff and the previous commit ("configure*: Move AC_SUBST calls to more obvious places").
See commit 15d7938 ("Try to fix netblue30#2310 -- Can't create run directory without suid-root", 2021-05-13) / PR netblue30#4273. It is the only "HAVE_" option whose value is set by if/else on a makefile. Also, it is set in different places to either "yes", "no", blank or "-DHAVE_SUID". Set the value only on configure.ac and only to either blank or to "-DHAVE_SUID". Misc: The `ifeq ($(HAVE_SUID),-DHAVE_SUID)` comparison that this adds is based on the existing `ifeq ($(HAVE_APPARMOR),-DHAVE_APPARMOR)` comparison on Makefile.in.
Added on commit 8d8686a ("Make installation of contrib scripts configurable", 2017-04-13). Remove redundant argument to AS_IF and make it look more like the other nearby AS_IF calls.
kmk3
force-pushed
the
configure-improvements2
branch
from
b500cef
to
8344145
Compare
|
Force-pushed to add a missing quote. |
reinerh
approved these changes
Nov 27, 2021
|
all in, thanks! |
kmk3
added a commit
to kmk3/firejail
that referenced
this pull request
Nov 28, 2022
Misc: I missed this one on commit 221c101 ("configure*: Add missing quotes to arguments", 2021-06-03) / PR netblue30#4712.
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.
This is mostly linting/cleanup. It undoes a few quirks to make configure.ac a
bit more consistent and easier to understand. See the commit messages for
details.
This is a continuation of #4316 and #4695.
Kind of relates to #4642.
Misc: I have a few more WIP configure-related changes (and some make-related
ones), but I need to test some of them and this branch is big enough.