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

Support Multiple targets per package #216

Merged
merged 10 commits into from
Sep 16, 2022
Merged

Conversation

jschwe
Copy link
Collaborator

@jschwe jschwe commented Sep 7, 2022

Blocked on #213 and #215

Each Cargo.toml package can contain multiple crates, e.g. a library
crate or 1 or more bin crates. Previously Corrosion only supported a
single target (crate) per package. This meant that if both a library and
a binary crate where present, the binary crate would silently be
ignored.
Corrosion now adds a CMake target for each target (bin or library) in a package.

Currently the target name is not namespaced at all for compatiblity reasons.
In the future we may want to expose targets as package_name::target_name. This is already tracked by issue #58.

@jschwe jschwe force-pushed the multibin branch 8 times, most recently from a304e4d to f85157a Compare September 12, 2022 14:40
Each Cargo.toml package can contain multiple crates, e.g. a library
crate or 1 or more bin crates. Previously Corrosion only supported a
single target (crate) per package. This meant that if both a library and
a binary crate where present, the binary crate would silently be
ignored.
Corrosion now adds a CMake target for each target in a package.
Adds a simple test project for the newly added
support for packages with multiple targets.
Creating the folders is (no longer) necessary. Presumably this was needed
before for the creation of the generated .cargo/config file, which we no
longer use.
There is also no point in passing `CMAKE_BUILD_TYPE` ,
`CMAKE_CONFIGURATION_TYPES` and `CMAKE_VS_PLATFORM_NAME`
as function parameters in CMake code.
This is probably a legacy artifact from translating the Rust Generator
to CMake.
The same in principle also applies to CARGO_VERSION and TARGET, however
in the future me way want to make these options per specific per
invocation of corrosion_import_crate, so keep these options for now

Also remove some unneeded source file properties usage and add useful
debug print
Which system libraries are required to link depends on the rust version,
since this depends on the version of the std library.
The cargo version may be different from the Rust version in some cases,
e.g. when compiling a Rust toolchain from source, but without compiling
cargo.
@jschwe jschwe marked this pull request as ready for review September 13, 2022 06:55
@jschwe jschwe changed the title Draft: Support Multiple targets per package Support Multiple targets per package Sep 13, 2022
jschwe and others added 6 commits September 14, 2022 16:50
This makes future adjustments easier, since the `os` variable  will
now actually always contain the OS.
The target triple is required to determine which system libraries must
be linked with the Rust code.
These variables are not documented and I'm not sure if they even exist.
Instead, the correct thing is to use a genex when setting
`INTERFACE_LINK_LIBRARIES`.
After previous refactoring a lot of the code in CorrosionGenerator
does not rely on newer CMake version and could
in principle be used by the Rust CMake generator,
simplifying code there and reducing code duplication.

This commit as a first step moves the sharable functions to Corrosion.cmake
After the previous refactoring of Corrosion CMake code, we can now reuse
a significant portion of logic and greatly simplify
the Rust application that generates some CMake code.

We still need it for legacy CMake versions to parse JSON, but a lot of
functionality was already implemented in CMake.

This commit removes this code duplication.
Signed-off-by: Jonathan Schwender <schwenderjonathan@gmail.com>
Signed-off-by: Jonathan Schwender <schwenderjonathan@gmail.com>
@jschwe jschwe force-pushed the multibin branch 2 times, most recently from a1a0be8 to dc015e4 Compare September 14, 2022 14:57
@jschwe jschwe merged commit 0eb1eb8 into corrosion-rs:master Sep 16, 2022
@jschwe jschwe deleted the multibin branch September 18, 2022 10:37
@jschwe jschwe added this to the v0.3 milestone Sep 18, 2022
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.

None yet

1 participant