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

dev-python/numcodecs: version bump 0.13.0 #1289

Open
wants to merge 2 commits into
base: master
Choose a base branch
from

Conversation

mschubert
Copy link
Contributor

@mschubert mschubert commented Jul 21, 2024

This is a draft PR for updating numcodecs that comes bundled with c-blosc and the latter includes zstd (setting aside that this shoud be unbundled)

If I run:

ebuild numcodecs-0.13.0.ebuild manifest clean compile
# python3.12 -m gpep517 build-wheel --prefix=/PREFIX/usr --backend setuptools.build_meta --output-fd 3 \
#    --wheel-dir /PREFIX/var/tmp/portage/dev-python/numcodecs-0.13.0/work/numcodecs-0.13.0-python3_12/wheel

and then, in "${WORKDIR}/numcodecs-0.13.0-python3_12/build/lib.linux-x86_64-cpython-312/numcodecs", I get the following error importing a generated .so:

from ctypes import cdll
cdll.LoadLibrary('./zstd.cpython-312-x86_64-linux-gnu.so')
# OSError: ./zstd.cpython-312-x86_64-linux-gnu.so: undefined symbol: ZSTD_compressBlock_btopt

This is the same error that I get when I install the ebuild and then try to import the package.

Surprisingly, manually cloning and building the package results in a functional .so:

git clone https://github.com/zarr-developers/numcodecs.git
cd numcodecs
git submodule update --init --recursive
git checkout v0.13.0
python3.12 -m gpep517 build-wheel --prefix=/home/schubert/gentoo/usr \
    --backend setuptools.build_meta --output-fd 1 --wheel-dir .
cd build/lib.linux-x86_64-cpython-312/numcodecs
# repeat python import as above -> works

The .so created by ebuild is also much smaller than by the manual build.

Any ideas why this might be? Also tagging @TheChymera who submitted the ebuild initially (likely related: zarr-developers/numcodecs#506, zarr-developers/zarr-python#961)

@AndrewAmmerlaan
Copy link
Member

The reason it works when you do it manually, and not in portage probably has to do with CFLAGS/LDFLAGS. Specifically, I suspect this is your problem: https://wiki.gentoo.org/wiki/Project:Quality_Assurance/As-needed#Failure_in_execution.2C_undefined_symbols

Signed-off-by: Michael Schubert <mschu.dev@gmail.com>
Signed-off-by: Michael Schubert <mschu.dev@gmail.com>
@mschubert
Copy link
Contributor Author

mschubert commented Jul 22, 2024

Thanks! I was able to track it down to issues with CFLAGS=-pipe and have disabled this -> builds and imports fine now

@mschubert mschubert marked this pull request as ready for review July 22, 2024 09:51
@AndrewAmmerlaan
Copy link
Member

AndrewAmmerlaan commented Jul 22, 2024

Thanks! I was able to track it down to issues with CFLAGS=-pipe and have disabled this -> builds and imports fine now

I doubt -pipe is the root cause of this issue, it should not affect the built binary, only the build process: https://wiki.gentoo.org/wiki/GCC_optimization#-pipe

@mschubert
Copy link
Contributor Author

I agree that this shouldn't make a difference, but I get the module import errors only when not filtering out this flag. I also tried changing the other flags (-O2, -Wl,O1, -Wl,--as-needed), but they did not make a difference.

With all (combinations of) flags the package builds and installs fine, but the error comes from:

from numcodecs import zstd # success with filter-flags -pipe
# otherwise: OSError: ./zstd.cpython-312-x86_64-linux-gnu.so: undefined symbol: ZSTD_compressBlock_btopt

@AndrewAmmerlaan
Copy link
Member

Try with LDFLAGS="-Wl,--no-as-needed" to explicitly override the default.

@mschubert
Copy link
Contributor Author

Exactly the same error with

python_compile() {
    LDFLAGS="-Wl,--no-as-needed" distutils-r1_python_compile
}

Comment on lines -35 to -36
local -x DISABLE_NUMCODECS_AVX2=1
local -x DISABLE_NUMCODECS_SSE2=1
Copy link
Member

Choose a reason for hiding this comment

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

This should be conditional to CPU_FLAGS_X86_SSE2 and CPU_FLAGS_X86_AVX2

@TheChymera
Copy link
Collaborator

I was also very confused by zarr-developers/zarr-python#961 ... if you know more about it, maybe it would be worth sharing with upstream. They closed it since I stopped nagging them, but I assume it hasn't just evaporated. We can just reopen if it persists with 0.13.0.

I have logs from the various combinations here → zarr-developers/numcodecs#506 (comment) for some reason -flto=auto also seems to fix this.

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.

3 participants