Hi @alalazo! I noticed that the following package(s) don't yet have maintainers:

• lz4

Are you interested in adopting any of these package(s)? If so, simply add the following to the package class:

    maintainers("alalazo")

If not, could you contact the developers of this package and see if they are interested? You can quickly see who has worked on a package with spack blame:

$ spack blame lz4

Thank you for your help! Please don't add maintainers without their consent.

You don't have to be a Spack expert or package developer in order to be a "maintainer," it just gives us a list of users willing to review PRs or debug issues relating to this package. A package can have multiple maintainers; just add a list of GitHub handles of anyone who wants to volunteer.

@alalazo shouldn't the run_after be used also for the cmake version ?

With the mods of this PR, if I do :

spack install --overwrite zstd +programs compression=lz4 ^lz4 build_system=makefile

I get a function zstd binary, but with :

spack install --overwrite zstd +programs compression=lz4 ^lz4 build_system=cmake

I get :

/Users/laurent/km3net/spack/opt/spack/darwin-ventura-m1/apple-clang-14.0.0/zstd-1.5.5-c3dgndugmy6syd4uhzsqg7jfro2uulpu/bin/zstd --help

dyld[44282]: Library not loaded: liblz4.1.dylib
  Referenced from: <736B0509-2846-3989-AB02-D26B552C6A2A> /Users/laurent/km3net/spack/opt/spack/darwin-ventura-m1/apple-clang-14.0.0/zstd-1.5.5-c3dgndugmy6syd4uhzsqg7jfro2uulpu/bin/zstd
  Reason: tried: 'liblz4.1.dylib' (no such file), '/System/Volumes/Preboot/Cryptexes/OSliblz4.1.dylib' (no such file), 'liblz4.1.dylib' (no such file), '/usr/local/lib/liblz4.1.dylib' (mach-o file, but is an incompatible architecture (have 'x86_64', need 'arm64')), '/usr/lib/liblz4.1.dylib' (no such file, not in dyld cache), '/Users/laurent/km3net/a/b/c/d/liblz4.1.dylib' (no such file), '/System/Volumes/Preboot/Cryptexes/OS/Users/laurent/km3net/a/b/c/d/liblz4.1.dylib' (no such file), '/Users/laurent/km3net/a/b/c/d/liblz4.1.dylib' (no such file), '/usr/local/lib/liblz4.1.dylib' (mach-o file, but is an incompatible architecture (have 'x86_64', need 'arm64')), '/usr/lib/liblz4.1.dylib' (no such file, not in dyld cache)
zsh: abort       --help

Hm, I thought cmake was supposed to generate shared libraries with @rpath/<name> install names, iirc we set an option for it.

https://cmake.org/cmake/help/latest/policy/CMP0042.html

This policy should make cmake add @rpath in install names since version 3.0 🤔

@aphecetche can you try with -DCMAKE_POLICY_DEFAULT_CMP0042=NEW and require cmake@3: if that's not already the case? It's probably cause the project allows a cmake min version < 3, so the policy is off on cmake 3.

for the cmake version dependency, where should it be put with a new multiple build package.py ? (I'm still trying to adapt my mind to it ;-) ) within the CMakeBuilder part ?

the previous comment was to update the package.py, but anyways, doing :

spack install --overwrite lz4 build_system=cmake ^cmake@3:

seems to lead to a correct RPATH'ed library indeed (no new to specify the policy)

for the cmake version dependency, where should it be put with a new multiple build package.py ?

No, the metadata (i.e. all the directives) goes into the Package not the Builder. You can write something like:

with when("build_system=cmake"):
    depends_on("cmake@3:", type="build")

Hm, confusing. I don't understand why it didn't work before with build_system=cmake then?

I checked, and setting that policy to ON seems necessary.

Don't know what I did exactly, might have been caught by some cache. Sorry about that.

If I redo it "from scratch"

spack clean -a
spack uninstall --all --dependents lz4
spack stage lz4
spack cd lz4
cd ..
mkdir build
cd build
cmake ../spack-src/build/cmake

there is indeed a warning about the issue :

CMake Warning (dev):
  Policy CMP0042 is not set: MACOSX_RPATH is enabled by default.  Run "cmake
  --help-policy CMP0042" for policy details.  Use the cmake_policy command to
  set the policy and suppress this warning.

  MACOSX_RPATH is not specified for the following targets:

   lz4_shared

This warning is for project developers.  Use -Wno-dev to suppress it.

so no, the PR is not ok as is I would say.

Let's just add the policy define, in the meantime I've opened a pr upstream: lz4/lz4#1228

Policy was added before the comments... or am I missing something?

Policy was added before the comments... or am I missing something?

No, I don't think you're missing something, but I did miss the "New changes since you last viewed" from GitHub UI 😉 hence my comment after your commit.

@spackbot run pipeline

I've started that pipeline for you!