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

simplify building zenoh-cpp in vscode #201

Open
milyin opened this issue Sep 3, 2024 · 9 comments
Open

simplify building zenoh-cpp in vscode #201

milyin opened this issue Sep 3, 2024 · 9 comments
Assignees
Labels
internal Changes not included in the changelog

Comments

@milyin
Copy link
Contributor

milyin commented Sep 3, 2024

Describe the release item

When zenoh-cpp project is opened in vscode with cmake extension installed, lots of cryptic errors appears. This happens because cmake extension tries to run configuration step on CMakeLists.txt and fails because zenoh-c and zenoh-pico libraries are not installed as packages.
Currently to start development zenoh-cpp with zenoh-c/zenoh-pico as packages the following steps should be performed:

  • run script scripts/build_from_git.sh ~/some_dir. It's not clear without thorough reading of the readme: why this script?, what destination directory to choose?
  • go to "CMake: Edit CMake Cache (UI)" window and set variable CMAKE_INSTALL_PREFIX to the ~/some_dir

Only after these steps the build with cmake extension in vscode works.

The proposal is to restore direct include of zenoh-c and zenoh-pico when project is opened in vscode (this can be detected by checking TERM_PROGRAM=="vscode" and CMAKE_CURRENT_BINARY_DIR == "${CMAKE_CURRENT_SOURCE_DIR}/build")

In this case the project can be built immediately after opening in vscode, without extra steps.

This requires also support from zenoh-c and zenoh-pico side for "add_subdirectory" command. This was done before but need to be updated now. This is useful to support different ways to use our library in customers cmake project. E.g. the FetchContent cmake command works through add_subdirectory and it would be nice to support it.

@DenisBiryukov91
Copy link
Contributor

Currently zenoh-cpp works the same way as majority of other C++ cmake projects: i.e. it is user responsibility to install all the dependencies.
The suggestion also contains a lot of short comings:

  1. It is unclear why we should favor vscode and specific cmake extension over other ide's
  2. What if one day we would require other non-zenoh dependencies like boost or ptotobuf for example. Should we also install them based on some cryptic logic hidden from user ? Should we also silently install rust toolchain given that it is the requirement for zenoh-c, what about cross-compilation support stuff ?
  3. This would lead to unexpected behavior if the user has already installed his own versions of zenoh-c/pico (since those would likely be ignored/overwritten) - something that might be not evident to debug. This would also lead to different behavior when building zenoh-cpp from vscode vs using some other way (i.e. just using cmake from terminal).
  4. In the past the approach of using add_subdirectory instead of find_package lead to broken find_package way of zenoh-cpp dependencies installation, i.e. the standard way of providing cmake project dependencies simply did not work, because no developers were using it.

@DenisBiryukov91
Copy link
Contributor

That's being said, I totally agree that scripts ergonomics could be improved and likely a private readme should be added.

@milyin
Copy link
Contributor Author

milyin commented Sep 4, 2024

Currently zenoh-cpp works the same way as majority of other C++ cmake projects: i.e. it is user responsibility to install all the dependencies. The suggestion also contains a lot of short comings:

1. It is unclear why we should favor vscode and specific cmake extension over other ide's

Because I'm using it and many people around using it, this is enough reason I believe :-)
Anyway this is not vscode-specific setting, this is just setting which allows to look at zenoh-cpp and start work on it / investigate it without spending extra time configuring

2. What if one day we would require other non-zenoh dependencies like boost or ptotobuf for example. Should we also install them based on some cryptic logic hidden from user ?  Should we also silently install rust toolchain given that it is the requirement for zenoh-c, what about cross-compilation support stuff ?

This is different situation, we don't add boost / protobuf as submodules. If we add them as submodules, then yes, I'll be in favor to include them in the same way.
Rust is a prerequisite for zenoh-c, there will be pretty clear building error if no rust installed.

3. This would lead to unexpected behavior if the user has already installed his own versions of zenoh-c/pico (since those would likely be ignored/overwritten) - something that might be not evident to debug. This would also lead to different behavior when building zenoh-cpp from vscode vs using some other way (i.e. just using cmake from terminal).

Yes, this is specific mode intended for

  1. giving smooth experience to developer when all he needs is to make some quick updates. He opens project in IDE and it works.
  2. giving smooth experience to user when we wants just to quickly clone and look at the project

When it's activated, the very visible message on cmake configuration stage appears, it's not silent. It can be easily turned on/off by explicitlly setting corresponding cmake cache variable.

4. In the past the approach of using add_subdirectory instead of find_package lead to broken find_package way of zenoh-cpp dependencies installation, i.e. the standard way of providing cmake project dependencies simply did not work, because no developers were using it.

It all can and should be covered by ci tests for both build variants. Including original cmake file (FetchContent) is one of standard ways of using cmake-based projects. I don't see reasons to not support it

@milyin milyin self-assigned this Sep 4, 2024
@DenisBiryukov91
Copy link
Contributor

DenisBiryukov91 commented Sep 4, 2024

Rust is a prerequisite for zenoh-c, there will be pretty clear building error if no rust installed.

There will be equally clear error if zenoh-c/pico are not installed.
Could you list the criteria we use when we decide to silently auto-install some dependency based on some cryptic logic (i.e. location where we try to build and ide we are using) ?
Also in this mode, rust becomes an actual prerequisite for zenoh-cpp (given that zenoh-c is auto-installed) which might be surprising for c++ developers since this is not mentioned anywhere.

Because I'm using it and many people around using it, this is enough reason I believe :-)

Given the shortcoming of this approach, I believe this only justifies you having a customized build environment with whatever scripts/vscode settings you need to make dev process more convenient.

  1. giving smooth experience to developer when all he needs is to make some quick updates. He opens project in IDE and it works.

Most of the time quick updates involve also modifying zenoh-c/pico, meaning that they will likely be installed locally for testing - in this context fetching predefined versions from git will have little value.

When it's activated, the very visible message on cmake configuration stage appears,

How often do people read cmake messages when everything sort of works ?

It can be easily turned on/off by explicitly setting corresponding cmake cache variable.

In this case it will need to be documented as well as the consequences of getting a broken install when using it, likely in bold if this is to be on by default, and if this is not on by default - you would still need to do the same amount of work (i.e. specify these cmake variable instead of CMAKE_INSTALL_PREFIX)

@doganulus
Copy link

A devcontainer with preinstalled dependencies might be better.

Would you be interested in that instead? This is easier to recommend to users.

@mschuckmann
Copy link

A devcontainer with preinstalled dependencies might be better.

Would you be interested in that instead? This is easier to recommend to users.

While dockers with pre-installed dependencies may seem easier, however, they actually make projects brittle and error prone when users become to dependent on them. It's better for users to learn about dependency management and install prerequisites themselves.

@doganulus
Copy link

doganulus commented Sep 11, 2024

It's better for users to learn about dependency management and install prerequisites themselves.

As an educator, I fully agree with that statement. But containers is a great tool for learning dependencies. A well written Containerfile is an executable specification how to setup the project. Local setups, on the other hand, have more hidden states that can bite users more in the long run.

Is this sentiment prevalent among Zenoh developers? Let's discuss more.

@DenisBiryukov91
Copy link
Contributor

Is this sentiment prevalent among Zenoh developers? Let's discuss more.

This issue was created more as a discussion, on how to simplify (or whether it is needed at all) building of Zenoh-cpp for developers and not for users. I believe for users the optimal solution would be to provide support for getting zenoh-cpp with all the necessary dependencies from one of the package managers like Conan, vcpkg, apt or brew.

@doganulus
Copy link

doganulus commented Sep 11, 2024

For the version zenoh-c v0.10.1, I needed to install a specific Rust toolchain (RUST_TOOLCHAIN=1.72.1-x86_64-unknown-linux-gnu) to start working with the library. As a non-Rust developer, it took some time for me to understand the Rust tooling and dig through build flags. So I went for containers.

A full containerization strategy often requires two container images (devel and runtime). You can see it in action for Nvidia CUDA images, for example.

  1. devel image includes all build dependencies to build the library and targets developers. It is helpful for users to build Zenoh from scratch for a configuration different from the default.
  2. runtime image includes Zenoh binaries and their runtime dependencies. Users can use this image as a base for their container-based deployments.

For your information, I needed such a separation for our containerized Zenoh experiments here. So I am interested in some container technology for Zenoh.

@milyin milyin added internal Changes not included in the changelog and removed release Part of the next release labels Oct 15, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
internal Changes not included in the changelog
Projects
Status: No status
Development

No branches or pull requests

4 participants