From 992874b95f67410bf37e023303531c941f2e7b2e Mon Sep 17 00:00:00 2001 From: Matthew Feickert Date: Sun, 15 Sep 2024 22:05:41 -0500 Subject: [PATCH 1/7] build: Add CXXFLAGS and LDFLAGS handling for aarch64 macOS --- setup.py | 54 ++++++++++++++++++++++++++++++++++++++++++++---------- 1 file changed, 44 insertions(+), 10 deletions(-) diff --git a/setup.py b/setup.py index 5697138..d61790c 100644 --- a/setup.py +++ b/setup.py @@ -11,6 +11,7 @@ import os import pathlib +import platform import shutil import subprocess import sys @@ -73,6 +74,28 @@ def build_extensions(self): cwd=FASTJET, ) + # Inject the required CXXFLAGS and LDFLAGS for building on + # aarch64 macOS. As Homebrew can not be used at any part in a + # conda-forge build, guard against this by checking for the + # conda-build environment variable. + # This is a bad hack, and will be alleviated if CMake can be used + # by FastJet and FastJet-contrib. + # c.f. https://github.com/scikit-hep/fastjet/issues/310 + if ( + sys.platform == "darwin" + and platform.processor() == "arm" + and "HOMEBREW_PREFIX" in os.environ + and "CONDA_BUILD" not in os.environ + ): + os.environ["CXXFLAGS"] = ( + os.environ.get("CXXFLAGS", "") + + f" -I{os.environ['HOMEBREW_PREFIX']}/include" + ) + os.environ["LDFLAGS"] = ( + os.environ.get("LDFLAGS", "") + + f" -L{os.environ['HOMEBREW_PREFIX']}/lib" + ) + # RPATH is set for shared libraries in the following locations: # * fastjet/ # * fastjet/_fastjet_core/lib/ @@ -81,7 +104,9 @@ def build_extensions(self): env = os.environ.copy() env["PYTHON"] = sys.executable env["PYTHON_INCLUDE"] = f'-I{sysconfig.get_path("include")}' - env["CXXFLAGS"] = "-O3 -Bstatic -lgmp -Bdynamic -std=c++17" + env["CXXFLAGS"] = "-O3 -Bstatic -Bdynamic -std=c++17 " + env.get( + "CXXFLAGS", "" + ) env["LDFLAGS"] = env.get("LDFLAGS", "") + f" -Wl,-rpath,{_rpath}" env["ORIGIN"] = "$ORIGIN" # if evaluated, it will still be '$ORIGIN' @@ -109,20 +134,32 @@ def build_extensions(self): subprocess.run(["cat", "config.log"], cwd=FASTJET, check=True) raise - env = os.environ.copy() - env["CXX"] = env.get("CXX", "g++") - env["LDFLAGS"] = env.get("LDFLAGS", "") + f" -Wl,-rpath,{_rpath}" env["ORIGIN"] = "$ORIGIN" # if evaluated, it will still be '$ORIGIN' subprocess.run(["make", "-j"], cwd=FASTJET, env=env, check=True) subprocess.run(["make", "install"], cwd=FASTJET, env=env, check=True) + # Update the environment for fastjet-contrib build + env = os.environ.copy() + env["CXX"] = env.get("CXX", "g++") + env["CXXFLAGS"] = "-O3 -Bstatic -Bdynamic -std=c++17 " + env.get( + "CXXFLAGS", "" + ) + env["LDFLAGS"] = env.get("LDFLAGS", "") + + # For aarch64 macOS need to set the LDFLAGS for Homebrew installed + # dependencies to be found. However, fastjet-contrib's configure + # script does not use/accept LDFLAGS as an argument, and so to get + # the library search path options passed to the linker it is necessary + # to improperly inject them into the CXXFLAGS (which are used). + # This is a bad hack, and will be alleviated if CMake can be used + # by FastJet and FastJet-contrib. + # c.f. https://github.com/scikit-hep/fastjet/issues/310 subprocess.run( [ "./configure", f"--fastjet-config={FASTJET}/fastjet-config", f'CXX={env["CXX"]}', - "CXXFLAGS=-O3 -Bstatic -Bdynamic -std=c++17", - f'LDFLAGS={env["LDFLAGS"]}', + f'CXXFLAGS={env["CXXFLAGS"]}{" "+env["LDFLAGS"] if sys.platform == "darwin" else ""}', ], cwd=FASTJET_CONTRIB, env=env, @@ -151,10 +188,7 @@ def run(self): shutil.copytree(OUTPUT, fastjetdir / "_fastjet_core", symlinks=True) - make = "make" - if sys.platform == "darwin": - make = "gmake" - + make = "gmake" if sys.platform == "darwin" else "make" pythondir = pathlib.Path( subprocess.check_output( f"""{make} -f Makefile --eval='print-pythondir: From 65f9cc9bd146790d98054390fdb2782173232e82 Mon Sep 17 00:00:00 2001 From: Matthew Feickert Date: Sun, 15 Sep 2024 22:15:32 -0500 Subject: [PATCH 2/7] ci: Add aarch64 macOS to CI matrix * Add 'macos-latest' runners which are macos-14 aarch64 machines. * Add libtool to brew install list. --- .github/workflows/ci.yml | 11 ++++------- 1 file changed, 4 insertions(+), 7 deletions(-) diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index 488c359..59e9c27 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -34,7 +34,7 @@ jobs: fail-fast: false matrix: python-version: ["3.8", "3.12"] - runs-on: [ubuntu-latest, macos-13] + runs-on: [ubuntu-latest, macos-13, macos-latest] arch: [auto64] steps: - uses: actions/checkout@v4 @@ -48,8 +48,7 @@ jobs: - name: Install compiler tools on macOS if: runner.os == 'macOS' run: | - brew install make automake swig gmp mpfr boost - export PATH="/usr/local/opt/make/libexec/gnubin:$PATH" + brew install make automake swig gmp mpfr boost libtool - name: Install extra deps on Linux if: runner.os == 'Linux' @@ -57,7 +56,6 @@ jobs: - name: Install package run: | - echo $PATH python -m pip install '.[test]' -v - name: Test package @@ -69,7 +67,7 @@ jobs: strategy: fail-fast: false matrix: - os: [ubuntu-latest, macos-13] + os: [ubuntu-latest, macos-13, macos-latest] python: [312] arch: [auto64] @@ -86,8 +84,7 @@ jobs: - name: Install compiler tools on macOS if: runner.os == 'macOS' run: | - brew install make automake swig mpfr boost - export PATH="/usr/local/opt/make/libexec/gnubin:$PATH" + brew install make automake swig mpfr boost libtool - name: Clone gmp if: runner.os == 'macOS' From 700458b74b0fc98c6f09eced29582349a1d83fa1 Mon Sep 17 00:00:00 2001 From: Matthew Feickert Date: Sun, 15 Sep 2024 22:19:35 -0500 Subject: [PATCH 3/7] ci: Add aarch64 macOS wheel builds * Add macos-14 runners for auto64 archs. * Add libtool to brew install list. --- .github/workflows/wheels.yml | 5 ++--- 1 file changed, 2 insertions(+), 3 deletions(-) diff --git a/.github/workflows/wheels.yml b/.github/workflows/wheels.yml index ece5d47..4a96d71 100644 --- a/.github/workflows/wheels.yml +++ b/.github/workflows/wheels.yml @@ -30,7 +30,7 @@ jobs: fail-fast: false matrix: python: [38, 39, 310, 311, 312] - os: [ubuntu-latest, macos-13] + os: [ubuntu-latest, macos-13, macos-14] arch: [auto64] include: - python: 38 @@ -62,8 +62,7 @@ jobs: - name: Install compiler tools on macOS if: runner.os == 'macOS' run: | - brew install make automake swig mpfr boost - export PATH="/usr/local/opt/make/libexec/gnubin:$PATH" + brew install make automake swig mpfr boost libtool - name: Clone gmp if: runner.os == 'macOS' From e600b6672fbd0a47874ea16949a38f8245db0e22 Mon Sep 17 00:00:00 2001 From: Matthew Feickert Date: Mon, 16 Sep 2024 18:07:37 -0500 Subject: [PATCH 4/7] fix: remove enable-cgal From ./configure --help: ``` --enable-cgal enables link with the CGAL library default=no --enable-cgal-header-only enable build with header-only install of CGAL, e.g. as for CGALv5; in that case do not use --enable-cgal default=no ``` --- setup.py | 1 - 1 file changed, 1 deletion(-) diff --git a/setup.py b/setup.py index d61790c..25fe24c 100644 --- a/setup.py +++ b/setup.py @@ -116,7 +116,6 @@ def build_extensions(self): "--disable-auto-ptr", "--enable-allcxxplugins", "--enable-cgal-header-only", - "--enable-cgal", f"--with-cgaldir={cgal_dir}", "--enable-swig", "--enable-pyext", From e905d598944e1bcf9960b268b540daa727ef59c8 Mon Sep 17 00:00:00 2001 From: Matthew Feickert Date: Mon, 16 Sep 2024 18:38:27 -0500 Subject: [PATCH 5/7] Try to get building without extra env copy --- setup.py | 10 +--------- 1 file changed, 1 insertion(+), 9 deletions(-) diff --git a/setup.py b/setup.py index 25fe24c..2554a8f 100644 --- a/setup.py +++ b/setup.py @@ -104,6 +104,7 @@ def build_extensions(self): env = os.environ.copy() env["PYTHON"] = sys.executable env["PYTHON_INCLUDE"] = f'-I{sysconfig.get_path("include")}' + env["CXX"] = env.get("CXX", "g++") env["CXXFLAGS"] = "-O3 -Bstatic -Bdynamic -std=c++17 " + env.get( "CXXFLAGS", "" ) @@ -133,18 +134,9 @@ def build_extensions(self): subprocess.run(["cat", "config.log"], cwd=FASTJET, check=True) raise - env["ORIGIN"] = "$ORIGIN" # if evaluated, it will still be '$ORIGIN' subprocess.run(["make", "-j"], cwd=FASTJET, env=env, check=True) subprocess.run(["make", "install"], cwd=FASTJET, env=env, check=True) - # Update the environment for fastjet-contrib build - env = os.environ.copy() - env["CXX"] = env.get("CXX", "g++") - env["CXXFLAGS"] = "-O3 -Bstatic -Bdynamic -std=c++17 " + env.get( - "CXXFLAGS", "" - ) - env["LDFLAGS"] = env.get("LDFLAGS", "") - # For aarch64 macOS need to set the LDFLAGS for Homebrew installed # dependencies to be found. However, fastjet-contrib's configure # script does not use/accept LDFLAGS as an argument, and so to get From f188ffdc80f0570dd1472e5c18f34ed28d4d713f Mon Sep 17 00:00:00 2001 From: Matthew Feickert Date: Tue, 17 Sep 2024 02:37:22 -0500 Subject: [PATCH 6/7] Remove manipulation of CXXFLAGS and LDFLAGS --- setup.py | 33 +-------------------------------- 1 file changed, 1 insertion(+), 32 deletions(-) diff --git a/setup.py b/setup.py index 2554a8f..45713ba 100644 --- a/setup.py +++ b/setup.py @@ -11,7 +11,6 @@ import os import pathlib -import platform import shutil import subprocess import sys @@ -74,28 +73,6 @@ def build_extensions(self): cwd=FASTJET, ) - # Inject the required CXXFLAGS and LDFLAGS for building on - # aarch64 macOS. As Homebrew can not be used at any part in a - # conda-forge build, guard against this by checking for the - # conda-build environment variable. - # This is a bad hack, and will be alleviated if CMake can be used - # by FastJet and FastJet-contrib. - # c.f. https://github.com/scikit-hep/fastjet/issues/310 - if ( - sys.platform == "darwin" - and platform.processor() == "arm" - and "HOMEBREW_PREFIX" in os.environ - and "CONDA_BUILD" not in os.environ - ): - os.environ["CXXFLAGS"] = ( - os.environ.get("CXXFLAGS", "") - + f" -I{os.environ['HOMEBREW_PREFIX']}/include" - ) - os.environ["LDFLAGS"] = ( - os.environ.get("LDFLAGS", "") - + f" -L{os.environ['HOMEBREW_PREFIX']}/lib" - ) - # RPATH is set for shared libraries in the following locations: # * fastjet/ # * fastjet/_fastjet_core/lib/ @@ -137,20 +114,12 @@ def build_extensions(self): subprocess.run(["make", "-j"], cwd=FASTJET, env=env, check=True) subprocess.run(["make", "install"], cwd=FASTJET, env=env, check=True) - # For aarch64 macOS need to set the LDFLAGS for Homebrew installed - # dependencies to be found. However, fastjet-contrib's configure - # script does not use/accept LDFLAGS as an argument, and so to get - # the library search path options passed to the linker it is necessary - # to improperly inject them into the CXXFLAGS (which are used). - # This is a bad hack, and will be alleviated if CMake can be used - # by FastJet and FastJet-contrib. - # c.f. https://github.com/scikit-hep/fastjet/issues/310 subprocess.run( [ "./configure", f"--fastjet-config={FASTJET}/fastjet-config", f'CXX={env["CXX"]}', - f'CXXFLAGS={env["CXXFLAGS"]}{" "+env["LDFLAGS"] if sys.platform == "darwin" else ""}', + f'CXXFLAGS={env["CXXFLAGS"]}', ], cwd=FASTJET_CONTRIB, env=env, From bc648c2101a57d4041ed13c004ab6eeec89551d0 Mon Sep 17 00:00:00 2001 From: Matthew Feickert Date: Tue, 17 Sep 2024 15:03:38 -0500 Subject: [PATCH 7/7] Revert "fix: remove enable-cgal" This reverts commit e600b6672fbd0a47874ea16949a38f8245db0e22. --- setup.py | 1 + 1 file changed, 1 insertion(+) diff --git a/setup.py b/setup.py index 45713ba..ccb1172 100644 --- a/setup.py +++ b/setup.py @@ -94,6 +94,7 @@ def build_extensions(self): "--disable-auto-ptr", "--enable-allcxxplugins", "--enable-cgal-header-only", + "--enable-cgal", f"--with-cgaldir={cgal_dir}", "--enable-swig", "--enable-pyext",