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

Corrosion is incompatible with Ninja Multi-Config #50

Closed
humenda opened this issue Sep 10, 2020 · 12 comments · Fixed by #137
Closed

Corrosion is incompatible with Ninja Multi-Config #50

humenda opened this issue Sep 10, 2020 · 12 comments · Fixed by #137
Labels
blocked Issues that are blocked on external dependencies (e.g. cargo or cmake) cmake-3.20

Comments

@humenda
Copy link

humenda commented Sep 10, 2020

When I try to build my project with Corrosion and the Ninja Multi-Config generator, I get a mesage like this:

$ cd build && cmake Ninja -G "Multi-Config" ..
[…]
-- Using Corrosion as a subdirectory
-- Configuring done
-- Generating done
-- Build files have been written to: /some/path/build

$ cmake --build . --
ninja: error: 'Debug/cargo/build/x86_64-unknown-linux-gnu/debug/libfoo.a', needed by 'src/bar/Debug/bar', missing and no known rule to make it

If I use -G Ninja, it works. I had this issue with single-conf vs. multi-conf. in my project as well. It usually boils doin to a few generator expressions at places where paths are constructed.

Thanks :-)

@AndrewGaspar
Copy link
Collaborator

I went to look into this and remembered that I had run into this before... The issue that makes this "hard" ultimately comes down to the problem that you can't use generator expressions in the BYPRODUCTS part of add_custom_target. Why is that important? Ninja must know which commands produce which outputs. This is fine for single config Ninja, but for multi-config, the "debug" and "release" configurations will put their files in different subdirectories of their build directories - x86_64-unknown-linux-gnu/debug vs. x86_64-unknown-linux-gnu/release.

I initially thought I'd make the "debug" and "release" builds separate custom targets. However, that doesn't work either because once you go to generate the imported targets (e.g. foo, in your case), you can't use generator expressions in the add_dependencies call either - so no add_dependencies(foo $<IF:$<CONFIG:Debug>,cargo-build-debug_foo,cargo-build-release_foo>).

An "easy" solution is to instead use the --out-dir flag to cargo to place debug and release builds in the same relative path. Unfortunately, it's not stable yet, so it would require a nightly rust toolchain: rust-lang/cargo#6790

A harder solution would be to instead copy all the outputs to a different location, which honestly might be a good idea anyway - they should probably go directly in the binary dir. Having to go digging for libs and executables in the cargo output directory is annoying, and is honestly not in the spirit of Corrosion.

@humenda
Copy link
Author

humenda commented Sep 11, 2020 via email

@AndrewGaspar
Copy link
Collaborator

OK, so I tried to implement the copying method and discovered that it would not solve the issue of not allowing generator expressions in BYPRODUCTS. I had incorrectly assumed relative paths to BYPRODUCTS would be relative to ${CMAKE_CURRENT_BINARY_DIR}/$<CONFIG>, but it is not... it's relative to ${CMAKE_CURRENT_BINARY_DIR}. Because of that, I think this is definitely blocked on the CMake issue I described.

I'll make a note on the CMake issue.

In the mean time, I'm going to rename this issue - this doesn't affect all multi-config generators, just Ninja multi-config.

@AndrewGaspar AndrewGaspar changed the title Fix failure for multi-config generators Corrosion is incompatible with Ninja Multi-Config Sep 13, 2020
@AndrewGaspar AndrewGaspar added the blocked Issues that are blocked on external dependencies (e.g. cargo or cmake) label Sep 13, 2020
AndrewGaspar added a commit that referenced this issue Sep 13, 2020
…hem in byproducts

Does not work with any existing version of CMake

Related to #50
@AndrewGaspar
Copy link
Collaborator

Looks like this will be supported in CMake 3.20: https://gitlab.kitware.com/cmake/cmake/-/merge_requests/5402

@humenda
Copy link
Author

humenda commented Dec 5, 2020 via email

@ratijas
Copy link
Contributor

ratijas commented Apr 9, 2021

CMake 3.20.0 was released few weeks ago. If it indeed fixed the issue, then the Corrosion's generator check regarding Ninja Multi-Config could be extended to check for compatible CMake version.

https://github.com/AndrewGaspar/corrosion/blob/4a1dbd27ee693b3ee093b7dc601a2019f728d5e8/cmake/Corrosion.cmake#L3-L7

@SchrodingerZhu
Copy link

How is this going now? on 3.22 it seems that Ninja and Ninja Multi-Config are both broken

@ogoffart
Copy link
Collaborator

Could you specify what you mean by "broken" ?

@tronical
Copy link
Contributor

How is this going now? on 3.22 it seems that Ninja and Ninja Multi-Config are both broken

I'm using 3.22 with a corrosion based project and ninja 1.10.0 and it seems fine. What errors are you encountering?

@SchrodingerZhu
Copy link

it was showing the exact error as above. I will upload some logs later

1 similar comment
@SchrodingerZhu
Copy link

it was showing the exact error as above. I will upload some logs later

@tronical
Copy link
Contributor

Ahh, great - yes please :). Please also include the command line you're using to invoke cmake. Do you have a CMAKE_GENERATOR environment variable set, by chance?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
blocked Issues that are blocked on external dependencies (e.g. cargo or cmake) cmake-3.20
Projects
None yet
Development

Successfully merging a pull request may close this issue.

6 participants