Skip to content

Conversation

@cbilz
Copy link
Contributor

@cbilz cbilz commented Feb 6, 2025

Autoconf configuration header templates often use #undef directives as substitution hooks, which we already support via ConfigHeader.Style.autoconf. However, many projects, particularly those using Gnulib, additionally rely on @FOO@-style output variables for header configuration. While some of these cases can be handled using Style.cmake, this fails when the template happens to contain strings like ${FOO}.

For example, the Gnulib header template lib/unistd.in.h uses @FOO@-style substitutions, but also happens to contain the string ${LOGNAME-$USER} in a code comment. This is misinterpreted as a substitution hook by Style.cmake. Attempting to work around this by adding a value like .{ @."LOGNAME-$USER" = ... } to ConfigHeader fails because Style.cmake rejects the invalid $ character in the variable name.

To resolve this issue, this pull request adds a new Style.autoconf_at specifically for @FOO@-style substitutions as used by Autoconf. Additionally, Style.autoconf is renamed to Style.autoconf_undef for clarity. The new Style.autoconf_at is similar to Style.cmake, but does not perform ${} or #cmakedefine substitutions, enabling the correct configuration of headers like the one described above.

cbilz added a commit to cbilz/gnu-m4-zig that referenced this pull request Feb 9, 2025
There were a few configuration value conflicts between `config.h` and
the Gnulib headers, so it made sense to draw their values from different
pools.

The Gnulib headers use `@FOO@`-style substitution hooks. So far, we used
`ConfigHeader.Style.cmake` to configure these, but this fails for
`unistd.h`, see ziglang/zig#22794.

Further, we add a function that allows build-time configuration of
`ConfigHeader` values, see ziglang/zig#22795.
This is more convenient and should improve overall build times as
comptime is still relatively slow.

We use a modified version of `ConfigHeader` until these pull requests
are merged.
Add the new style `Style.autoconf_at`. Rename the existing
`Style.autoconf` (which uses `#undef` directives) to
`Style.autoconf_undef`.
@andrewrk
Copy link
Member

Thanks for the patch. Can you explain why it's a new style and not an enhancement of the already existing autoconf style?

@cbilz
Copy link
Contributor Author

cbilz commented Feb 23, 2025

Can you explain why it's a new style and not an enhancement of the already existing autoconf style?

The user should have control over whether to expand #undef hooks or @FOO@ hooks, because Autoconf-based header templates typically use only one of these two mechanisms in a single file, not both.

If Style.autoconf were to expand both at the same time, it could lead to issues like in the following:

Example 1

A header that uses @FOO@-style hooks might contain #undef directives that are not intended as substitution hooks, but are meant to undefine macros. ConfigHeader would be unable to keep the #undef directives intact. (The value null is rendered as a commented out /* #undef ... */)

Example 2

A header that uses #undef-style hooks might contain a code comment (or string) like

// Please contact john@email.com or doe@email.com.

ConfigHeader would misinterpret @email.com or doe@ as a substitution hook with invalid characters (period and spaces) and would refuse to configure the header.

Alternative interface

An alternative to adding separate Style.autoconf_at and Style.autoconf_undef would be to add an option to the existing Style.autoconf, like this:

pub const Style = union(enum) {
    autoconf: struct {
        file: std.Build.LazyPath,
        mode: enum { at, undef },
    },
    //...
};

Perhaps .mode could default to .undef because it is more common, and to make it slightly easier to adapt existing build scripts that use Style.autoconf.

Let me know if I this looks better, and I can change it.

@andrewrk
Copy link
Member

Understood thank you. I did not know autoconf itself had two different styles to choose from. In this case, happy to accept your original patchset.

@andrewrk andrewrk merged commit 86f3547 into ziglang:master Feb 23, 2025
9 checks passed
@andrewrk
Copy link
Member

FYI e47f340

@cbilz cbilz deleted the autoconf_at branch February 23, 2025 22:43
cbilz added a commit to cbilz/gnu-m4-zig that referenced this pull request Feb 26, 2025
cbilz added a commit to cbilz/gnu-m4-zig that referenced this pull request Feb 26, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants