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 librerun-sdk output for the C++ library #30

Closed
wants to merge 9 commits into from

Conversation

traversaro
Copy link
Contributor

@traversaro traversaro commented Mar 2, 2024

Fix #18 . Work in progress. I am not totally sold on the name of the package, as typically the scheme in conda-forge is lib<name> for the C/C++ backend, and then <name> or <name>-python for the python bindings, while in this case both the C++ and Python packages are actually bindings of the Rust library.

Checklist

  • Used a personal fork of the feedstock to propose changes
  • Bumped the build number (if the version is unchanged)
  • Reset the build number to 0 (if the version changed)
  • Re-rendered with the latest conda-smithy (Use the phrase @conda-forge-admin, please rerender in a comment in this PR for automated rerendering)
  • Ensured the license file is being packaged.

@conda-forge-webservices
Copy link
Contributor

Hi! This is the friendly automated conda-forge-linting service.

I just wanted to let you know that I linted all conda-recipes in your PR (recipe) and found it was in an excellent condition.

I do have some suggestions for making it better though...

For recipe:

  • Jinja2 variable references are suggested to take a {{<one space><variable name><one space>}} form. See lines [23, 56].

@traversaro
Copy link
Contributor Author

@conda-forge-admin, please rerender

conda-forge-webservices[bot] and others added 3 commits March 2, 2024 15:32
@conda-forge-webservices
Copy link
Contributor

Hi! This is the friendly automated conda-forge-linting service.

I just wanted to let you know that I linted all conda-recipes in your PR (recipe) and found it was in an excellent condition.

I do have some suggestions for making it better though...

For recipe:

  • Jinja2 variable references are suggested to take a {{<one space><variable name><one space>}} form. See lines [23, 57].

@conda-forge-webservices
Copy link
Contributor

Hi! This is the friendly automated conda-forge-linting service.

I just wanted to let you know that I linted all conda-recipes in your PR (recipe) and found it was in an excellent condition.

I do have some suggestions for making it better though...

For recipe:

  • Jinja2 variable references are suggested to take a {{<one space><variable name><one space>}} form. See lines [20, 54].

@traversaro
Copy link
Contributor Author

@conda-forge-admin, please rerender

@traversaro
Copy link
Contributor Author

traversaro commented Mar 3, 2024

Ok, Linux is passing, while macOS and Windows are failing but with errors that are easy to deal with. However, there is currently a bigger problem. As the two different outputs of the recipe depends on distinct dependencies that have multiple pinned version (libarrow for C++, python for python, see https://github.com/conda-forge/conda-forge-pinning-feedstock/blob/0a71f3c21840d35ea1d4a039e7a72f97de8dc5bb/recipe/conda_build_config.yaml#L455-L459 and https://github.com/conda-forge/conda-forge-pinning-feedstock/blob/0a71f3c21840d35ea1d4a039e7a72f97de8dc5bb/recipe/conda_build_config.yaml#L764-L769) conda-smithy instead of generating separate build matrices for the C++ and Python version, it is generating a matrix of 4 libarrow versions x 4 python versions, even if this is completely unnecessary.

At this point, as I have no idea how to change this behavior, I guess it make sense to package the C++ version as its own feedstock, given that anyhow the compilation of the rust part is not shared?

@ruben-arts
Copy link
Contributor

Do you(@traversaro) know how conda-smithy reacts to multiple feedstocks on one source release? Does it just trigger a rerender on all of them?
If so I wouldn't see a problem with spitting them either.

@traversaro
Copy link
Contributor Author

Do you(@traversaro) know how conda-smithy reacts to multiple feedstocks on one source release? Does it just trigger a rerender on all of them?

I guess so, but to be honest it is just an intuition and I do not remember an example of that.

@jleibs
Copy link
Contributor

jleibs commented Mar 4, 2024

instead of generating separate build matrices for the C++ and Python version, it is generating a matrix of 4 libarrow versions x 4 python versions

Ahh yeah, that matrix explosion is unfortunate. In general does the build actually need to be parameterized by python version? For pypi I know we only build a single wheel with -cp38-abi3 that works across all python versions. Is there a way to do something similar in conda-forge?

@traversaro
Copy link
Contributor Author

traversaro commented Mar 4, 2024

For pypi I know we only build a single wheel with -cp38-abi3 that works across all python versions. Is there a way to do something similar in conda-forge?

I think there have been discussion in that direction (see conda-forge/conda-forge.github.io#1865 and conda-forge/python-feedstock#669), but I am afraid conda-forge tooling at the moment is still not working for that, but I may be wrong.

@traversaro
Copy link
Contributor Author

Ok, I opened a PR for adding this as a new feedstock, see conda-forge/staged-recipes#25648 . Let's close this.

@traversaro traversaro closed this Mar 6, 2024
@h-vetinari
Copy link
Member

Do you know how conda-smithy reacts to multiple feedstocks on one source release? Does it just trigger a rerender on all of them?

Yes. LLVM is split over at least 8 feedstocks, and they all get a PR when a new version is released

@traversaro
Copy link
Contributor Author

Great, thanks!

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.

Package C++ version of rerun-sdk?
4 participants