eclipse-zenoh
Fix installation when ZENOHC_CUSTOM_TARGET is not set and CARGO_BUILD_TARGET env variable is set
In the case ZENOHC_CUSTOM_TARGET is not set, no --target option is passed to cargo build, but a given target can be set with the CARGO_BUILD_TARGET env variable. In that case the locatio of the library depends on the specified CARGO_BUILD_TARGET env variable. This PR adds a logic branch to handle this case, and avoid the error:
CMake Error at install/cmake_install.cmake:51 (file):
file INSTALL cannot find
"/home/conda/staged-recipes/build_artifacts/libzenohc_1719160267267/work/target/release/libzenohc.so":
No such file or directory.
Call Stack (most recent call first):
cmake_install.cmake:47 (include)
if CARGO_BUILD_TARGET env variable is set.
@traversaro Does it really makes sense to account for cargo env variable when building a technically c-package? I understand that we heavily use cargo in build process, and that this env variable indeed might be set, maybe it is better to produce a error message if we detect that CARGO_BUILD_TARGET is set and ZENOHC_CUSTOM_TARGET is not (or different from ZENOHC_CUSTOM_TARGET) ? Or alternatively force-set ZENOHC_CUSTOM_TARGET to the same value as CARGO_BUILD_TARGET (and produce a diagnostic message) ?
@traversaro Does it really makes sense to account for cargo env variable when building a technically c-package? I understand that we heavily use cargo in build process, and that this env variable indeed might be set, maybe it is better to produce a error message if we detect that CARGO_BUILD_TARGET is set and ZENOHC_CUSTOM_TARGET is not (or different from ZENOHC_CUSTOM_TARGET) ? Or alternatively force-set ZENOHC_CUSTOM_TARGET to the same value as CARGO_BUILD_TARGET (and produce a diagnostic message) ?
Just to give you a bit more context on this change, I encountered this when building a conda-forge package for zenoh-c in https://github.com/conda-forge/staged-recipes/pull/26736 . In there, the CARGO_BUILD_TARGET is set automatically when building rust packages by the build environment, so I encounted this problem. I could have solved by just setting ZENOHC_CUSTOM_TARGET equal to the CARGO_BUILD_TARGET env variable, but I thought other could experience my problem eventually.
maybe it is better to produce a error message if we detect that CARGO_BUILD_TARGET is set and ZENOHC_CUSTOM_TARGET is not (or different from ZENOHC_CUSTOM_TARGET) ?
Yes, that can be done, if you like I can implement it.
Or alternatively force-set ZENOHC_CUSTOM_TARGET to the same value as CARGO_BUILD_TARGET (and produce a diagnostic message) ?
I would avoid that. diagnostic messages are frequently ignored, and the final net effect would be for ZENOHC_CUSTOM_TARGET to be ignored if CARGO_BUILD_TARGET is set, that does not seem super intuitive.
Hello @sashacmc @DenisBiryukov91, do you have any further feedback on the PR? Thanks!
I don't mind to the proposed logic: if CARGO_BUILD_TARGET is set then use it, but if ZENOHC_CUSTOM_TARGET is set it overrides the CARGO_BUILD_TARGET.
There is only one update I'd like to propose. On the configuration stage CMakeLists script shows all it's settings to allow developer to fully observe the configuration. Notice that it shows not only ZENOHC_... variables, but also may show other variables which could be important for user (like CMAKE_BUILD_TYPE and BUILD_SHARED_LIBS)
[cmake] -- CMAKE_BUILD_TYPE = Release
[cmake] -- ZENOHC_BUILD_IN_SOURCE_TREE = TRUE
[cmake] -- ZENOHC_BUILD_WITH_SHARED_MEMORY = FALSE
[cmake] -- ZENOHC_BUILD_WITH_UNSTABLE_API = FALSE
[cmake] -- ZENOHC_BUILD_TESTS_WITH_CXX = FALSE
[cmake] -- ZENOHC_CUSTOM_TARGET =
[cmake] -- ZENOHC_CARGO_CHANNEL =
[cmake] -- ZENOHC_CARGO_FLAGS =
[cmake] -- BUILD_SHARED_LIBS = TRUE
[cmake] -- ZENOHC_TREAT_WARNING_AS_ERROR = OFF
[cmake] -- project_version = 1.2.1
[cmake] -- Mode: IDE, Single-Config generator (Unix Makefiles)
I think it would be nice to to show also diagnostic like
-- ZENOHC_CUSTOM_TARGET = xxx
-- CARGO_BUILD_TARGET = yyy
...
-- Build target: xxxx (from ZENOHC_CUSTOM_TARGET)
or
-- ZENOHC_CUSTOM_TARGET =
-- CARGO_BUILD_TARGET = yyy
...
-- Build target: yyy (from CARGO_BUILD_TARGET)
PR missing one of the required labels: {'internal', 'documentation', 'new feature', 'breaking-change', 'enhancement', 'dependencies', 'bug'}
![github-actions[bot] avatar](https://avatars.githubusercontent.com/in/15368?v=4 "Published by a pub.dev verified publisher"){kind=small}
I don't mind to the proposed logic: if CARGO_BUILD_TARGET is set then use it, but if ZENOHC_CUSTOM_TARGET is set it overrides the CARGO_BUILD_TARGET.
There is only one update I'd like to propose. On the configuration stage CMakeLists script shows all it's settings to allow developer to fully observe the configuration. Notice that it shows not only ZENOHC_... variables, but also may show other variables which could be important for user (like CMAKE_BUILD_TYPE and BUILD_SHARED_LIBS)
[cmake] -- CMAKE_BUILD_TYPE = Release [cmake] -- ZENOHC_BUILD_IN_SOURCE_TREE = TRUE [cmake] -- ZENOHC_BUILD_WITH_SHARED_MEMORY = FALSE [cmake] -- ZENOHC_BUILD_WITH_UNSTABLE_API = FALSE [cmake] -- ZENOHC_BUILD_TESTS_WITH_CXX = FALSE [cmake] -- ZENOHC_CUSTOM_TARGET = [cmake] -- ZENOHC_CARGO_CHANNEL = [cmake] -- ZENOHC_CARGO_FLAGS = [cmake] -- BUILD_SHARED_LIBS = TRUE [cmake] -- ZENOHC_TREAT_WARNING_AS_ERROR = OFF [cmake] -- project_version = 1.2.1 [cmake] -- Mode: IDE, Single-Config generator (Unix Makefiles)I think it would be nice to to show also diagnostic like
-- ZENOHC_CUSTOM_TARGET = xxx -- CARGO_BUILD_TARGET = yyy ... -- Build target: xxxx (from ZENOHC_CUSTOM_TARGET)or
-- ZENOHC_CUSTOM_TARGET = -- CARGO_BUILD_TARGET = yyy ... -- Build target: yyy (from CARGO_BUILD_TARGET)
Thanks for the feedback @milyin ! I implemented the proposed solution in https://github.com/eclipse-zenoh/zenoh-c/pull/469/commits/d191a19000707110a0b57f9f186f78c2c8d88a9a .
PR missing one of the required labels: {'internal', 'documentation', 'new feature', 'breaking-change', 'enhancement', 'dependencies', 'bug'}
I do not think I have the permissions for adding any labels.