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

Add support for adding link dependencies via --cxx-link. #1244

Merged
merged 1 commit into from
Jul 12, 2022

Conversation

bbannier
Copy link
Contributor

@bbannier bbannier commented Jul 7, 2022

We already supported specifying additional include directories, but
injecting additional link dependencies into the link always required
manually compiling and linking files. So while users could already
declare e.g., existing functions with &cxxname, we provided no tooling
to use such functions if we were defined in external libraries or static
archives.

This patch adds support for that. One can now reference static archives
or shared libraries to include in the link by passing them with
--cxx-link <lib>. This will copy symbols for static archives or
introduce a shared library dependency into the created HLTO file and
needs only to be specified for the step creating the HLTO file.

Closes #1236.

@bbannier bbannier self-assigned this Jul 7, 2022
@bbannier
Copy link
Contributor Author

bbannier commented Jul 7, 2022

@rsmmr, I still need to figure out how to make the test case work on freebsd13, but I think the idea for the user API is where I want it to be. Could you take a look?

@rsmmr
Copy link
Member

rsmmr commented Jul 8, 2022

I think my main question is if an environment variable is the right mechanism here. On the plus side, it matches what we do for include paths. On the other hand, it seems unusual to pass specific libraries/targets through an environment variables (whereas it's common for a search path like the include dirs). The alternative would be a command line option (and we could add one for include directories, too, then on top of the env var).

@bbannier
Copy link
Contributor Author

I think my main question is if an environment variable is the right mechanism here. On the plus side, it matches what we do for include paths. On the other hand, it seems unusual to pass specific libraries/targets through an environment variables (whereas it's common for a search path like the include dirs). The alternative would be a command line option (and we could add one for include directories, too, then on top of the env var).

I think we want to support cases where Spicy analyzers use e.g., the CMake macro spicy_add_analyzer which does not invoke spicyz directly (this is e.g., used in the package template). If we add a command line argument for libs we sjould add a new LIBS arg to that macro as well. Let me check on that.

@bbannier bbannier changed the title Add support for adding link dependencies via HILTI_CXX_LINK_LIBS. Add support for adding link dependencies via --cxx-link. Jul 11, 2022
@bbannier
Copy link
Contributor Author

bbannier commented Jul 11, 2022

Updated this as discussed offline. Libs can now be passed with the flag --cxx-link (potentially used multiple times).

I also tried adding the global way to pass arbitrary command line args to our drivers that you mentioned (e.g., HILTI_OPTIONS="-c --cxx-link foo.so" would set the two command line options). This will be rather tricky to implement and use since we somehow would need to reconstruct int argc and char** argv from that which becomes hard when arguments need escaping e.g., due to embedded whitespace. I'd suggest we do not attempt this, but instead add a new named argument to spicy_add_analyzer over in zeek/spicy-plugin as a follow-up.

rsmmr
rsmmr previously approved these changes Jul 12, 2022
hilti/toolchain/include/compiler/context.h Outdated Show resolved Hide resolved
hilti/toolchain/src/compiler/jit.cc Outdated Show resolved Hide resolved
hilti/toolchain/src/compiler/driver.cc Outdated Show resolved Hide resolved
We already supported specifying additional include directories, but
injecting additional link dependencies into the link always required
manually compiling and linking files. So while users could already
declare e.g., existing functions with `&cxxname`, we provided no tooling
to use such functions if we were defined in external libraries or static
archives.

This patch adds support for that. One can now reference static archives
or shared libraries to include in the link by passing them with
`--cxx-link <lib>`. This will copy symbols for static archives or
introduce a shared library dependency into the created HLTO file and
needs only to be specified for the step creating the HLTO file.

Closes #1236.
@bbannier bbannier merged commit 079864d into main Jul 12, 2022
@bbannier bbannier deleted the topic/bbannier/issue-1236 branch July 12, 2022 08:54
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.

Make it possible to specify link dependencies when building HLTO files
2 participants