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

Use different target-dir for each package or enable workspace target-dir #309

Closed
kanru opened this issue Jan 23, 2023 · 8 comments
Closed

Comments

@kanru
Copy link
Contributor

kanru commented Jan 23, 2023

Currently all the cargo-build targets use the same cargo_target_dir "${CMAKE_BINARY_DIR}/${build_dir}/cargo/build"

In my case a workspace with multiple packages, this always causes rebuilding each package for each build.

Ideally packages in the same workspace should be able to share artifacts but I'm not sure if it's easy to do. At least if we change the cargo_target_dir to something like "${CMAKE_BINARY_DIR}/${build_dir}/cargo/${package_name}/target" then each package can reuse their artifact at the cost of building each dependency once.

@jschwe
Copy link
Collaborator

jschwe commented Jan 23, 2023

In my case a workspace with multiple packages, this always causes rebuilding each package for each build.

Are you perhaps setting different rustflags for each package (e.g. via corrosion_add_target_rustflags())? If possible consider using corrosion_add_target_local_rustflags which only changes the rustflags for the main crate, but not dependencies.

If you are not setting any rustflags I would need some more information to be able to reproduce your problem. At work we also use a large workspace and are not experiencing this problem with corrosion.

@kanru
Copy link
Contributor Author

kanru commented Jan 23, 2023

Thanks for reply! I don't have a minimum reproducible repository yet, the repository I'm working on is this branch https://github.com/chewing/libchewing/tree/rust

I'm not setting any rustflags. The structure of my workspace is like this

libchewing [root package]
|- src [both c and rust files]
|- capi/chewing-internal [staticlib package, depends on the root package]
`- tools [bin package]

The final target libchewing.so is chewing-internal linked with some C object files. And the tools package is used to build static data files.

For each build, the tools and the chewing-internal package will clobber each others' build dir and cause rebuild.

@jschwe
Copy link
Collaborator

jschwe commented Jan 23, 2023

I checked out the rust branch of your project, but can't reproduce. The rust libraries compile fine and don't recompile when retrying, however the C code fails to compile due to some warnings. I tried configuring cmake like this:

 cmake -S. -Bbuild -GNinja -DMAKEINFO=/opt/homebrew/bin/makeinfo --compile-no-warning-as-error
cmake --build build

@kanru
Copy link
Contributor Author

kanru commented Jan 24, 2023

I created this minimum reproducer https://github.com/kanru/corrosion-issue-309

It looks like it's caused by the lib member and the bin member both depending on the root package. If I remove the dependency from the bin member then I can no longer reproduce.

Test setup

git clone https://github.com/kanru/corrosion-issue-309

cd corrosion-issue-309
mkdir build
cmake -G ninja ..
ninja

Expected: run second ninja should not rebuild everything
Actual: each ninja run will always rebuild both the lib and bin

@jschwe
Copy link
Collaborator

jschwe commented Jan 24, 2023

Thanks for the minimal example! This is a regression most likely introduced by commit fa4e241 -CARGO_TARGET_<xxx>_LINKER seems to cause the recompiles if it differs.
Since you are not using C++ (as far as I can tell), you can fix this for the moment by simply adding the option NO_LINKER_OVERRIDE when calling corrosion_import_crate()

@kanru
Copy link
Contributor Author

kanru commented Jan 24, 2023

Thanks. Related PR: #270 #272

I can confirm that adding NO_LINKER_OVERRIDE fixes the issue for now. I do plan to build the library on Windows and eventually let Rust produce the cdylib, not sure if this will cause issues.

Btw, I notice the doc for NO_LINKER_OVERRIDE and PROFILE option is reversed in the README.md file.

@jschwe
Copy link
Collaborator

jschwe commented Jan 25, 2023

I do plan to build the library on Windows and eventually let Rust produce the cdylib, not sure if this will cause issues.

Windows and cdylib or not should not really affect this issue. It only matters if you link C++ code into your Rust executable. In the medium term I hope to resolve this with #310.

@jschwe
Copy link
Collaborator

jschwe commented Feb 18, 2023

Closing - The immediate issue was already solved. Tracking the possibility of overrding the target-dir per cmake target in issue #328

@jschwe jschwe closed this as completed Feb 18, 2023
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