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

Fortran, cmake based formulae from source w/ Intel ifort installed causes problems #4293

Closed
5 tasks done
zbeekman opened this issue Jun 6, 2018 · 20 comments
Closed
5 tasks done
Labels
outdated PR was locked due to age stale No recent activity

Comments

@zbeekman
Copy link
Contributor

zbeekman commented Jun 6, 2018

Please note:

  1. I am not sure if a bug report or a feature request best describes the issue I'm experiencing; I used a bug report template and included all info below.
  2. I know a simple work-around exists: Unlink Intel's ifort so it's not in the PATH when homebrew is performing builds; however, in the interest of avoiding future confusion with users and simplifying/unifying homebrew packages, formulae & maintenance I think it's worth filing a bug report/feature request.

TL;DR:
Please consider setting the FC environment variable to point to a brewed gfortran inside the build environment; without it CMake based builds can stupidly pick up ifort (and other compilers too, probably), happily building your Fortran formula, but then leading to much confusion down the road when you try to USE module_from_brewed_formula with brewed gfortran at a later date.

Requested bug report template info:

  • are reporting a bug others will be able to reproduce and not asking a question. If you're not sure or want to ask a question do so on our Discourse: https://discourse.brew.sh
  • ran a brew command and reproduced the problem with multiple formulae? If it's a problem with a single, official formula (not cask) please file this issue at Homebrew/homebrew-core: https://github.com/Homebrew/homebrew-core/issues/new/choose. If it's a brew cask problem please file this issue at https://github.com/Homebrew/homebrew-cask/issues/new/choose. If it's a tap (e.g. Homebrew/homebrew-php) problem please file this issue at the tap.
  • ran brew update and can still reproduce the problem?
  • ran brew doctor, fixed all issues and can still reproduce the problem?
  • ran brew config and brew doctor and included their output with your issue?

Brew config:

$ brew config
HOMEBREW_VERSION: 1.6.7-4-g9c1a988
ORIGIN: https://github.com/Homebrew/brew
HEAD: 9c1a988c326ba20f1b8a06b4b4f6af11b34722a6
Last commit: 72 minutes ago
Core tap ORIGIN: https://github.com/Homebrew/homebrew-core
Core tap HEAD: a930ced7bc7f391f9d488d904254f9a9ace1db04
Core tap last commit: 3 hours ago
HOMEBREW_PREFIX: /usr/local
HOMEBREW_DEV_CMD_RUN: 1
CPU: quad-core 64-bit haswell
Homebrew Ruby: 2.3.3 => /usr/local/Homebrew/Library/Homebrew/vendor/portable-ruby/2.3.3_2/bin/ruby
Clang: 9.1 build 902
Git: 2.17.1 => /usr/local/bin/git
Curl: 7.54.0 => /usr/bin/curl
Java: 10.0.1, 1.8.0_172, 1.8.0_152, 1.6.0_65-b14-468
macOS: 10.13.5-x86_64
CLT: 10.0.0.0.1.1527767617
Xcode: 9.4
XQuartz: 2.7.11 => /opt/X11

Brew doctor:

$ brew doctor
Please note that these warnings are just used to help the Homebrew maintainers
with debugging if you file an issue. If everything you use Homebrew for is
working fine: please don't worry or file an issue; just ignore this. Thanks!

Warning: The filesystem on / appears to be case-sensitive.
The default macOS filesystem is case-insensitive. Please report any apparent problems. # I'm not changing my FS for a homebrew bug report

Warning: You have unlinked kegs in your Cellar
Leaving kegs unlinked can lead to build-trouble and cause brews that depend on
those kegs to fail to run properly once built. Run `brew link` on these:
  emacs # Not linking b/c I have a cask linked
  markdown # Not linking due to conflict
  mpich # Not linking due to conflict w/ open-mpi

To help us debug your issue please explain:

  • What you were trying to do (and why)

Installing a formula with a dependency on Fortran that uses CMake and installs fortran .mod module files, e.g. json-fortran

  • What happened (include command output)

The install succeeded, however, even when FC=/usr/local/bin/gfortran-8 the Fortran compiler picked was ifort because I have intel-parallel-studio installed and linked (installed to /usr/local/Cellar/intel-parallel-studio). Note: this behavior is specific to CMake, AFAICT. The solution is to either import and respect $FC into the super-env or intelligently set $FC to point to Homebrewed GCC's gfortran in the superenv. This will ensure CMake doesn't pick up and prefer ifort over Homebrew's gfortran. IMO, this it is a bug that CMake defaults to ifort over gfortran, but it is what it is. The output indicating that we will run into problems in the future is:

-- Checking whether /usr/local/bin/ifort supports Fortran 90
-- Checking whether /usr/local/bin/ifort supports Fortran 90 -- yes
  • What you expected to happen

Homebrew sets FC=/path/to/brewed/gfortran in the superenv so that CMake doesn't stupidly pick up ifort OR (less preferred) homebrew respects (imports) the users $FC environment variable. (The latter option is probably pretty error prone and dumb so I'm ruling it out.)

  • Step-by-step reproduction instructions (by running brew commands)
  1. Install an Intel Fortran compiler into /usr/local/Cellar/intel-parallel-studio/2019.0.0 (you can sign up to be a beta tester and get a beta trial version license until they release the production version)
  2. brew link intel-parallel-studio
  3. Build a fortran formula from source that provides a module file and uses CMake as the build system: e.g.,
$ FC=$(type -P gfortran-8) brew install --build-from-source --verbose json-fortran
/usr/bin/sandbox-exec -f /tmp/homebrew20180606-3992-10vi3su.sb nice /usr/local/Homebrew/Library/Homebrew/vendor/portable-ruby/2.3.3_2/bin/ruby -W0 -I /usr/local/Homebrew/Library/Homebrew -- /usr/local/Homebrew/Library/Homebrew/build.rb /usr/local/Homebrew/Library/Taps/homebrew/homebrew-core/Formula/json-fortran.rb --verbose
==> Downloading https://github.com/jacobwilliams/json-fortran/archive/6.3.0.tar.gz
Already downloaded: /Users/ibeekman/Library/Caches/Homebrew/json-fortran-6.3.0.tar.gz
==> Verifying json-fortran-6.3.0.tar.gz checksum
tar xzf /Users/ibeekman/Library/Caches/Homebrew/json-fortran-6.3.0.tar.gz
==> cmake .. -DCMAKE_C_FLAGS_RELEASE=-DNDEBUG -DCMAKE_CXX_FLAGS_RELEASE=-DNDEBUG -DCMAKE_INSTALL_PREFIX=/usr/local/Cellar/json-fortran/6.3.0_1 -DCMAKE_BUILD_TYPE=Release -DCMAKE_FIND_FRAMEWORK=LAST -DCMAKE_VERBOSE_MAKEFILE=ON -Wno-dev -DUSE_GNU_INSTALL_CONVENTION:BOOL=TRUE
-- The Fortran compiler identification is Intel 19.0.0.20180317
-- Check for working Fortran compiler: /usr/local/bin/ifort
-- Check for working Fortran compiler: /usr/local/bin/ifort  -- works
-- Detecting Fortran compiler ABI info
-- Detecting Fortran compiler ABI info - done
-- Checking whether /usr/local/bin/ifort supports Fortran 90
-- Checking whether /usr/local/bin/ifort supports Fortran 90 -- yes
-- CMake build configuration for JSON-Fortran 6.3.0
-- Dynamically computing FORD output information...
-- Running FORD to dynamically compute documentation outputs, this could take a while...
^C
  1. After a successful install try compiling a dumb test program with GFortran that consumes one of the installed .mod files from the formula:
! usemod.f90
program usemod
use json_kinds
implicit none
print*, 'if this compiled then there is no problem'
end program
$ gfortran-8 -I /usr/local/include usemod.f90
f951: Fatal Error: Reading module 'json_kinds' at line 1 column 2: Unexpected EOF
compilation terminated.

Obviously, brew unlink intel-parallel-studio before building JSON-Fortran with Homebrew solves this issue. But since Fortran compilers are not in general inter-operable, it would be very nice to have Homebrew ensure that packages with depends_on "gcc" due to Fortran dependencies actually use gfortran for compilation. For CMake based packages this is not the case due to CMake's quirks.

Thanks for you time and for your great work as volunteers. If someone points me in the right direction I'll try to find time to work up a PR that will inject FC=/path/to/brewed/gfortran into the build environment.

Best,
Zaak

@ilovezfs
Copy link
Contributor

ilovezfs commented Jun 6, 2018

The formula is responsible for setting the desired FC. Does it not work to

ENV["FC"] = "/path/to/whichever"

at the beginning of def install?

@zbeekman
Copy link
Contributor Author

zbeekman commented Jun 6, 2018

@ilovezfs Sure, that would work. But AFAICT, most CMake + Fortran formulae assume that GFortran will be used, since it's the only Fortran compiler currently available in homebrew (unless someone added flang when I wasn't paying attention). The C compiler is always set as Clang by homebrew, why not exert the same level of control and predictability for Fortran?

If you show me an easy way to find all formulae with depends_on "gcc" and depends_on "cmake" => :build I can make a PR to homebrew-core that enforces GFortran for all of them.

But I really don't see any point in providing users/formula authors with that flexibility when you're already enforcing MPI => open-mpi and the C compiler as clang.

@ilovezfs
Copy link
Contributor

ilovezfs commented Jun 6, 2018

Unlike the superenv compiler shims, Fortran is going to have to be supplied by a specific dependency in the formula so it makes sense to handle that in the formula.

@zbeekman
Copy link
Contributor Author

zbeekman commented Jun 6, 2018

If the general consensus is that picking the right Fortran compiler is the formula's responsibility then I will defer to the judgement of the homebrew maintainers. But as a formula contributor, it's not really clear that not doing setting FC in your formula can get you or other users into trouble in the future. I.e., if they build any software from source and have the audacity to install an additional Fortran compiler on their system. By default the "gcc" dependency for Fortran is already saying "please use gfortran" ever since depends_on => :fortran was removed. However, thanks to CMake's quirks, merely having another Fortran compiler present on the system can break this behavior and cause very confusing and un-anticipated results that are hard to debug.

An alternate (better?) approach would be to modify the std_cmake_args when a depends_on "gcc" is present so that -DCMAKE_Fortran_COMPILER=/path/to/brewed/gfortran is included by default. If any formulae do anything naughty (from a CMake perspective) like ENV["FC"] = /path/to/brewed/open-mpi/bin/mpif90 and also use CMake for the build then it might cause some problems. Right now, this is not the case for any formulae; hdf5 and petsc are the only formulae that set ENV["FC"] = ... (Both DO set it to mpif90, but neither use CMake for the build.)

If it is decided that Formulae are responsible for providing their one ENV["FC"] = ..., then I would recommend (and try to do it myself) that formulae with a depends_on "gcc" and a depends_on "cmake" => :build" get edited to use ENV["FC"] = /path/to/homebrew/gfortran. Show me an easy way to get a list of those formulae and I can test & implement this in a PR to homebrew-core. But I'd love to hear if anyone (including @ilovezfs) finds my argument convincing that the onus of this should not be left to formula maintainers.

@ilovezfs
Copy link
Contributor

ilovezfs commented Jun 6, 2018

If it is decided that Formulae are responsible for providing their one ENV["FC"] = ..., then I would recommend (and try to do it myself) that formulae with a depends_on "gcc" and a depends_on "cmake" => :build" get edited to use ENV["FC"] = /path/to/homebrew/gfortran. Show me an easy way to get a list of those formulae and I can test & implement this in a PR to homebrew-core.

To be clear, it's the formula's responsibility to set it if needed, which would mean anywhere where CI actually fails without it. I didn't mean to imply that we should be setting everywhere. I favor as little magic as possible on the backend, and given how infrequently it's been necessary to intervene and set FC, going back to set it globally or in std_cmake_args would be an unnecessary step backwards in the direction of unneeded magic.

@ilovezfs
Copy link
Contributor

ilovezfs commented Jun 6, 2018

IMO, this it is a bug that CMake defaults to ifort over gfortran, but it is what it is.

Have you actually reported it to CMake upstream to request gfortran take precedence over ifort and been turned away with a won't-fix? That seems like a reasonable request especially given ifort isn't even in PATH during the build.

@zbeekman
Copy link
Contributor Author

zbeekman commented Jun 6, 2018 via email

@ilovezfs
Copy link
Contributor

ilovezfs commented Jun 6, 2018

It is in the PATH during the build.

How? That will only happen if you set it as a dependency in the formula.

I don’t really see it as magic if we’re already saying we want GFortran through the gcc dependency.

Setting an environment variable (or passing extra args) that may or may not be necessary for a successful build based on attempted deductions from a combination of dependencies is pretty magical.

@ilovezfs
Copy link
Contributor

ilovezfs commented Jun 6, 2018

get a list of those formulae

List of possibly affected should be

Josephs-MacBook-Pro:Formula joe$ (cd $(brew --repo homebrew/core)/Formula; grep -rlE "depends_on.*gcc" .|while read f; do brew deps --include-build -1 $f | grep -q cmake && echo $(basename $f .rb); done)
eccodes
gmsh
grib-api
json-fortran
lapack
libcds
mmseqs2
netcdf
opencoarrays
plplot
root
scalapack
spades
sundials

modulo false positives for openmp use.

@zbeekman
Copy link
Contributor Author

zbeekman commented Jun 6, 2018

It is in the PATH during the build.

How? That will only happen if you set it as a dependency in the formula.

Oh, I guess if /usr/local/bin is ignored unless a formula dep exists then I am mistaken and CMake is doing some pretty damn magical BS to find ifort. In this case, I think (hope) the CMake devs would be willing to default to the Fortran compiler actually available on the path...

List of possibly affected should be

Josephs-MacBook-Pro:Formula joe$ (cd $(brew --repo homebrew/core)/Formula; grep -rlE "depends_on.*gcc" .|while read f; do brew deps --include-build -1 $f | grep -q cmake && echo $(basename $f .rb); done)
eccodes
gmsh
grib-api
json-fortran
lapack
libcds
mmseqs2
netcdf
opencoarrays
plplot
root
scalapack
spades
sundials
modulo false positives for openmp use.

@ilovezfs you rock! If the final ruling on this issue is "won't fix/invalid", then I can go through and check/verify those formulae. In addition, I think we might want to add a note to the formula cookbook documentation in the Fortran section to warn new formula submitters that they may want to take a little extra care if their build system uses both Fortran and CMake.

@ilovezfs
Copy link
Contributor

ilovezfs commented Jun 6, 2018

Oh, I guess if /usr/local/bin is ignored unless a formula dep exists

/usr/local/bin is never in the PATH during a build (unless you're using --env=std). Instead we use /usr/local/opt/foo/bin:/usr/local/opt/bar/bin:etc.

CMake is doing some pretty damn magical BS to find ifort.

Yeah. Even putting aside whatever is allowing it to find ifort, and assuming that part is legitimate in CMake world, requesting a gfortran > ifort order of preference upstream would be lovely.

@ilovezfs
Copy link
Contributor

ilovezfs commented Jun 6, 2018

I think (hope) the CMake devs would be willing to default to the Fortran compiler actually available on the path

👍

@zbeekman
Copy link
Contributor Author

zbeekman commented Jun 6, 2018

Yeah. Even putting aside whatever is allowing it to find ifort, and assuming that part is legitimate in CMake world, requesting a gfortran > ifort order of preference upstream would be lovely.

Agreed.

I'll await the final ruling on the issue (unless you've already made it) before looking at the formula you've identified and the formula cookbook documentation upgrade.

Thanks again.

@zbeekman
Copy link
Contributor Author

zbeekman commented Jun 6, 2018

@MikeMcQuaid MikeMcQuaid added the wontfix Will not fix this issue label Jun 6, 2018
@MikeMcQuaid
Copy link
Member

If I've understood this correctly this seems to be an upstream issue and a wontfix for us. @ilovezfs if I've misunderstood: please reopen. Thanks for the report, @zbeekman!

@zbeekman
Copy link
Contributor Author

zbeekman commented Jun 7, 2018

@ilovezfs @MikeMcQuaid Can we please reopen this? I have been in touch with upstream, and have determined that this is a result of L193 in super.rb:

  def determine_cmake_prefix_path
    PATH.new(
      keg_only_deps.map(&:opt_prefix),
      HOMEBREW_PREFIX.to_s, # line 193
    ).existing
  end

The effect of this line is to say to CMake: "Go and throw HOMEBREW_PREFIX at the front of every single search path." Any find_program() and find_library() calls and a whole bunch of other stuff will be searching in /usr/local/{bin,lib,include,share,etc,...} or wherever the HOMBREW_PREFIX is. (Note that, while the documentation of the variable itself is somewhat sparse, the documentation of it's affect on find_program() which CMake uses to determine the compiler, is clear that this will get picked up from the user's environment.)

This is most certain the type of magic one should avoid, and seems to defeat the purpose of the superenv if CMake is going to go ahead and blithely ignore part it. CMake will be picking up every installed package in calls to find_program() etc. with this set.

I have confirmed that removing it from the build environment (by deleting line 193 of super.rb) resolves this issue for me. I have no idea if any CMake based packages will get caught out by the removal of this but, if they do, it most likely means that they have dependencies that have not been correctly enumerated.

I'm going to open a PR to see what happens, and make your lives easier in the event you agree with me that this is a bug in super.rb. Looking at the commit log, this seems to have been the default behavior since legacy homebrew May 2014 in commit Homebrew/legacy-homebrew@8a6c747.

@MikeMcQuaid
Copy link
Member

Nice catch!

@MikeMcQuaid MikeMcQuaid reopened this Jun 7, 2018
zbeekman added a commit to zbeekman/brew that referenced this issue Jun 7, 2018
 - `HOMEBREW_PREFIX.to_s` was getting added to the CMake environment variable:
   `CMAKE_PREFIX_PATH` which causes CMake to look for stuff in:
   `HOMEBREW_PREFIX.to_s/{bin,lib,share,include,etc,...}`
 - This can cause Homebrew to pick up alternate fortran compilres if you provie
   links to them in e.g., `/usr/local/bin` assuming you installed homebrew in
   the standard location.
 - This may cause problems with libraries, programs, headers etc. getting picked
   up that aren't enumerated as formula dependencies.
 - Fixes Homebrew#4293
@ilovezfs ilovezfs removed the wontfix Will not fix this issue label Jun 10, 2018
@stale
Copy link

stale bot commented Jul 1, 2018

This issue has been automatically marked as stale because it has not had recent activity. It will be closed if no further activity occurs.

@stale stale bot added the stale No recent activity label Jul 1, 2018
@zbeekman
Copy link
Contributor Author

zbeekman commented Jul 1, 2018 via email

@stale stale bot removed the stale No recent activity label Jul 1, 2018
@stale
Copy link

stale bot commented Jul 22, 2018

This issue has been automatically marked as stale because it has not had recent activity. It will be closed if no further activity occurs.

@stale stale bot added the stale No recent activity label Jul 22, 2018
@stale stale bot closed this as completed Jul 29, 2018
@lock lock bot added the outdated PR was locked due to age label Aug 28, 2018
@lock lock bot locked as resolved and limited conversation to collaborators Aug 28, 2018
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
outdated PR was locked due to age stale No recent activity
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants