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

MacOS build fails on libpng package #572

Closed
srherbener opened this issue May 9, 2023 · 11 comments · Fixed by JCSDA/spack#265
Closed

MacOS build fails on libpng package #572

srherbener opened this issue May 9, 2023 · 11 comments · Fixed by JCSDA/spack#265
Assignees
Labels
INFRA JEDI Infrastructure

Comments

@srherbener
Copy link
Collaborator

I just attempted to do a clean build of the unified-dev environment on my arm64 Mac. I'm running Ventura 13.3.1 with command line tools version 14.1 (Clang 14.0.0) and GNU Fortran 12.2.0.

I see the following error during spack install:

[ 69%] Building C object CMakeFiles/png.dir/arm/palette_neon_intrinsics.c.o
 47221 /Users/stephenh/spack-stack/spack/lib/spack/env/clang/clang -DPNG_ARM_NEON_CHECK_SUPPORTED -D       png_EXPORTS -I/Users/stephenh/spack-stack/envs/unified.mymacos/install/apple-clang/14.0.0/zli       b-1.2.13-4qnh2d4/include -I/Users/stephenh/spack-stack/cache/build_stage/spack-stage-libpng-1       .6.37-kls6grh5fubc2tjjklr57siclkcfebwy/spack-build-kls6grh -I/Users/stephenh/spack-stack/cach       e/build_stage/spack-stage-libpng-1.6.37-kls6grh5fubc2tjjklr57siclkcfebwy/spack-src -O2 -g -DN       DEBUG -arch arm64 -isysroot /Library/Developer/CommandLineTools/SDKs/MacOSX13.0.sdk -mmacosx-       version-min=13.0 -fPIC -MD -MT CMakeFiles/png.dir/arm/palette_neon_intrinsics.c.o -MF CMakeFi       les/png.dir/arm/palette_neon_intrinsics.c.o.d -o CMakeFiles/png.dir/arm/palette_neon_intrinsi       cs.c.o -c /Users/stephenh/spack-stack/cache/build_stage/spack-stage-libpng-1.6.37-kls6grh5fub       c2tjjklr57siclkcfebwy/spack-src/arm/palette_neon_intrinsics.c
 47222 /Users/stephenh/spack-stack/cache/build_stage/spack-stage-libpng-1.6.37-kls6grh5fubc2tjjklr57       siclkcfebwy/spack-src/arm/arm_init.c:49:4: error: "PNG_ARM_NEON_FILE undefined: no support fo       r run-time ARM NEON checks"
 47223 #  error "PNG_ARM_NEON_FILE undefined: no support for run-time ARM NEON checks"
 47224    ^
 47225 /Users/stephenh/spack-stack/cache/build_stage/spack-stage-libpng-1.6.37-kls6grh5fubc2tjjklr57       siclkcfebwy/spack-src/arm/arm_init.c:86:27: error: implicit declaration of function 'png_have       _neon' is invalid in C99 [-Werror,-Wimplicit-function-declaration]
 47226                no_neon = !png_have_neon(pp);

I waited until both spack and spack-stack fork_merge PRs were merged and tried this using spack-stack develop and spack jcsda_emc_spack_stack branches.

It looks like the build is trying to use the C99 standard which is outdated and seems wrong to me, but I don't know why this is happening.

Does anyone have any ideas what might be the issue? A missing (or incorrectly set) environment variable for example. I don't think the spack-stack and spack configurations are broken since we just finished verifying that the fork_merge branches (before merging) worked on the MacOS platform.

I'm going to try creating the clean slate again (more carefully this time) and try the build again. I didn't do a spack clean --all before trying this build for example and perhaps that will help.

@climbfuji
Copy link
Collaborator

I see ... this is arm64. I have been building on x86_64 (Rosetta2), same for the macOS CI.

@srherbener
Copy link
Collaborator Author

I've got the new build running now (with the even cleaner slate). Let's see if I run into the same error.

If I do run into the same error, perhaps the libpng package.py or CMake config don't recognize arm64 or aarch64 OS specs and it's falling back on an old default?

@srherbener
Copy link
Collaborator Author

Bummer, still got the error:

[ 61%] Building C object CMakeFiles/png_static.dir/arm/filter_neon_intrinsics.c.o
/Users/stephenh/spack-stack/spack/lib/spack/env/clang/clang -DPNG_ARM_NEON_CHECK_SUPPORTED -I/Users/
stephenh/spack-stack/envs/unified.mymacos/install/apple-clang/14.0.0/zlib-1.2.13-4qnh2d4/include -I/
Users/stephenh/spack-stack/cache/build_stage/spack-stage-libpng-1.6.37-kls6grh5fubc2tjjklr57siclkcfe
bwy/spack-build-kls6grh -I/Users/stephenh/spack-stack/cache/build_stage/spack-stage-libpng-1.6.37-kl
s6grh5fubc2tjjklr57siclkcfebwy/spack-src -O2 -g -DNDEBUG -arch arm64 -isysroot /Library/Developer/Co
mmandLineTools/SDKs/MacOSX13.0.sdk -mmacosx-version-min=13.0 -MD -MT CMakeFiles/png_static.dir/arm/f
ilter_neon_intrinsics.c.o -MF CMakeFiles/png_static.dir/arm/filter_neon_intrinsics.c.o.d -o CMakeFil
es/png_static.dir/arm/filter_neon_intrinsics.c.o -c /Users/stephenh/spack-stack/cache/build_stage/sp
ack-stage-libpng-1.6.37-kls6grh5fubc2tjjklr57siclkcfebwy/spack-src/arm/filter_neon_intrinsics.c
/Users/stephenh/spack-stack/cache/build_stage/spack-stage-libpng-1.6.37-kls6grh5fubc2tjjklr57siclkcfebwy/spack-src/arm/arm_init.c:49:4: error: "PNG_ARM_NEON_FILE undefined: no support for run-time ARM NEON checks"
#  error "PNG_ARM_NEON_FILE undefined: no support for run-time ARM NEON checks"
   ^
/Users/stephenh/spack-stack/cache/build_stage/spack-stage-libpng-1.6.37-kls6grh5fubc2tjjklr57siclkcfebwy/spack-src/arm/arm_init.c:86:27: error: implicit declaration of function 'png_have_neon' is invalid in C99 [-Werror,-Wimplicit-function-declaration]
               no_neon = !png_have_neon(pp);

It looks like the configuration understood the arm64 architecture since there is a -arch arm64 option being used on the clang command line.

@AlexanderRichert-NOAA
Copy link
Collaborator

Is it using CMake or autotools for libpng? It just changed recently in main spack, so whichever one it's using, maybe you could try using the other package.py and see if that magically fixes it?

@srherbener
Copy link
Collaborator Author

@AlexanderRichert-NOAA it's using CMake now. By the "other package.py" do you mean the one that we had before the fork_merge PR was merged in? Thanks!

@srherbener
Copy link
Collaborator Author

Or is there a way to select autotools in the current package.py?

@AlexanderRichert-NOAA
Copy link
Collaborator

Yeah, I mean reverting to the previous version of the package so you can use autotools. Then if that works, hopefully you can either narrow down the relevant build option(s) or worst case scenario, we could let libpng use either cmake or autotools.

@climbfuji
Copy link
Collaborator

@srherbener @AlexanderRichert-NOAA FYI: spack/spack#35105 specifically spack/spack#35105 (comment) (and what hopefully follows).

@srherbener
Copy link
Collaborator Author

I just found this: conan-io/conan-center-index#2366. Tried -DPNG_ARM_NEON=off and that got the cmake based build to work!

Here's what I did in the libpng/package.py:

class CMakeBuilder(CMakeBuilder):
    def cmake_args(self):
        return [
            self.define("CMAKE_CXX_FLAGS", self.spec["zlib"].headers.include_flags),
            self.define("PNG_ARM_NEON", "off"),                                      <---- added this line
            self.define("ZLIB_ROOT", self.spec["zlib"].prefix),
            self.define("PNG_SHARED", "shared" in self.spec.variants["libs"].value),
            self.define("PNG_STATIC", "static" in self.spec.variants["libs"].value),
        ]

This probably isn't the best way to fix this. I'm thinking that it should only add the PNG_ARM_NEON=off when on arm64/aarch64 platform. Is this okay, or do you recommend coding this a different way?

@climbfuji
Copy link
Collaborator

Nice detective work - yes, that should be conditional on darwin/arch64. I can create the PR for you if you like

@srherbener
Copy link
Collaborator Author

I'll do the PR. Thanks!

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

Successfully merging a pull request may close this issue.

4 participants