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

Review compilation-related env vars in Spicy feature #44

Open
J-Gras opened this issue Jan 13, 2025 · 4 comments
Open

Review compilation-related env vars in Spicy feature #44

J-Gras opened this issue Jan 13, 2025 · 4 comments

Comments

@J-Gras
Copy link
Contributor

J-Gras commented Jan 13, 2025

For Spicy packages, the template manually resets some environment variables in btest.cfg:

# Manually merge Spicy analyzer-specific changes to `testing/btest.cfg`.
with open(pkg_file("testing", "btest.cfg"), "ab") as btest_cfg:
btest_cfg.write(
bytes(
textwrap.dedent(
"""\
DIST=%(testbase)s/..
# Set compilation-related variables to well-defined state.
CC=
CXX=
CFLAGS=
CPPFLAGS=
CXXFLAGS=
LDFLAGS=
DYLDFLAGS=
"""
),
"ascii",
)
)

Would be nice to review whether it's worth to keep this and if so, if it makes sense to add this to plugins in general.

@bbannier
Copy link
Member

The general idea with suppressing compiler- and linker-related env vars is that users might set these to undesirable values which could have an effect on tests compiling C++ files (e.g., tests of plugin installation picking them up). I do not recall a reason why they would be relevant for Spicy since we use namespaced variables like HILTI_CXX (the variables are documented in https://docs.zeek.org/projects/spicy/en/latest/toolchain.html). The only needed variable is DIST which is used in a generated test.

At the very least we should probably move these into the Plugin feature if we do not want to drop them completely.

@bbannier bbannier changed the title Review spicy-specific env vars Review compilation-related env vars in Spicy feature Jan 13, 2025
@J-Gras
Copy link
Contributor Author

J-Gras commented Feb 3, 2025

I think there is another gotcha: It looks like the env vars are just appended to the file. I've a config that uses multiple alternatives. Hence, the new vars are only added to the last alternative.

@bbannier
Copy link
Member

bbannier commented Feb 3, 2025

I think there is another gotcha: It looks like the env vars are just appended to the file. I've a config that uses multiple alternatives. Hence, the new vars are only added to the last alternative.

Is that a use case that comes up when combining features this template ships, or something downstream from here? The current "templating" mechanism here is not very smart (e.g., it does not really know the full contents of the file it modifies) but we took some care that certain features here combine compose reasonably, but this is likely impossible in general.

@J-Gras
Copy link
Contributor Author

J-Gras commented Feb 3, 2025

Is that a use case that comes up when combining features this template ships, or something downstream from here?

This came up when combining a custom feature that configures multiple alternatives in btest.cfg and the Spicy feature.

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

No branches or pull requests

2 participants