Replace cdts and yum_requirements.txt with Conda packages
#25
Conversation
|
@conda-forge-admin , please re-render |
|
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 ( |
…nda-forge-pinning 2024.10.30.03.03.49
|
@conda-forge-admin , please re-render |
…nda-forge-pinning 2024.10.30.03.03.49
|
@conda-forge-admin , please re-render |
|
Hi! This is the friendly automated conda-forge-webservice. I tried to rerender for you, but it looks like there was nothing to do. This message was generated by GitHub actions workflow run https://github.com/conda-forge/conda-forge-webservices/actions/runs/11591765229. |
jakirkham
changed the title
cdts and yum_requirements.txt with Conda packages
|
Ok now have this working. This is ready for review! 🙂 |
| -D CMAKE_INSTALL_PREFIX:PATH="${PREFIX}" \ | ||
| "${SRC_DIR}" \ | ||
| ; | ||
| cmake --build . --parallel ${CPU_COUNT} |
There was a problem hiding this comment.
Just curious, is there a reason/benefit for using --parallel instead of -GNinja?
There was a problem hiding this comment.
We are still using -G Ninja in the previous invocation of cmake. This command generalizes over commands like make or ninja (without needing to change should the CMake generator change)
Note the next command does the same for make install or ninja install
There was a problem hiding this comment.
Ah right, my bad, I overlooked that. Thanks for pointing that out!
There was a problem hiding this comment.
All good. It is syntax not everyone is familiar with
| @@ -22,12 +22,13 @@ requirements: | |||
| - {{ compiler("c") }} | |||
| - {{ stdlib("c") }} | |||
| - {{ compiler("cxx") }} | |||
| - {{ cdt("systemd-devel") }} | |||
There was a problem hiding this comment.
I'm not confident I know/understand why systemd-devel was needed before and is not anymore? Was that unnecessary all the time or was there anything that happened lately that now removes that requirement?
There was a problem hiding this comment.
There were some issues while making these changes where it wasn't detecting libsystemd headers. Part of this was addressed by updating the CMake build above
However the other issue is libsystemd and libudev were in requirements/build instead of requirements/host. As these are libraries we are linking to, they should be in requirements/host. So have made that change below: #25 (comment)
There was a problem hiding this comment.
Thanks for addressing that.
These are linked to in the build. So should be in `requirements/host`.
| - libsystemd | ||
| - libudev |
There was a problem hiding this comment.
These were in requirements/build before. However the build links to them. So these should be in requirements/host. Hence this move
With these changes we now get
-- Found UDev: $PREFIX/lib/libudev.so
...
-- Found Systemd: $PREFIX/lib/libsystemd.so
|
Thanks Peter! 🙏 |
|
Please let me know how these changes go. Happy to follow up on anything as needed |
Fixes #19
Drops usage of
cdt's and system packages (viayum_requirements.txt). These are all supplied via Conda Packages. So just use those instead. This will also help prep for building AlmaLinux 8, which has fewer CDTs suppliedxref: