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

Add zenoh-c-prebuilt Conan recipe #334

Open
wants to merge 7 commits into
base: main
Choose a base branch
from

Conversation

oteffahi
Copy link
Contributor

@oteffahi oteffahi commented Apr 15, 2024

This PR add a Conan recipe to install Zenoh-C (Currently only 0.10.1-rc) without requiring Rust on the target host.
The recipe pulls the necessary artefacts from upstream's github release for the requested version.

This recipe only supports targets for which Zenoh-C is compiled in its releases.
Library artefacts for all targets are dynamically linked.

Copy link

@gregmedd gregmedd left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Aside from the one issue noted on the test package, this worked for me. I was able to build another conan package (up-zenoh-client-cpp) using zenohc/0.10.1-rc as a dependency.

Comment on lines 10 to 14
test_type = "explicit"

def requirements(self):
self.tool_requires("cmake/[>=3.16 <4]")
self.requires(self.tested_reference_str)

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm not entirely sure why, but with conan 1.63 this didn't work for me. I had to move the tool_requires statement out of requirements(). The documentation indicates that either one should work.

Suggested change
test_type = "explicit"
def requirements(self):
self.tool_requires("cmake/[>=3.16 <4]")
self.requires(self.tested_reference_str)
test_type = "explicit"
tool_requires = "cmake/[>=3.16 <4]"
def requirements(self):
self.requires(self.tested_reference_str)

It is entirely possible this is just something I was doing wrong.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@gregmedd Thank you for testing it on 1.63, as I personally only tested with conan v2. I will look into this issue as soon as possible.

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@oteffahi Happy to help out. Thanks for getting these recipes put together, by the way - they're going to help a lot of uProtocol users and developers!

Let me know if you want me to pull this branch and test in my environment again.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@gregmedd The issue was happening because the tool_requires was being called in requirements instead of build_requirements. Conan2 does not raise an error in this case, but older versions do. Fixed in Move tool_requires to build_requirements function.

"Linux":
"x86_64":
url: "https:/eclipse-zenoh/zenoh-c/releases/download/0.10.1-rc/zenoh-c-0.10.1-rc-x86_64-unknown-linux-gnu.zip"
sha256: "0554b0fe753df3c023b09e005b309d654b1cc33ddc34190b5a5c370b72281c3b"

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I've been thinking on this for a few days to figure out how to deal with tagged releases and Conan packages on a few of the uProtocol libraries I'm working on, and have a question:

What is the workflow for making a tagged release? It looks like a tag would first need to be added and all of the binaries built / published. Then a follow-up commit after the tagged commit would update the recipes so the Conan packages can be built from the new release. Is that correct?

That would mean that the tagged commit is not actually 100% the release because the recipes are wrong. But the tag can't be moved after the recipes are updated (maybe only in the build from source case?) without changing some of the hashes.

I'm sure I'm just missing a step somewhere 😅

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

You are right, there is a bootstrap-like problem. It's something we have discussed internally and so far moving the tag and updating the hashes seems to be the way to go. This will be done as part of the release process.

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.

3 participants