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

linear_algebra PG: update options and BLIS variant; NTPoly: revbump to ensure correct linking #23395

Closed
wants to merge 3 commits into from

Conversation

barracuda156
Copy link
Contributor

Description

Update PG and NTPoly port to ensure correct linking.

Type(s)
  • bugfix
  • enhancement
  • security fix
Tested on

macOS 14.4.1
Xcode 15.3

Verification

Have you

  • followed our Commit Message Guidelines?
  • squashed and minimized your commits?
  • checked that there aren't other open pull requests for the same change?
  • referenced existing tickets on Trac with full URL in commit message?
  • checked your Portfile with port lint --nitpick?
  • tried existing tests with sudo port test?
  • tried a full install with sudo port -vst install?
  • tested basic functionality of all binary files?
  • checked that the Portfile's most important variants haven't been broken?

@barracuda156
Copy link
Contributor Author

@szhorvat @catap What was the conclusion re MPICH on macOS 13? Apparently it is again (or still?) broken:

      /opt/local/bin/mpicc-mpich-mp -pipe -Os -DNDEBUG -I/opt/local/include -isysroot/Library/Developer/CommandLineTools/SDKs/MacOSX13.sdk  -arch x86_64 -isysroot /Library/Developer/CommandLineTools/SDKs/MacOSX13.sdk -mmacosx-version-min=13.0 -Wl,-search_paths_first -Wl,-headerpad_max_install_names -L/opt/local/lib -Wl,-headerpad_max_install_names -Wl,-rpath,/opt/local/lib/libgcc -Wl,-ld_classic -Wl,-syslibroot,/Library/Developer/CommandLineTools/SDKs/MacOSX13.sdk  CMakeFiles/cmTC_d5a3a.dir/testCCompiler.c.o -o cmTC_d5a3a
      ld: library not found for -ld_classic
      clang: error: linker command failed with exit code 1 (use -v to see invocation)
      make[1]: *** [cmTC_d5a3a] Error 1
      make: *** [cmTC_d5a3a/fast] Error 2

@szhorvat
Copy link
Contributor

szhorvat commented Apr 7, 2024

I don't really use MPICH (often had trouble building in the past), and I no longer have macOS 13, so unfortunately I have no input here.

On macOS 14 with Xcode 15.3 and Apple clang version 15.0.0 (clang-1500.3.9.4) the -Wl,-ld_classic option still works.

@barracuda156
Copy link
Contributor Author

I don't really use MPICH (often had trouble building in the past), and I no longer have macOS 13, so unfortunately I have no input here.

On macOS 14 with Xcode 15.3 and Apple clang version 15.0.0 (clang-1500.3.9.4) the -Wl,-ld_classic option still works.

@szhorvat Yes, it is still needed on macOS 14, but should be removed for macOS 13. (At least going by results of CI on either. Locally I can only check macOS 14.)

@barracuda156
Copy link
Contributor Author

@reneeotten @pmetzger Unless this PR is merged, we won’t be able to update any MPICH ports, since CI will fail on macOS 13.
Example: #23458

@pmetzger
Copy link
Member

Can you explain what the change to the portgroup does?

@barracuda156
Copy link
Contributor Author

@pmetzger The earlier version did not work in a way I thought it was.

What we need is to make sure that the selected BLAS is linked to, even when multiple implementations are available. And we need to be able to override PG defaults when needed. And when that is done, it should actually link to the correct BLAS.

With the earlier version, for example, I had NTPoly claiming +openblas, but in fact linked to FlexiBLAS. I only noticed that when I deactivated FlexiBLAS, and got a broken port.

This one seems to work correctly.

@barracuda156
Copy link
Contributor Author

barracuda156 commented Apr 14, 2024

@pmetzger @reneeotten Maybe this can be merged? It is needed for other ports too.

UPD. Now this cannot be updated too: #23510

@barracuda156 barracuda156 mentioned this pull request Apr 15, 2024
12 tasks
@ryandesign
Copy link
Contributor

Yes, it is still needed on macOS 14, but should be removed for macOS 13. (At least going by results of CI on either. Locally I can only check macOS 14.)

It seems highly unlikely that the need for this flag would be based on the OS version. Surely it is based entirely on whether you are using the Xcode 15 or later linker.

The condition in the portgroup was: if Xcode 15 or later or the Xcode 15 command line tools or later are installed, then use -ld_classic.

On the macOS 13 GitHub Actions runner, according to CI logs, it has Xcode 15.0.1, CLT 14.3.1.0.1.1683849156. Therefore the condition was satisfied. The error probably occurred because the particular port was using the CLT, not Xcode.

So, the condition needs to be changed to: if use_xcode is yes and the Xcode version is 15 or greater, or use_xcode is no and the Xcode CLT version is 15 or greater, then use -ld_classic.

@ryandesign
Copy link
Contributor

It looks like the same problematic condition was copy/pasted into at least the gcc-devel, gcc11, gcc12, gcc13, sundials5, and root6 portfiles.

@ryandesign
Copy link
Contributor

I am not certain that checking use_xcode is the correct solution because if for example a port does does not set use_xcode yes but the user does not have the CLT installed then Xcode will be used. We need to check the version of whatever is being used—Xcode or the CLT—and I don't know what variable in MacPorts represents that fact.

@barracuda156
Copy link
Contributor Author

I am not certain that checking use_xcode is the correct solution because if for example a port does does not set use_xcode yes but the user does not have the CLT installed then Xcode will be used. We need to check the version of whatever is being used—Xcode or the CLT—and I don't know what variable in MacPorts represents that fact.

@ryandesign To be honest I do not know how that Xcode checks work, and I can set that however it is appropriate, if someone can instruct me.

What we need in result is that ld_classic should not be used anymore on macOS 13. Otherwise we cannot update anything that uses MPICH, and that means a lot.

@barracuda156
Copy link
Contributor Author

Yes, it is still needed on macOS 14, but should be removed for macOS 13. (At least going by results of CI on either. Locally I can only check macOS 14.)

It seems highly unlikely that the need for this flag would be based on the OS version. Surely it is based entirely on whether you are using the Xcode 15 or later linker.

The condition in the portgroup was: if Xcode 15 or later or the Xcode 15 command line tools or later are installed, then use -ld_classic.

On the macOS 13 GitHub Actions runner, according to CI logs, it has Xcode 15.0.1, CLT 14.3.1.0.1.1683849156. Therefore the condition was satisfied. The error probably occurred because the particular port was using the CLT, not Xcode.

So, the condition needs to be changed to: if use_xcode is yes and the Xcode version is 15 or greater, or use_xcode is no and the Xcode CLT version is 15 or greater, then use -ld_classic.

Well, does anyone have macOS 13 locally to confirm, what is the case? On CI now everything consistently fails, as long as ld_classic is used. Apple was supposed to remove it – perhaps it did?

@catap Kirill, could you actually test this? I think you had macOS 13. Otherwise we are in trouble, updates and, yet worse, fixes are blocked.

@catap
Copy link
Contributor

catap commented Apr 16, 2024 via email

@barracuda156
Copy link
Contributor Author

To illustrate what happens now, I am trying to upgrade mfem port, which is installed as mfem @4.6+accelerate+gcc13:

36-142% sudo port -v upgrade mfem
Password:
Warning: configured user/group macports does not exist, will build as root
--->  Computing dependencies for mfem.
--->  Fetching distfiles for mfem
--->  Verifying checksums for mfem
--->  Checksumming mfem-4.6.tar.gz
--->  Extracting mfem
--->  Extracting mfem-4.6.tar.gz
Executing:  cd "/opt/local/var/macports/build/_opt_local_var_macports_sources_rsync.macports.org_macports_release_tarballs_ports_science_mfem/mfem/work" && /usr/bin/gzip -dc '/opt/local/var/macports/distfiles/mfem/mfem-4.6.tar.gz' | /usr/bin/gnutar --no-same-owner -xf - 
--->  Applying patches to mfem
--->  Applying patch-cstdint.diff
Executing:  cd "/opt/local/var/macports/build/_opt_local_var_macports_sources_rsync.macports.org_macports_release_tarballs_ports_science_mfem/mfem/work/mfem-4.6" && /usr/bin/patch -p0 < '/opt/local/var/macports/sources/rsync.macports.org/macports/release/tarballs/ports/science/mfem/files/patch-cstdint.diff'
patching file general/kdtree.hpp
--->  Configuring mfem
Executing:  cd "/opt/local/var/macports/build/_opt_local_var_macports_sources_rsync.macports.org_macports_release_tarballs_ports_science_mfem/mfem/work/build" && /opt/local/bin/cmake -G "CodeBlocks - Unix Makefiles" -DCMAKE_BUILD_TYPE=MacPorts -DCMAKE_INSTALL_PREFIX="/opt/local" -DCMAKE_INSTALL_NAME_DIR="/opt/local/lib" -DCMAKE_SYSTEM_PREFIX_PATH="/opt/local;/usr" -DCMAKE_C_COMPILER="$CC" -DCMAKE_CXX_COMPILER="$CXX" -DCMAKE_OBJC_COMPILER="$CC" -DCMAKE_OBJCXX_COMPILER="$CXX" -DCMAKE_POLICY_DEFAULT_CMP0025=NEW -DCMAKE_POLICY_DEFAULT_CMP0060=NEW -DCMAKE_VERBOSE_MAKEFILE=ON -DCMAKE_COLOR_MAKEFILE=ON -DCMAKE_FIND_FRAMEWORK=LAST -DCMAKE_EXPORT_COMPILE_COMMANDS=ON -DCMAKE_MAKE_PROGRAM=/usr/bin/make -DCMAKE_MODULE_PATH="/opt/local/share/cmake/Modules" -DCMAKE_PREFIX_PATH="/opt/local/share/cmake/Modules" -DCMAKE_BUILD_WITH_INSTALL_RPATH:BOOL=ON -DCMAKE_INSTALL_RPATH="/opt/local/lib" -Wno-dev -DBUILD_SHARED_LIBS=ON -DGinkgo_DIR=/opt/local/lib -DHDF5_DIR=/opt/local -DHDF5_LIBRARIES=/opt/local/lib -DHDF5_INCLUDE_DIRS=/opt/local/lib -DMFEM_ENABLE_EXAMPLES=ON -DMFEM_ENABLE_MINIAPPS=ON -DMFEM_ENABLE_TESTING=ON -DMFEM_USE_CUDA=OFF -DMFEM_USE_GINKGO=OFF -DMFEM_USE_GNUTLS=ON -DMFEM_USE_HIOP=OFF -DMFEM_USE_HIP=OFF -DMFEM_USE_LAPACK=ON -DMFEM_USE_LIBUNWIND=OFF -DMFEM_USE_MEMALLOC=ON -DMFEM_USE_METIS=OFF -DMFEM_USE_METIS_5=OFF -DMFEM_USE_MPFR=ON -DMFEM_USE_MPI=OFF -DMFEM_USE_MUMPS=OFF -DMFEM_USE_NETCDF=ON -DMFEM_USE_OCCA=OFF -DMFEM_USE_OPENMP=ON -DMFEM_USE_RAJA=OFF -DMFEM_USE_SIMD=OFF -DMFEM_USE_STRUMPACK=OFF -DMFEM_USE_SUITESPARSE=OFF -DMFEM_USE_SUNDIALS=OFF -DMFEM_USE_SUPERLU=OFF -DMFEM_USE_ZLIB=ON -DVERBOSE=ON -DCMAKE_OSX_ARCHITECTURES="ppc" -DCMAKE_OSX_DEPLOYMENT_TARGET="10.6" -DCMAKE_OSX_SYSROOT="/" /opt/local/var/macports/build/_opt_local_var_macports_sources_rsync.macports.org_macports_release_tarballs_ports_science_mfem/mfem/work/mfem-4.6 
-- CMake version: 3.29.2
-- Loading USER_CONFIG = /opt/local/var/macports/build/_opt_local_var_macports_sources_rsync.macports.org_macports_release_tarballs_ports_science_mfem/mfem/work/mfem-4.6/config/user.cmake (NOTFOUND)
-- USE_XSDK_DEFAULTS = 'OFF'
-- The CXX compiler identification is GNU 13.2.0
-- Checking whether CXX compiler has -isysroot
-- Checking whether CXX compiler has -isysroot - yes
-- Checking whether CXX compiler supports OSX deployment target flag
-- Checking whether CXX compiler supports OSX deployment target flag - yes
-- Detecting CXX compiler ABI info
-- Detecting CXX compiler ABI info - done
-- Check for working CXX compiler: /opt/local/bin/g++-mp-13 - skipped
-- Detecting CXX compile features
-- Detecting CXX compile features - done
-- Found OpenMP_CXX: -fopenmp (found version "4.5")
-- Found OpenMP: TRUE (found version "4.5")
-- Found ZLIB: /opt/local/lib/libz.dylib (found version "1.3.1")
-- Looking for sgemm_
-- Looking for sgemm_ - not found
-- Performing Test CMAKE_HAVE_LIBC_PTHREAD
-- Performing Test CMAKE_HAVE_LIBC_PTHREAD - Success
-- Found Threads: TRUE
-- Looking for sgemm_
-- Looking for sgemm_ - found
-- Found BLAS: /opt/local/lib/libflexiblas.dylib
-- Looking for cheev_
-- Looking for cheev_ - found
-- Found LAPACK: /opt/local/lib/libflexiblas.dylib;-lm;-ldl
-- Looking for _GnuTLS ...
-- Found _GnuTLS: /opt/local/lib/libgnutls.dylib
-- GNUTLS_INCLUDE_DIRS=/opt/local/include
-- The C compiler identification is GNU 13.2.0
-- Checking whether C compiler has -isysroot
-- Checking whether C compiler has -isysroot - yes
-- Checking whether C compiler supports OSX deployment target flag
-- Checking whether C compiler supports OSX deployment target flag - yes
-- Detecting C compiler ABI info
-- Detecting C compiler ABI info - done
-- Check for working C compiler: /opt/local/bin/gcc-mp-13 - skipped
-- Detecting C compile features
-- Detecting C compile features - done
-- Looking for NetCDF ...
-- NetCDF: looking for required package: HDF5, COMPONENTS: C;HL
-- Looking for HDF5 ...
--    in HDF5_DIR = /opt/local
-- Found HDF5: /opt/local/lib/libhdf5_hl.dylib;/opt/local/lib/libhdf5.dylib
-- Found NetCDF: /opt/local/lib/libnetcdf.dylib;/opt/local/lib/libhdf5_hl.dylib;/opt/local/lib/libhdf5.dylib
-- NETCDF_INCLUDE_DIRS=/opt/local/include;/opt/local/lib
-- Looking for MPFR ...
--    checking library: mpfr
-- Found MPFR: mpfr
-- MFEM: using package OPENMP
-- MFEM: using package LAPACK
-- MFEM: using package BLAS
-- MFEM: using package GNUTLS
-- MFEM: using package NETCDF
-- MFEM: using package MPFR
-- MFEM: using package ZLIB
-- MFEM shared library: BUILD_SHARED_LIBS = ON
-- MFEM build type: CMAKE_BUILD_TYPE = MacPorts
-- MFEM version: v4.6.0
-- MFEM git string: (unknown)
-- TPL_INCLUDE_DIRS = /opt/local/include;/opt/local/lib
-- Writing substitute header --> "mfem.hpp"
-- Writing substitute header --> "mfem-performance.hpp"
-- performance_ex1: add flags "-mcpu=native -mtune=native -Wall -pedantic --param max-completely-peel-times=3"
This miniapp requires MPI to be enabled.Please compile with MFEM_USE_MPI.
-- Found Doxygen: /opt/local/bin/doxygen (found version "1.9.8") found components: doxygen dot
-- CMAKE_INSTALL_PREFIX = /opt/local
-- Configuring done (12.0s)
-- Generating done (2.0s)

It opportunistically picks flexiBLAS, which is nowhere asked for. This is a real problem that has to be resolved.

@pmetzger
Copy link
Member

This PR appears to be stalled. What can be done to move it forward that does not involve convincing people who don't want to work on something to work on it?

@barracuda156
Copy link
Contributor Author

barracuda156 commented Jun 12, 2024

@pmetzger What would you like to be changed? It should work fine as is.

We can remove MPICH part, that can be done separately. I am more concerned about BLAS issue.

@pmetzger
Copy link
Member

@barracuda156 I will want to hear that @ryandesign and @reneeotten are satisfied.

@barracuda156
Copy link
Contributor Author

@pmetzger Well, @ryandesign issue was about ld_classic, which is the MPICH issue. I do not insist on that, since while it fixes the problem, I do not really care about macOS-13 personally. Let me just remove that commit from here, and whoever will be interested to fix it later could deal with it.

I do not get why @reneeotten was unhappy about a comment in a port group. It certainly cannot hurt anyone to have it there.

Re BLAS, let me run a few builds once again to make sure the problem does not show up, that I will do today soon.

@barracuda156 barracuda156 marked this pull request as draft June 12, 2024 13:48
@barracuda156
Copy link
Contributor Author

I close this for a bit, needs more testing. Gonna make a new PR in a couple of days.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
7 participants