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

Parse the native staticlib dependencies output from rustc #335

Open
jschwe opened this issue Feb 28, 2023 · 8 comments · May be fixed by #472
Open

Parse the native staticlib dependencies output from rustc #335

jschwe opened this issue Feb 28, 2023 · 8 comments · May be fixed by #472

Comments

@jschwe
Copy link
Collaborator

jschwe commented Feb 28, 2023

Currently, corrosion hardcodes the libraries required by Rust static libs (OS-dependent).
This list may not be always up-to-date and cover all required libraries, since the exact list may change between different rust versions, and additional libraries may be added by #[link] attributes in user code or from build scripts!
Getting this information requires actually building the crate (with --print=native-static-libs) and all its dependencies.

Probably the only way to pass these dependencies from the rustc output at build-time to the CMake linker command line would be by saving the required arguments to a response file, which clang/gcc support loading via @file. I think MSVC also has such an option.

@jschwe
Copy link
Collaborator Author

jschwe commented Apr 16, 2023

An intermediate solution was merged in #362, which parses the required libraries at configure time from a dummy demo project for the target arch.

@ogoffart
Copy link
Collaborator

The other thing i'm trying to do is to link a staticlib that itself uses some shared library.

So the build.rs of a dependency might do

    println!("cargo:rustc-link-search={foolib_path}");
    println!("cargo:rustc-link-lib=foo");

And that information is also conveyed by --print=native-static-libs

I've tried to add the --print=native-static-libs as an arg to rustc when building the crate and that works. But I wonder how to actually get this information in the INTERFACE_LINK_LIBRARIES of the target.
Since we use USES_TERMINAL it seems we have no way to parse the stderr of the cargo rustc invokation. And we want to keep using USES_TERMINAL.
Also how would we feed that into INTERFACE_LINK_LIBRARIES ?

@jschwe
Copy link
Collaborator Author

jschwe commented Apr 19, 2023

But I wonder how to actually get this information in the INTERFACE_LINK_LIBRARIES of the target.

We don't, since we get that information only at build time. CMake itself does not support dynamic dependencies.

What we could however do, is the following:

  1. Instead of calling cargo rustc directly, we call a CMake script (cmake -P some_script.cmake), which calls cargo rustc via execute_process, parse the output and write the result to a predetermined file.
  2. CMake adds a linker argument, which adds @file to the link line (so called response file), so that clang / gcc read the file containing the additional link arguments.

@ogoffart
Copy link
Collaborator

which calls cargo rustc via execute_process, parse the output

Does that mean we wouldn't see the progress output anymore in the console. I mean stuff like

   Compiling syn v2.0.13
   Compiling arrayvec v0.7.2
   Compiling hashbrown v0.12.3
   Building [==>                       ] 61/427: libc, syn, libm, miniz_oxide, aho-corasick, num-traits(build)  

Is there a way to use tee or something like

CMake adds a linker argument, which adds @file to the link line

Ah, clever!
But where would that file be, and will it be installed if the library is installed?

@jschwe
Copy link
Collaborator Author

jschwe commented Apr 20, 2023

Is there a way to use tee or something like

CMake >= 3.18 adds ECHO_OUTPUT_VARIABLE and ECHO_ERROR_VARIABLE which would allow us to see the output. For versions before that there is no cross-platform way.
Personally I've been thinking about releasing v0.4 as a sort of LTS version to support ubuntu 20.04 LTS, more or less at the state of the current master, and then releasing a v0.5 non-LTS version which is more liberal in bumping the minimum supported CMake / Rust versions, when it allows improving corrosion.

But where would that file be,

The file path would need to be a parameter passed from corrosion to the CMake script we invoke. It would be a path in build directory, unique per target.

and will it be installed if the library is installed?

I hadn't thought about that yet! I haven't really looked at the install step of corrosion yet, but since there is #64, I'm guessing install for libraries is not implemented by corrosion at all yet, and left up to the user for now.
I'm not all that familiar with CMakes install procedure, but if I remember correctly when installing libraries, you usually also install some CMake file along with it, containing stuff like INTERFACE_LINK_LIBRARIES. Ideally, we could configure this file during the install step to contain the correct libraries in INTERFACE_LINK_LIBRARIES, based on the contents of @file or a seconday intermediate file generated during the build step.

@Be-ing
Copy link

Be-ing commented May 8, 2023

I could work on implementing this.

CMake >= 3.18 adds ECHO_OUTPUT_VARIABLE and ECHO_ERROR_VARIABLE which would allow us to see the output. For versions before that there is no cross-platform way.
Personally I've been thinking about releasing v0.4 as a sort of LTS version to support ubuntu 20.04 LTS, more or less at the state of the current master, and then releasing a v0.5 non-LTS version which is more liberal in bumping the minimum supported CMake / Rust versions, when it allows improving corrosion.

Any update on these release plans 3 weeks later? I am surprised you are still supporting more than the most recent Ubuntu LTS. I suppose ECHO_ERROR_VARIABLE could be omitted for CMake < 3.18 as a fallback, which of course wouldn't be a great user experience.

@jschwe
Copy link
Collaborator Author

jschwe commented May 9, 2023

I could work on implementing this.

That would be a great help! I probably won’t have any time this month to work on this.
I previously started the implementation, you can take a look here: master...jschwe:corrosion:dynamic_link_libs

Any update on these release plans 3 weeks later?

I'm planning on releasing current master as v0.4-alpha1 (this week hopefully), hope that people will test and then release it as v0.4 (approx. +2 weeks after alpha1).
I'm not sure yet what to do about the cbindgen / cxxbridge integration. If I don't get positive feedback I'll probably leave it in as an experimental option only and delay stabilization until later.
I'd probably release v0.5 shortly after v0.4, with increased minimum CMake / Rust versions.

I am surprised you are still supporting more than the most recent Ubuntu LTS.

There are still a surprising amount of people on "old" CMake / Ubuntu versions. I'd like to keep the hurdle of incrementally adopting Rust low and that should mean Corrosion doesn't require upgrading CMake / ubuntu.
But it has become clear, that this restriction is limiting corrosion in some aspects, which is motivating me to split corrosion into a v0.4 LTS support version, which is basically done and will only receive fixes from me.
Development can then continue on v0.5, using new rustc features (e.g. --crate-type ) and increasing the minimum supported CMake version to 3.22 (ubuntu 22.04)

@ratmice
Copy link
Contributor

ratmice commented May 9, 2023

I am surprised you are still supporting more than the most recent Ubuntu LTS.

There are still a surprising amount of people on "old" CMake / Ubuntu versions. I'd like to keep the hurdle of incrementally adopting Rust low and that should mean Corrosion doesn't require upgrading CMake / ubuntu.

Thank you for supporting these, I have a corrosion using project which is still stuck supporting "old" (but at this time still supported) LTS versions, which is counting the days until these sunset. But until then, I have very much appreciated that support for these older builds has been kept intact.

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 a pull request may close this issue.

4 participants