units icon indicating copy to clipboard operation
units copied to clipboard

Conan package

Open jl-wynen opened this issue 3 years ago • 12 comments

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?

jl-wynen avatar Aug 23 '21 13:08 jl-wynen

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.

phlptp avatar Aug 23 '21 14:08 phlptp

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.

jl-wynen avatar Aug 24 '21 15:08 jl-wynen

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

nightlark avatar Aug 24 '21 16:08 nightlark

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 avatar Aug 26 '21 22:08 phlptp

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?

petersteneteg avatar Aug 28 '23 19:08 petersteneteg

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).

SimonHeybrock avatar Aug 29 '23 04:08 SimonHeybrock

@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.

petersteneteg avatar Aug 31 '23 11:08 petersteneteg

@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?

SimonHeybrock avatar Aug 31 '23 12:08 SimonHeybrock

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?

phlptp avatar Sep 01 '23 00:09 phlptp

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

petersteneteg avatar Sep 01 '23 07:09 petersteneteg

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.

phlptp avatar Sep 02 '23 01:09 phlptp

@phlptp fyi https://github.com/microsoft/vcpkg/pull/37417

petersteneteg avatar Mar 17 '24 20:03 petersteneteg