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

Fix: Static Builds Against C-Blosc2 #3715

Merged
merged 3 commits into from
Aug 11, 2023

Conversation

ax3l
Copy link
Contributor

@ax3l ax3l commented Jul 26, 2023

This uses the new C-Blosc2 Blosc2Config.cmake packages, so we can do purely static builds.

Close #3714

@ax3l
Copy link
Contributor Author

ax3l commented Aug 7, 2023

C-Blosc2 v2.10.1 contains the fix! 🎉
https://github.com/Blosc/c-blosc2/releases/tag/v2.10.1

Copy link
Collaborator

@vicentebolea vicentebolea left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@ax3l many thanks for this contributions. If you do not have cycles to follow up my request let me know and I can take of it.

cmake/DetectOptions.cmake Outdated Show resolved Hide resolved
source/adios2/CMakeLists.txt Outdated Show resolved Hide resolved
@vicentebolea vicentebolea force-pushed the fix-blosc2-findpackage branch 2 times, most recently from 4481696 to 592efa2 Compare August 10, 2023 21:47
@vicentebolea vicentebolea changed the base branch from release_29 to master August 10, 2023 21:47
@vicentebolea vicentebolea added this to the v2.9.2 milestone Aug 10, 2023
@vicentebolea vicentebolea force-pushed the fix-blosc2-findpackage branch 2 times, most recently from beb9966 to de9df0a Compare August 10, 2023 22:15
- Give priority to Blosc2Config.cmake file rather than FindBlosc2.cmake.
- Prefer shared Blosc2 library.

Co-Authored-By: Axel Huebl <axel.huebl@plasma.ninja>
@vicentebolea
Copy link
Collaborator

@scottwittenburg I am adding a blosc2 dependency to the spack builds. Note that Blosc2 does not have a Spack pkg and the reason to install this in our Spack images is that they are our standard images that we test most of the configurations. I am not too strong about it, we can move them to the nonspack build. Let me know.

On another note I changed the tag naming while building the builds so that we can avoid having to retag and we can build a leaf image without rebuilding the base image (if no changes in base).

@scottwittenburg
Copy link
Collaborator

@vicentebolea

Regarding building a non-spack blosc in the spack image: does blosc share any dependencies with adios2 which might be picked up from the system when building and possibly conflict later? Maybe it's nothing to worry about, I'm just thinking out loud.

Regarding your change to remove the explicit tagging step in favor of just using the right tag when building: sounds good to me.

Thanks!

@vicentebolea
Copy link
Collaborator

@vicentebolea

Regarding building a non-spack blosc in the spack image: does blosc share any dependencies with adios2 which might be picked up from the system when building and possibly conflict later? Maybe it's nothing to worry about, I'm just thinking out loud.

Good question. I do not see this possibly happening we are building a very minimal blosc2

@scottwittenburg
Copy link
Collaborator

Good question. I do not see this possibly happening we are building a very minimal blosc2

I see by default it turns on zlib, zstd, and lz4, and also by default it uses it's own internal "thirdparty" versions of those.

c-blosc2 internal dependency versions:

lz4-1.9.4
zlib-1.2.13
zstd-1.5.5

adios2 spack image contains:

lz4@1.9.4
zlib@1.2.13
zstd@1.5.2

So there's a pretty close match between what we have in the spack image, and what c-blosc2 needs, so if problems arise from doing it this way we could try to get the c-blosc2 build to use our spack-installed dependencies. Or we could also just move c-blosc2 to the non-spack build.

By the way, our image also contains c-blosc@1.21.1, but I guess that's something different. Maybe there's a c-blosc2 spack package in the works?

Actually, there is a c-blosc2 package in spack, and it has the version mentioned above. At that's what it looks like from this merged PR: spack/spack#39297

@scottwittenburg
Copy link
Collaborator

However, it does seem that the adios2 spack package only knows about c-blosc, nothing about c-blosc2, so maybe that's why we can't use the spack package.

@vicentebolea
Copy link
Collaborator

However, it does seem that the adios2 spack package only knows about c-blosc, nothing about c-blosc2, so maybe that's why we can't use the spack package.

@scottwittenburg thanks for pointing out this, that is indeed the Spack package for blosc2, I did made a search using spack search to [bB]losc2 without luck but indeed the name is c-blosc2. Will have to update our adios2 pkg to use c-blocs2 rather than c-blocs.

@scottwittenburg
Copy link
Collaborator

Will have to update our adios2 pkg to use c-blocs2 rather than c-blocs.

Yeah. But I think it makes sense to go ahead with the current approach here for now, as that will take some time to get merged and appear in the E4S image we're using as a base.

@vicentebolea
Copy link
Collaborator

Yeah. But I think it makes sense to go ahead with the current approach here for now, as that will take some time to get merged and appear in the E4S image we're using as a base.

I am adding c-blocs2 to our spec.yml in the meanwhile.

@vicentebolea
Copy link
Collaborator

@scottwittenburg we need to install this manually for the time being since the blosc2 in the E4S spack is older than what we req. I am building it with the adios2-ci-serial environment activated

@scottwittenburg
Copy link
Collaborator

I see the cmakecache.txt now reflect that blosc2 is enabled and found in your manual install directory. So this looks good to me, once you remove the tmp from the image names.

@vicentebolea
Copy link
Collaborator

I see the cmakecache.txt now reflect that blosc2 is enabled and found in your manual install directory. So this looks good to me, once you remove the tmp from the image names.

Done

@vicentebolea vicentebolea self-assigned this Aug 11, 2023
@vicentebolea vicentebolea merged commit 276383a into ornladios:master Aug 11, 2023
25 checks passed
@ax3l ax3l deleted the fix-blosc2-findpackage branch August 16, 2023 20:47
Comment on lines +91 to +92
if (Blosc2_VERSION VERSION_GREATER_EQUAL 2.10.1)
set(adios2_blosc2_tgt Blosc2::blosc2_$<IF:$<BOOL:${ADIOS2_Blosc2_PREFER_SHARED}>,shared,static>)
Copy link
Contributor Author

@ax3l ax3l Aug 16, 2023

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@vicentebolea there is a bug here:
ADIOS2_Blosc2_PREFER_SHARED should not assume there is always a shared c-blosc2 library available.

C-Blosc2 builds allow to control static and shared builds independently, thus this will cause issues with ADIOS2_Blosc2_PREFER_SHARED=ON (default) for purely static builds (prefer shared != require shared).

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for pointing this out.

ADIOS2_Blosc2_PREFER_SHARED should not assume there is always a shared c-blosc2 library available.

We do not assume this, a couple of lines above we decide this. However, the prefer is not respected since if the user force ADIOS2_Blosc2_PREFER_SHARED=ON it will fail with an static lib

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes, that's what I mean, thx! Do you want to patch this or track it for later patching?

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

A possible solution at #3767

ax3l added a commit to franzpoeschel/openPMD-api that referenced this pull request Aug 16, 2023
@ax3l ax3l restored the fix-blosc2-findpackage branch August 16, 2023 21:03
ax3l added a commit to franzpoeschel/openPMD-api that referenced this pull request Aug 16, 2023
ax3l added a commit to openPMD/openPMD-api that referenced this pull request Aug 17, 2023
* Update blosc -> blosc2, ADIOS2 -> v2.9.0-rc1

ADIOS2 should be replace by v2.9.0 finally

* Fix Blosc2 CMake params

* Guard against CMAKE_OSX_ARCHITECTURES

* For now, deactivate the pthread patch in ADIOS2

* Test small patch for blosc2 CMake

* Add -fPIC flag

* MacOS compatibility

* -DADIOS2_USE_Blosc2=ON

* Build atl and ffs manually to avoid CMake import error

* Revert "Build atl and ffs manually to avoid CMake import error"

This reverts commit c5469a3.

* RC1 -> full release

* Don't build ADIOS1

* -DADIOS2_USE_MHS=OFF

* Use -DADIOS2_INSTALL_GENERATE_CONFIG=OFF workaround

See
ornladios/ADIOS2#3348 (comment)

* Do not disable find calls

* Revert "Do not disable find calls"

This reverts commit 7065b89.

* Try setting ADIOS2_PATH

* Keep ADIOS1 + c-blosc1

* Undo `build.yml` Changes

* ADIOS2: no c-blosc1 pthread issue

* HDF5: 1.14.1-2

* macOS CI: Now has 3 cores

* C-Blosc2 Updates

- CMake options
- Windows
- build stamp

* Windows Updates

* HDF5 Patches

* C-Blosc 2.9.3 + CMake Patch

* ARM: H5Detect

* Windows: Avoid CXX20 in C-Blosc2

* macOS arm64 HDF5: H5detect cross-compile

* HDF5 arm64: cross-compiling H5detect

* [Patch] ADIOS2 c-blosc2 windows.h std::min

* [Patch] C-Blosc2: External Zlib

* [Hack] Windows: Ignore Ext. ZLIB for c-blosc2

Just build again and close eyes.

* ppc64le: Skip C-Blosc & ADIOS1

Due to time limits on Travis-CI.

* Unix: Keep HDF5 1.12.2 for Now

- issues with cross-compile cannot quickly be solved
  for macOS arm64/aarch64
- disable HDF5 support in ADIOS2 for now

* [Patch] C-Blosc2 PUBLIC Linkage Zlib

* Use Blosc2Config.cmake Package

* Cleanup and `cat` ADIOS2 Config File

* [Patch] [Hack] CMake --trace-expand

* Cleanup: cat adios2-config.cmake

* [Travis] PPC64le: `travis_wait 30`

Takes longer now w/o output to the terminal than the default
wait time of 10min.
https://docs.travis-ci.com/user/common-build-problems/#build-times-out-because-no-output-was-received

* [ADIOS2] CMake --trace-expand

* Undo Trace Expand Hacks

* [Travis] PPC64le: `travis_wait 45`

* ADIOS2: w/ SST

* Blosc2: CMake Config PR Merged

* macOS (x86): 10.15 -> 11.0

The macOS 10.15 build images on GH Actions are gone, so we remove
this EOL platform for good now.

* Windows: Skip Building Blosc1

Co-authored-by: Franz Pöschel <franz.poeschel@gmail.com>

* Python: 3.8+

Skip older Python releases, 3.7 is EOL since last month.

* [Patch] ADIOS2: Remove FindBlosc2.cmake

* C-Blosc 2.10.1

* Win: ADIOS 2.9.1

* ADIOS2_Blosc2_PREFER_SHARED=OFF

ornladios/ADIOS2#3715 (comment)

* cibuildwheel 2.12.1 -> 2.14.1

For `pp310-manylinux_aarch64`

---------

Co-authored-by: Axel Huebl <axel.huebl@plasma.ninja>
@ax3l
Copy link
Contributor Author

ax3l commented Aug 18, 2023

@vicentebolea @scottwittenburg just FYI, for transitive, external dependencies in C-Blosc2 you might want to use v2.10.2+
#3714 (comment)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Blosc2: Config.cmake Package in 2.10.2+
3 participants