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

Conan package #164

Open
jl-wynen opened this issue Aug 23, 2021 · 12 comments
Open

Conan package #164

jl-wynen opened this issue Aug 23, 2021 · 12 comments
Labels
help wanted Extra attention is needed

Comments

@jl-wynen
Copy link
Contributor

We are currently installing units as a CMake external project. While this works, it requires some manual maintenance on our end (and on anyone else's who uses the library in this fashion). This would be alleviated if there were a Conan package of units.

I created a simple test recipe and managed to make it work locally. But it would of course be best to publish a package. We could do so on conancenter directly or on our own, public server as ESS.

I would like to get your input. Did you consider using conan or a different package manager at any point?

@phlptp
Copy link
Collaborator

phlptp commented Aug 23, 2021

All our current use cases use the library as a git submodule or just use the code directly. Some of the platforms we need to support don't have Conan available easily so support for it hasn't been a priority.

I have not looked much into what is involved in supporting it. I have no issue with it being added to Conan.

@jl-wynen
Copy link
Contributor Author

Thanks for the insight. Here is a little update:

We have uploaded a conan recipe to our in-house server: https://artifactoryconan.esss.dk/artifactory/webapp/#/artifacts/browse/tree/General/scipp/scipp/LLNL-Units/0.5.0/stable

We chose this solution because it simplifies testing. But we might revisit it in the future.

@nightlark
Copy link
Collaborator

Eventually it might be nice having it added to conan-center-index, though they currently require signing a CLA to submit packages.

@phlptp
Copy link
Collaborator

phlptp commented Aug 26, 2021

Ah yes, I am not allowed to sign CLA's for the time being which means though we would be happy to have it, we would need an outside contributor to make any additions to conan-center

@phlptp phlptp added the help wanted Extra attention is needed label Mar 28, 2022
@petersteneteg
Copy link
Contributor

Hi,

I was also considering adding units to vcpkg, but ran into a naming issue, in vcpkg units already points to https://github.com/nholthaus/units hence using units will not work since that name has to be unique.
I see that jl-wynen used llnl-units above, which would work also in vcpkg. The only problem then is the cmake package (find_package(units)), using units there is probably fine, since a user will probably only have one units library, but it might lead to collisions? Would you consider renaming also the cmake package to something like llnl-units?

@SimonHeybrock
Copy link
Contributor

In our recipe we have named the cmake package LLNL-Units, see our conanfile and our cmake-conan config and our
target_link_libraries(... LLNL-Units::LLNL-Units).

@petersteneteg
Copy link
Contributor

petersteneteg commented Aug 31, 2023

@SimonHeybrock so you manually patched the units target to be named LLNL-Units instead? or does Conan generate its own cmake targets? Vcpkg generally uses the same targets that is exported by the library it self. And generally vcpkg ports tries hard to avoid changing any names from the source, to make it easier just to "drop in" a vcpkg package.

@SimonHeybrock
Copy link
Contributor

@SimonHeybrock so you manually patched the units target to be named LLNL-Units instead? or does Conan generate its own cmake targets?

We are not patching anything, just using the regular sources from git, i.e., probably Conan makes its own targets?

@phlptp
Copy link
Collaborator

phlptp commented Sep 1, 2023

I added a cmake variable to allow a rename of the project as an experiment. I don't really want to rename the project by default, but I do see the potential for conflict with other "units" libraries. #310 Not sure if that will work for your needs or not?

@petersteneteg
Copy link
Contributor

@phlptp do you have a preferred alternative name? LLNL-Units? or all lowercase llnl-units?

@phlptp
Copy link
Collaborator

phlptp commented Sep 2, 2023

release has been made with the CMAKE variable to customize the project name. -DUNITS_CMAKE_PROJECT_NAME=LLNL-UNITS should do what you want it to do. The default is left as UNITS.

@petersteneteg
Copy link
Contributor

@phlptp fyi microsoft/vcpkg#37417

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
help wanted Extra attention is needed
Projects
None yet
Development

No branches or pull requests

5 participants