Add lz4-c as runtime dependency

vicentebolea commented 7 months ago

Checklist

[x] Used a personal fork of the feedstock to propose changes
[x] 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 code>@<space/conda-forge-admin, please rerender in a comment in this PR for automated rerendering)
[x] Ensured the license file is being packaged.

We are hitting the current error when trying to find_package(blosc2) with cmake.

2024-03-14T21:40:24.3713579Z -- No LZ4 library found.  Using internal sources.
2024-03-14T21:40:24.3717552Z CMake Warning at cmake/DetectOptions.cmake:77 (find_package):
2024-03-14T21:40:24.3718437Z   Found package configuration file:
2024-03-14T21:40:24.3718832Z 
2024-03-14T21:40:24.3719087Z     C:/Miniconda/Library/cmake/Blosc2Config.cmake
2024-03-14T21:40:24.3719546Z 
2024-03-14T21:40:24.3719967Z   but it set Blosc2_FOUND to FALSE so package "Blosc2" is considered to be
2024-03-14T21:40:24.3720736Z   NOT FOUND.  Reason given by package:
2024-03-14T21:40:24.3721096Z 
2024-03-14T21:40:24.3721464Z   Blosc2 could not be found because dependency LZ4 could not be found.
2024-03-14T21:40:24.3722010Z 
2024-03-14T21:40:24.3722198Z Call Stack (most recent call first):
2024-03-14T21:40:24.3722718Z   CMakeLists.txt:194 (include)
2024-03-14T21:40:24.3723044Z 
2024-03-14T21:40:24.3723050Z

conda-forge-webservices[bot] commented 7 months ago

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.

hmaarrfk commented 7 months ago

This is a general problem with condos forge where we don't distinguish between run time and builf time dependencies.

Let's please not merger this. I'll point to the original discussion when I get to a com9uter again

For now. With condos tooling, you must specify these dependencies manually

hmaarrfk commented 7 months ago

Conda-forge. And more generally conda itself

hmaarrfk commented 7 months ago

See: https://github.com/conda-forge/conda-forge.github.io/issues/1880

and more specifically the comment from pkgw https://github.com/conda-forge/conda-forge.github.io/issues/1880#issuecomment-1408733984

vicentebolea commented 7 months ago

@hmaarrfk that is fair, thanks for the quick response.

Notice that at this moment any user of blosc2 from conda will not be able to link their application against blosc2 due to lz4-c missing, they will have to manually load lz4-c, which is makes its usage more difficult. I suggest in this case to use internal lz4-c in blosc2 instead, so that we can make this library usable again without runtime deps.

vicentebolea commented 7 months ago

@hmaarrfk I have created #56 that uses the internal lz4. This will allow cmake users to use this package from a CMake project. I am closing this to avoid confussion.

hmaarrfk commented 7 months ago

Notice that at this moment any user of blosc2 from conda will not be able to link their application against blosc2 due to lz4-c missing

I'm very well aware of this issue. I opened the original issue that I referenced.

However, it is important to put the issues in the correct context.

Exactly how many people are using c-blosc2? truthfully, before today, I thought I was the only one that cared about this feedstock.

which is makes its usage more difficult

yes, lets work together to fix this. Please take time to read through the issue I posted above. Isuru made a great suggestion that I suggest you explore. https://github.com/conda-forge/conda-forge.github.io/issues/1880#issuecomment-1385012026

We use dynamic linking at conda-forge, static linking, or vendoring is actively discouraged. https://github.com/conda-forge/cfep/blob/main/cfep-18.md

for now, i don't think it is a big deal to add lz-4 as a dependency.

Some have started to build -dev packages, but in my mind seriously complicates things with conda tooling and I'm not a fan of maintaining them unless there are some serious advantages.

Again, referring to that same discussion https://github.com/conda-forge/conda-forge.github.io/issues/1880#issuecomment-1409675003

hmaarrfk commented 7 months ago

i'm not saying that disucssion is the authority on packaging, but you aren't alone in feeling this way, I was seriously frustrated debugging this.

can you point me to a recipe that uses this package. maybe i can help investigate

vicentebolea commented 7 months ago

Exactly how many people are using c-blosc2? truthfully, before today, I thought I was the only one that cared about this feedstock.

Many ECP/Kitware projects depend on it.

I have recently started using cblosc2 from Conda in the Github CI windows build for the project adios2, it did not work, even if I installed the lz4-c project through conda. I can see the error lies in the findlz4.cmake, it does not find the miniconda libs. We either fix that or we use the internal lz4 lib which will solve everything but is it allowed in Conda?

Some have started to build -dev packages, but in my mind seriously complicates things with conda tooling and I'm not a fan of maintaining them unless there are some serious advantages.

I understand, probably thats the way to go. In other package managers it will be:

libblosc2 for the so/static libs.
blosc2-dev for libs, headers and cmake files

hmaarrfk commented 7 months ago

Many ECP/Kitware projects depend on it.

this is great to hear.

I have recently started using cblosc2 from Conda

i presume you mean conda-forge. Anaconda defaults, and conda-forge are two different projects.

Looking at the cmake file, i don't really know why the project is attempting to find lz4. It should't be linked publicly. so it should not attempt to find it at all.

Helping you on your CI that uses a different layout is really hard to find with "just a short snippet of your log".

Can we please continue this integration challenge on a different issue?

The original premise you shared was not entirely true. lz4-c is pulled in automatically when you install c-blosc2:

$ mamba create --name c-blosc2 c-blosc2 --channel conda-forge --override-channels

Looking for: ['c-blosc2']

conda-forge/linux-64                                        Using cache
conda-forge/noarch                                          Using cache
Transaction

  Prefix: /home/mark/miniforge3/envs/c-blosc2

  Updating specs:

   - c-blosc2

  Package          Version  Build        Channel           Size
─────────────────────────────────────────────────────────────────
  Install:
─────────────────────────────────────────────────────────────────

  + _libgcc_mutex      0.1  conda_forge  conda-forge     Cached
  + libstdcxx-ng    13.2.0  h7e041cc_5   conda-forge     Cached
  + libgomp         13.2.0  h807b86a_5   conda-forge     Cached
  + _openmp_mutex      4.5  2_gnu        conda-forge     Cached
  + libgcc-ng       13.2.0  h807b86a_5   conda-forge     Cached
  + lz4-c            1.9.4  hcb278e6_0   conda-forge     Cached
  + libzlib         1.2.13  hd590300_5   conda-forge     Cached
  + zlib-ng          2.0.7  h0b41bf4_0   conda-forge     Cached
  + zstd             1.5.5  hfc55251_0   conda-forge     Cached
  + c-blosc2        2.13.2  hb4ffafa_0   conda-forge     Cached

  Summary:

  Install: 10 packages

  Total download: 0 B

─────────────────────────────────────────────────────────────────

Confirm changes: [Y/n]

we use something called run_exports.

Honestly, I had to react quickly since your change looks innocent, but it creates alot of churn by adding "one extra" line.

I really want to help, but I think it would be best if you opened an issue pointing us to your full log file or CI attempts to integrate the library from conda-forge.

conda-forge / c-blosc2-feedstock

Add lz4-c as runtime dependency #55