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

Kluge to get the gfortran linker to work correctly for SciPy on Big Sur. #20367

Merged
merged 4 commits into from
Dec 15, 2020

Conversation

benc303
Copy link
Contributor

@benc303 benc303 commented Dec 14, 2020

On Big Sur, gcc can’t detect the macOS version correctly. This causes a linker invocation with gfortran to try to link to libgcc_s.10.4, which isn’t present on the system. This apparently only happens when gfortran is used to link a .so library, so it doesn’t affect SciPy’s dependencies: OpenBLAS uses gfortran to link, but is producing a .dylib; NumPy uses clang to link. It looks like the SciPy build system injects the -mmacosx-version-min flag (in _build_utils/compiler_helper.py) for the C++ compiler, but not for the Fortran compiler.

This PR gets around that by setting the MACOSX_DEPLOYMENT_TARGET to 10.15 (Catalina) when on Big Sur. This is incorrect, since it causes gfortran to identify the OS as Catalina, but at least it’s close enough that it gets the linked library right. It looks like the gfortran behavior is updated for Big Sur in this commit to the gcc codebase, but that’s not available in a gcc release yet.

@benc303
Copy link
Contributor Author

benc303 commented Dec 14, 2020

I think the flake8 failure is a false positive, see PyCQA/pycodestyle#373.

@adamjstewart
Copy link
Member

It does indeed look like a false-positive. Can you remove the spacing around : for now? We don't yet use black, but we do use flake8, so luckily we don't need to please both.

@adamjstewart
Copy link
Member

Out of curiosity, what is the fix? Maybe we could backport that patch instead.

@benc303
Copy link
Contributor Author

benc303 commented Dec 15, 2020

Out of curiosity, what is the fix? Maybe we could backport that patch instead.

The fix to what? The SciPy package.py, or gcc?

@adamjstewart
Copy link
Member

Ah nvm, you linked to the fix in your initial comment. I wonder if we should be setting this globally for all packages that use the GCC compiler on macOS 11. This seems to be the only package that breaks so far, but if it's a GCC bug I imagine there will be many others. I'm actually surprised to see someone using GCC on macOS, as Python can't even be built with GCC on macOS. You might be the only user who encounters this problem. For now, I'm going to merge this.

@adamjstewart adamjstewart merged commit c02625e into spack:develop Dec 15, 2020
@benc303
Copy link
Contributor Author

benc303 commented Dec 15, 2020

I'm actually surprised to see someone using GCC on macOS, as Python can't even be built with GCC on macOS. You might be the only user who encounters this problem.

Isn’t GCC (the gcc package, not the compiler itself) commonly used to get a Fortran compiler on macOS? This is needed by a bunch of packages (OpenBLAS and any dependents, etc.). I installed gcc via Spack and then set up the compiler following step 5 of the getting started instructions. So I’m using clang for C/C++ and gfortran for Fortran.

@adamjstewart
Copy link
Member

Ah, I see. Yes, I'm using Apple Clang + GFortran too. I haven't hit this yet because my laptop is too old to update to Big Sur, but I'm sure this will become a big issue if it affects Fortran builds.

@benc303 benc303 deleted the bigSurPy branch December 15, 2020 20:59
bollig pushed a commit to bollig/spack that referenced this pull request Jan 12, 2021
…ur. (spack#20367)

* Kluge to get the gfortran linker to work correctly on Big Sur.

* Fixed formatting error; stetting the other.

* Removed spaces.

* Added comment, mainly to re-trigger Spack CI.
loulawrence pushed a commit to loulawrence/spack that referenced this pull request Jan 19, 2021
…ur. (spack#20367)

* Kluge to get the gfortran linker to work correctly on Big Sur.

* Fixed formatting error; stetting the other.

* Removed spaces.

* Added comment, mainly to re-trigger Spack CI.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants