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

add osx_arm64 build #61

Merged
merged 54 commits into from
Mar 9, 2022

Conversation

matthiasdiener
Copy link
Contributor

Checklist

  • Used a personal fork of the feedstock to propose changes
  • Bumped the build number (if the version is unchanged)
  • Reset the build number to 0 (if the version changed)
  • Re-rendered with the latest conda-smithy (Use the phrase @conda-forge-admin, please rerender in a comment in this PR for automated rerendering)
  • Ensured the license file is being packaged.

@conda-forge-linter
Copy link

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 (recipe) and found it was in an excellent condition.

@matthiasdiener matthiasdiener changed the title Bot pr arm osx he1886c arm osx Dec 23, 2021
@Tobias-Fischer
Copy link
Contributor

FYI - I was able to compile this locally (natively) after applying the following patch:

diff --git a/recipe/build.sh b/recipe/build.sh
index c8ca87c..dc8529b 100755
--- a/recipe/build.sh
+++ b/recipe/build.sh
@@ -23,6 +23,7 @@ if [[ "$PKG_NAME" == "scotch" ]]; then
   # build
   cd src/
   make esmumps
+  cd ..
 
   # install
   mkdir -p $PREFIX/lib/

I uploaded the resulting packages to https://anaconda.org/tobiasrobotics/

@Tobias-Fischer
Copy link
Contributor

@conda-forge-admin please restart ci

@Tobias-Fischer
Copy link
Contributor

@matthiasdiener any chance you can give me write access to your fork?

@Tobias-Fischer
Copy link
Contributor

@conda-forge-admin please rerender

@matthiasdiener
Copy link
Contributor Author

@matthiasdiener any chance you can give me write access to your fork?

Sure, you should have gotten an invitation.

@Tobias-Fischer
Copy link
Contributor

So it seems like this will need considerable work to get to cross-compile unfortunately.

They use an executable that needs to run on the build platform to generate some header files (see https://gitlab.inria.fr/scotch/scotch/-/blob/master/src/libscotch/CMakeLists.txt#L52-111). So a native build for osx-64 has to be done first, and then the cross-compile build. General how-to if anyone is keen on giving it a go: https://cmake.org/cmake/help/book/mastering-cmake/chapter/Cross%20Compiling%20With%20CMake.html#running-executables-built-in-the-project

@Tobias-Fischer
Copy link
Contributor

Also I'm not sure why we now see a linker error in the osx-64 build:

x86_64-apple-darwin13.4.0-clang -shared -Wl,-pie -Wl,-headerpad_max_install_names -Wl,-dead_strip_dylibs -Wl,-rpath,/Users/runner/miniforge3/conda-bld/scotch-pkgs_1645054245672/_h_env_placehold_placehold_placehold_placehold_placehold_placehold_placehold_placehold_placehold_placehold_placehold_placehold_placehold_placehold_placehold_placehold_placehold_placehold_placeho/lib -L/Users/runner/miniforge3/conda-bld/scotch-pkgs_1645054245672/_h_env_placehold_placehold_placehold_placehold_placehold_placehold_placehold_placehold_placehold_placehold_placehold_placehold_placehold_placehold_placehold_placehold_placehold_placehold_placeho/lib -lm -lz -pthread -lm -lz -pthread -Wl,-install_name,@rpath/libptscotcherr-6.dylib library_error_pt.o -o libptscotcherr-6.dylib
ld: warning: -pie being ignored. It is only used when linking a main executable
Undefined symbols for architecture x86_64:
  "_MPI_Comm_rank", referenced from:
      _SCOTCH_errorPrint in library_error_pt.o
      _SCOTCH_errorPrintW in library_error_pt.o
  "_MPI_Initialized", referenced from:
      _SCOTCH_errorPrint in library_error_pt.o
      _SCOTCH_errorPrintW in library_error_pt.o
  "_ompi_mpi_comm_world", referenced from:
      _SCOTCH_errorPrint in library_error_pt.o
      _SCOTCH_errorPrintW in library_error_pt.o
ld: symbol(s) not found for architecture x86_64
clang-11: error: linker command failed with exit code 1 (use -v to see invocation)
make[1]: *** [Makefile:3120: libptscotcherr.a] Error 1

This was referenced Feb 16, 2022
@matthiasdiener
Copy link
Contributor Author

So it seems like this will need considerable work to get to cross-compile unfortunately.

They use an executable that needs to run on the build platform to generate some header files (see https://gitlab.inria.fr/scotch/scotch/-/blob/master/src/libscotch/CMakeLists.txt#L52-111). So a native build for osx-64 has to be done first, and then the cross-compile build. General how-to if anyone is keen on giving it a go: https://cmake.org/cmake/help/book/mastering-cmake/chapter/Cross%20Compiling%20With%20CMake.html#running-executables-built-in-the-project

Maybe it is enough to compile dummysizes/ptdummysizes with the host compiler.

@Tobias-Fischer
Copy link
Contributor

It probably is, but I'm not CMake guru enough to work that out .. @traversaro - sorry to ping you - do you have any ideas what the easiest way forward would be?

@traversaro
Copy link
Contributor

It probably is, but I'm not CMake guru enough to work that out .. @traversaro - sorry to ping you - do you have any ideas what the easiest way forward would be?

Sorry, I had lost this mention! Fortunatly I considered "check what's the status of recursive ipopt transitive dependency on osx-64" a fun activity anyhow. :D

@traversaro
Copy link
Contributor

traversaro commented Feb 19, 2022

So, a bit of the things that I understood:

CMake build system not available

  • The CMake build system for scotch is available since scotch 7, while we are building scotch 6.0.9 at the moment, so the only build system available is the Makefile one. If we ever switch to CMake, we need to remember to to this at the same time of a version update to avoid ABI issues (fortunatly scotch's run_exports has max_pin='x.x.x'
    - {{ pin_subpackage('scotch', max_pin='x.x.x') }}
    , so even a patch version update will be enough)

dummysize and ptdummysize

I inspected a bit what the dummysize tool is doing, and apparently is computing a bunch of paddings/scrut sizes via sizeof() (see https://gitlab.inria.fr/scotch/scotch/-/blob/v6.0.9/src/libscotch/dummysizes.c#L240). That means that running dummysize on the build platform (as describe in https://cmake.org/cmake/help/book/mastering-cmake/chapter/Cross%20Compiling%20With%20CMake.html#running-executables-built-in-the-project, i.e. osx intel64) may not work and introduce subtle bugs, because nothing ensures that a sizeof of a structure on osx intel64 is the same of osx arm64. For cross-compilation to work, a osx-arm64 dummysize (either manually, or via emulation that I do not think is available) should produce the headers:

  • scotch.h
  • scotchf.h
  • ptscotch.h
  • ptscotchf.h

Then, just for cross-compilation builds we should comment out the rules to build those files from the make file (i.e. from mhttps://gitlab.inria.fr/scotch/scotch/-/blob/v6.0.9/src/libscotch/Makefile#L3079 to https://gitlab.inria.fr/scotch/scotch/-/blob/v6.0.9/src/libscotch/Makefile#L3093) as those files are already available and should not be generated as part of the build. Probaly other modifications may be necessary to handle the installation of those files.

In a nutshell, we may take those headers from @Tobias-Fischer's builds, but we should add a logic to clearly fail as soon as the version of scotch is bumped, as in that case those headers should be generated again manually.

@matthiasdiener matthiasdiener changed the title arm osx add osx_arm64 build Mar 8, 2022
@matthiasdiener matthiasdiener marked this pull request as ready for review March 8, 2022 19:21
@matthiasdiener
Copy link
Contributor Author

Thank you for the great work @traversaro @Tobias-Fischer ! This is ready for review.


# build
cd src/
make ptesmumps
Copy link
Member

@isuruf isuruf Mar 8, 2022

Choose a reason for hiding this comment

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

If you do the following, then you don't need the new patch.

Suggested change
make ptesmumps
make ptesmumps CCD=$CC_FOR_BUILD

Copy link
Contributor Author

Choose a reason for hiding this comment

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

If I understood #67 (comment) correctly, setting CCD might not be enough?

Copy link
Contributor

Choose a reason for hiding this comment

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

I may be wrong (I am definititely not a Make expert) but in the end I went for the patch as it seemed to me that in https://gitlab.inria.fr/scotch/scotch/-/blob/v6.0.9/src/libscotch/Makefile#L64 there was some recursive make logic that was basically overwriting the CCD value.

Copy link
Contributor

Choose a reason for hiding this comment

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

For future reference, this fails: https://dev.azure.com/conda-forge/feedstock-builds/_build/results?buildId=470850&view=logs&jobId=7afb22dc-3239-5456-794e-5378c2396929

+ make esmumps CCD=/Users/runner/miniforge3/conda-bld/scotch-pkgs_1646774941871/_build_env/bin/x86_64-apple-darwin13.4.0-clang
(cd libscotch ;      make VERSION=6 RELEASE=0 PATCHLEVEL=9 scotch && make install)
make[1]: Entering directory '/Users/runner/miniforge3/conda-bld/scotch-pkgs_1646774941871/work/src/libscotch'
make 					\
				CC="arm64-apple-darwin20.0.0-clang"					\
				CCD="arm64-apple-darwin20.0.0-clang"					\
				scotch.h					\
				scotchf.h					\
				libscotch.a					\
				libscotcherr.a				\
				libscotcherrexit.a
make[2]: Entering directory '/Users/runner/miniforge3/conda-bld/scotch-pkgs_1646774941871/work/src/libscotch'
arm64-apple-darwin20.0.0-clang -ftree-vectorize -fPIC -fPIE -fstack-protector-strong -O2 -pipe -isystem /Users/runner/miniforge3/conda-bld/scotch-pkgs_1646774941871/_h_env_placehold_placehold_placehold_placehold_placehold_placehold_placehold_placehold_placehold_placehold_placehold_placehold_placehold_placehold_placehold_placehold_placehold_placehold_placeho/include -fdebug-prefix-map=/Users/runner/miniforge3/conda-bld/scotch-pkgs_1646774941871/work=/usr/local/src/conda/scotch-6.0.9 -fdebug-prefix-map=/Users/runner/miniforge3/conda-bld/scotch-pkgs_1646774941871/_h_env_placehold_placehold_placehold_placehold_placehold_placehold_placehold_placehold_placehold_placehold_placehold_placehold_placehold_placehold_placehold_placehold_placehold_placehold_placeho=/usr/local/src/conda-prefix -O3 -I/Users/runner/miniforge3/conda-bld/scotch-pkgs_1646774941871/_h_env_placehold_placehold_placehold_placehold_placehold_placehold_placehold_placehold_placehold_placehold_placehold_placehold_placehold_placehold_placehold_placehold_placehold_placehold_placeho/include -DIDXSIZE64 -DSCOTCH_RENAME -Drestrict=__restrict -DCOMMON_FILE_COMPRESS_GZ -DCOMMON_RANDOM_FIXED_SEED -DCOMMON_PTHREAD -DSCOTCH_PTHREAD -DCOMMON_PTHREAD_BARRIER -DCOMMON_TIMING_OLD -O3 -I/Users/runner/miniforge3/conda-bld/scotch-pkgs_1646774941871/_h_env_placehold_placehold_placehold_placehold_placehold_placehold_placehold_placehold_placehold_placehold_placehold_placehold_placehold_placehold_placehold_placehold_placehold_placehold_placeho/include -DIDXSIZE64 -DSCOTCH_RENAME -Drestrict=__restrict -DCOMMON_FILE_COMPRESS_GZ -DCOMMON_RANDOM_FIXED_SEED -DCOMMON_PTHREAD -DSCOTCH_PTHREAD -DCOMMON_PTHREAD_BARRIER -DCOMMON_TIMING_OLD -O3 -I/Users/runner/miniforge3/conda-bld/scotch-pkgs_1646774941871/_h_env_placehold_placehold_placehold_placehold_placehold_placehold_placehold_placehold_placehold_placehold_placehold_placehold_placehold_placehold_placehold_placehold_placehold_placehold_placeho/include -DIDXSIZE64 -DSCOTCH_RENAME -Drestrict=__restrict -DCOMMON_FILE_COMPRESS_GZ -DCOMMON_RANDOM_FIXED_SEED -DCOMMON_PTHREAD -DSCOTCH_PTHREAD -DCOMMON_PTHREAD_BARRIER -DCOMMON_TIMING_OLD -fPIC -c arch.c -o arch.o -DSCOTCH_VERSION_NUM=6 -DSCOTCH_RELEASE_NUM=0 -DSCOTCH_PATCHLEVEL_NUM=9
arm64-apple-darwin20.0.0-clang -ftree-vectorize -fPIC -fPIE -fstack-protector-strong -O2 -pipe -isystem /Users/runner/miniforge3/conda-bld/scotch-pkgs_1646774941871/_h_env_placehold_placehold_placehold_placehold_placehold_placehold_placehold_placehold_placehold_placehold_placehold_placehold_placehold_placehold_placehold_placehold_placehold_placehold_placeho/include -fdebug-prefix-map=/Users/runner/miniforge3/conda-bld/scotch-pkgs_1646774941871/work=/usr/local/src/conda/scotch-6.0.9 -fdebug-prefix-map=/Users/runner/miniforge3/conda-bld/scotch-pkgs_1646774941871/_h_env_placehold_placehold_placehold_placehold_placehold_placehold_placehold_placehold_placehold_placehold_placehold_placehold_placehold_placehold_placehold_placehold_placehold_placehold_placeho=/usr/local/src/conda-prefix -O3 -I/Users/runner/miniforge3/conda-bld/scotch-pkgs_1646774941871/_h_env_placehold_placehold_placehold_placehold_placehold_placehold_placehold_placehold_placehold_placehold_placehold_placehold_placehold_placehold_placehold_placehold_placehold_placehold_placeho/include -DIDXSIZE64 -DSCOTCH_RENAME -Drestrict=__restrict -DCOMMON_FILE_COMPRESS_GZ -DCOMMON_RANDOM_FIXED_SEED -DCOMMON_PTHREAD -DSCOTCH_PTHREAD -DCOMMON_PTHREAD_BARRIER -DCOMMON_TIMING_OLD -O3 -I/Users/runner/miniforge3/conda-bld/scotch-pkgs_1646774941871/_h_env_placehold_placehold_placehold_placehold_placehold_placehold_placehold_placehold_placehold_placehold_placehold_placehold_placehold_placehold_placehold_placehold_placehold_placehold_placeho/include -DIDXSIZE64 -DSCOTCH_RENAME -Drestrict=__restrict -DCOMMON_FILE_COMPRESS_GZ -DCOMMON_RANDOM_FIXED_SEED -DCOMMON_PTHREAD -DSCOTCH_PTHREAD -DCOMMON_PTHREAD_BARRIER -DCOMMON_TIMING_OLD -O3 -I/Users/runner/miniforge3/conda-bld/scotch-pkgs_1646774941871/_h_env_placehold_placehold_placehold_placehold_placehold_placehold_placehold_placehold_placehold_placehold_placehold_placehold_placehold_placehold_placehold_placehold_placehold_placehold_placeho/include -DIDXSIZE64 -DSCOTCH_RENAME -Drestrict=__restrict -DCOMMON_FILE_COMPRESS_GZ -DCOMMON_RANDOM_FIXED_SEED -DCOMMON_PTHREAD -DSCOTCH_PTHREAD -DCOMMON_PTHREAD_BARRIER -DCOMMON_TIMING_OLD -DSCOTCH_VERSION_NUM=6 -DSCOTCH_RELEASE_NUM=0 -DSCOTCH_PATCHLEVEL_NUM=9 dummysizes.c -o dummysizes -Wl,-pie -Wl,-headerpad_max_install_names -Wl,-dead_strip_dylibs -Wl,-rpath,/Users/runner/miniforge3/conda-bld/scotch-pkgs_1646774941871/_h_env_placehold_placehold_placehold_placehold_placehold_placehold_placehold_placehold_placehold_placehold_placehold_placehold_placehold_placehold_placehold_placehold_placehold_placehold_placeho/lib -L/Users/runner/miniforge3/conda-bld/scotch-pkgs_1646774941871/_h_env_placehold_placehold_placehold_placehold_placehold_placehold_placehold_placehold_placehold_placehold_placehold_placehold_placehold_placehold_placehold_placehold_placehold_placehold_placeho/lib -lm -lz -pthread -lm -lz -pthread -lm -lz -pthread
./dummysizes "-s" library.h scotch.h
/bin/sh: ./dummysizes: Bad CPU type in executable

Copy link
Contributor

Choose a reason for hiding this comment

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

Perhaps a simpler patch could be to remove the line https://gitlab.inria.fr/scotch/scotch/-/blob/v6.0.9/src/libscotch/Makefile#L64, but to be honest I am not 100% sure of the other side effects of that patch, while the existing patch of using CC_FOR_BUILD only when I know it was necessary it was more clear to me.

@basnijholt
Copy link
Contributor

Wow, great work!

Everything looks good to me, especially since the tests are passing.

recipe/build.sh Show resolved Hide resolved
@Tobias-Fischer
Copy link
Contributor

@conda-forge-admin please restart ci

@jschueller
Copy link
Contributor

thanks for all the work guys!

@dalcinl
Copy link
Contributor

dalcinl commented Mar 9, 2022

@matthiasdiener Should I merge? Please confirm.

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.