Skip to content

Use gfortran 11.3.0 from repackaged conda tarball for x86_64 #11

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

Merged
merged 5 commits into from
Nov 9, 2022

Conversation

isuruf
Copy link
Contributor

@isuruf isuruf commented Nov 6, 2022

No description provided.

@isuruf
Copy link
Contributor Author

isuruf commented Nov 6, 2022

This code is untested

@andyfaff
Copy link

andyfaff commented Nov 7, 2022

@isuruf I've just tried using the changes made in this PR in andyfaff/scipy#38. Unfortunately the compiler doesn't seem to work. I tried to compile a test program and I get:

  + gfortran --version
  GNU Fortran (GCC) 11.3.0
  Copyright (C) 2021 Free Software Foundation, Inc.
  This is free software; see the source for copying conditions.  There is NO
  warranty; not even for MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.
  
  + gfortran tools/wheels/test.f
  ld: library not found for -lm
  collect2: error: ld returned 1 exit status

The test run is here.

@isuruf
Copy link
Contributor Author

isuruf commented Nov 7, 2022

For the compiler to work, you need to set SDKROOT like you do for arm64 in https://github.com/scipy/scipy/blob/4b73d5e7150e05ac66baef19c6ddffc5ec4b3748/.github/workflows/wheels.yml#L175

@rgommers
Copy link

rgommers commented Nov 7, 2022

Thanks @isuruf. Using gfortran 11.3.0 from conda-forge sounds great to me. Compiler version wise I'm not aware of any issues, and I've been using it locally on arm64 macOS for a little while.

@isuruf isuruf marked this pull request as ready for review November 8, 2022 00:50
@isuruf
Copy link
Contributor Author

isuruf commented Nov 8, 2022

This is ready. The compilers here were tested by @andyfaff in the scipy CI and they work.

@andyfaff
Copy link

andyfaff commented Nov 8, 2022

Note, I used the x86_64-native and arm64-cross, I haven't tried the other two.

@mattip
Copy link
Contributor

mattip commented Nov 8, 2022

Why didn't CI run?

@isuruf
Copy link
Contributor Author

isuruf commented Nov 8, 2022

CI hasn't run in a while because the CI here used travis-ci.org and it was shut down ~6 months ago. We can move to travis-ci.com, but you'll run out of credits quickly.

@mattip
Copy link
Contributor

mattip commented Nov 9, 2022

@isuruf: I think this should force the MACOSX_DEPLOYMENT_TARGET (in get_distutils_platform) to 10_9:

diff --git a/gfortran_utils.sh b/gfortran_utils.sh
index d2ac55f..8cd7573 100644
--- a/gfortran_utils.sh
+++ b/gfortran_utils.sh
@@ -25,9 +25,8 @@ function get_distutils_platform {
         echo "manylinux1_$plat"
         return
     fi
-    # macOS 32-bit arch is i386
-    [ "$plat" == "i686" ] && plat="i386"
-    local target=$(echo $MACOSX_DEPLOYMENT_TARGET | tr .- _)
+    # The gfortran downloads build for macos 10.9
+    local target="10_9"
     echo "macosx_${target}_${plat}"
 }
 
@@ -51,9 +50,8 @@ function get_distutils_platform_ex {
         echo "manylinux${mb_ml_ver}_${plat}"
         return
     fi
-    # macOS 32-bit arch is i386
-    [ "$plat" == "i686" ] && plat="i386"
-    local target=$(echo $MACOSX_DEPLOYMENT_TARGET | tr .- _)
+    # The gfortran downloads build for macos 10.9
+    local target="10_9"
     echo "macosx_${target}_${plat}"
 }

@isuruf
Copy link
Contributor Author

isuruf commented Nov 9, 2022

@mattip, we shouldn't force the target for other projects. It should be a per project setting.

@mattip
Copy link
Contributor

mattip commented Nov 9, 2022

Maybe then a comment that 10.9 is supported, or a check that the value is at least 10.9?

@isuruf
Copy link
Contributor Author

isuruf commented Nov 9, 2022

Yes, checking that the value is at least 10.9 is a good idea.

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.

5 participants