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

gcc 12.1.0 #106755

Closed
wants to merge 57 commits into from
Closed

gcc 12.1.0 #106755

wants to merge 57 commits into from

Conversation

carlocab
Copy link
Member

  • Have you followed the guidelines for contributing?
  • Have you ensured that your commits follow the commit style guide?
  • Have you checked that there aren't other open pull requests for the same formula update/change?
  • Have you built your formula locally with brew install --build-from-source <formula>, where <formula> is the name of the formula you're submitting?
  • Is your test running fine brew test <formula>, where <formula> is the name of the formula you're submitting?
  • Does your build pass brew audit --strict <formula> (after doing brew install --build-from-source <formula>)? If this is a new formula, does it pass brew audit --new <formula>?

Continuation of #100411, since rebasing that was a little tricky.

Closes #100411.

@carlocab carlocab added the CI-no-fail-fast Continue CI tests despite failing GitHub Actions matrix builds. label Jul 28, 2022
@BrewTestBot BrewTestBot added the automerge-skip `brew pr-automerge` will skip this pull request label Jul 28, 2022
@carlocab carlocab mentioned this pull request Jul 28, 2022
Comment on lines +71 to +74
# Use `lib/gcc/current` to provide a path that doesn't change with GCC's version.
args = %W[
--prefix=#{opt_prefix}
--libdir=#{opt_lib}/gcc/#{version_suffix}
--libdir=#{opt_lib}/gcc/current
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

CC @fxcoudert, in case this is not a good idea.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think it's a good idea, and I cannot see any drawback. But, this is a subtle issue, so I might be overlooking something 😄

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes, it's the subtle issues that I'm concerned about. Ah well, hopefully nothing 💥s

@@ -210,7 +208,7 @@ def post_install
# * `-L#{HOMEBREW_PREFIX}/lib` instructs gcc to find the rest
# brew libraries.
# Note: *link will silently add #{libdir} first to the RPATH
libdir = HOMEBREW_PREFIX/"lib/gcc/#{version_suffix}"
libdir = HOMEBREW_PREFIX/"lib/gcc/current"
Copy link
Member Author

@carlocab carlocab Jul 28, 2022

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@iMichka @danielnachun I don't know what this does, so would be good to check that this makes sense.

I'm also not clear on why we don't want opt_lib here, as in opt_lib/"gcc/current".

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We can clean this up in a second step as this is in the postinstall block.

current looks ok to me given the other changes to gcc made here.
There are some explanations above: the spec file just makes sure the right -L flags are passed when using brewed gcc (without shims, so when you want to compile your own stuff locally). With this our users don't need to set the -L flags manually for the internal gcc libraries.

As to why not using opt_lib: why not, might be worth a try.

@carlocab carlocab added long build Set a long timeout for formula testing CI-long-timeout [DEPRECATED] Use longer GitHub Actions CI timeout. labels Jul 28, 2022
@cho-m cho-m added the CI-linux-self-hosted Build on Linux self-hosted runner label Jul 29, 2022
@fxcoudert
Copy link
Member

fxcoudert commented Jul 29, 2022

    # Provide a `lib/gcc/xy` directory to align with the versioned GCC formulae.
     (lib/"gcc").install_symlink "current" => version_suffix

I'm wondering if we really need to do this. I don't think anything inside Homebrew relies on this, and I think it's more confusing to add a duplicate tree structure (especially since we cannot have a single symlink).

@carlocab
Copy link
Member Author

carlocab commented Jul 30, 2022

I would find it pretty confusing to have a lib/gcc/{9,10,11} but no lib/gcc/12.

If we don't want the symlinks, why not just make it lib/gcc? The extra current directory seems entirely superfluous then.

Maybe as additional context, this is really motivated by the directory structure you find in a typical macOS framework. There, you have a Versions directory which contains the various framework versions, but also a Current symlink pointing to one of those versions (typically the latest).

If this is very confusing, we could make current really just a symlink, at the expense of slightly more code.

@fxcoudert
Copy link
Member

@carlocab I see your reasoning. One point of note is that if several directories exist, we might start having stuff link to specific version numbers instead of current. But maybe I'm paranoid.

@carlocab
Copy link
Member Author

carlocab commented Jul 30, 2022

No, the linker will use the library install names, which reference current and not the version number (because that's what we used for --libdir).

In particular, you can do something like

clang foo.c -L$HOMEBREW_PREFIX/lib/gcc/12 -lgcc_s

and yet you will still link using the current directory.

On Linux all that matters is the RPATH.

@carlocab
Copy link
Member Author

Linkage failures:

abyss
adios2
arpack
augustus
btop
cgns
cp2k
dungeon
dynare
eccodes
fastme
fftw
giza
gromacs
hdf5
hdf5-mpi
hdf5@1.10
hdf5@1.8
highs
ipopt
json-fortran
kahip
kim-api
lapack
libxc
mpich
msc-generator
netcdf
nwchem
octave
open-mpi
openblas
opencoarrays
openfast
openkim-models
openmodelica
packmol
petsc
petsc-complex
plplot
pnetcdf
qd
qrupdate
r
reprepro
root
scalapack
scipy

This was referenced Jul 30, 2022
@carlocab
Copy link
Member Author

Looks like we need all the revision bumps from #100411. Let me cherry-pick those and re-add the ones that get lost because of changes on master.

If someone could help me make sure I didn't lose any commits from #100411, that'd be much appreciated.

@carlocab
Copy link
Member Author

carlocab commented Jul 30, 2022

ARM failures:

brew test --verbose augustus
brew install --verbose --build-bottle dynare
brew fetch --retry ipopt --build-bottle --force
brew install --verbose --build-bottle ipopt
  • dynare: patch needs removal (already in 5.2)
  • ipopt: network error
  • augustus: no idea

Formula/dynare.rb Outdated Show resolved Hide resolved
@carlocab
Copy link
Member Author

Dropped gcc@11 so we can get a better sense of what's broken on Linux. I was hoping to run Intel tests to the end, but it's now taking too long.

@carlocab
Copy link
Member Author

augustus is probably broken, and digging into some Git logs suggests it's been that way for some time: #40220

If it fails again I don't think it should block merging this.

@fxcoudert
Copy link
Member

  • There is a Linux openblas test failure :(
==> /usr/bin/gcc-5 test.c -I/home/linuxbrew/.linuxbrew/Cellar/openblas/0.3.20_1/include -L/home/linuxbrew/.linuxbrew/Cellar/openblas/0.3.20_1/lib -lopenblas -o test
==> ./test
./test: error while loading shared libraries: libopenblas.so.0: ELF load command address/offset not properly aligned
Error: openblas: failed

which triggers failures in arpack numpy qrupdate pythran

  • There is a Linux build failure of octave (might be related, not 100% sure).
  • Linux build failure of kim-api

@fxcoudert
Copy link
Member

fxcoudert commented Jul 31, 2022

In addition to the first PR removing Linux-only dependencies that fail at #106926 here is a list of failures in the previous (unfinished) run: cgl chakra chapel clazy clp coinutils csound dlib dpp eigenpy enzyme faiss fibjs gnustep-base igraph jags julia lc0 ldc libobjc2 libsigrok lit llnode llvm llvm@13 mesa minizinc nu objfw odin or-tools rtags scs. I'm going to open another PR for those, see which ones depend on gcc and which are due to some other issue.

@carlocab
Copy link
Member Author

  • There is a Linux openblas test failure :(
==> /usr/bin/gcc-5 test.c -I/home/linuxbrew/.linuxbrew/Cellar/openblas/0.3.20_1/include -L/home/linuxbrew/.linuxbrew/Cellar/openblas/0.3.20_1/lib -lopenblas -o test
==> ./test
./test: error while loading shared libraries: libopenblas.so.0: ELF load command address/offset not properly aligned
Error: openblas: failed

which triggers failures in arpack numpy qrupdate pythran

Some Googling suggests this could be a consequence of running strip on the shared libraries. Does openblas do this during the build? If it does, can we convince it not to?

@BrewTestBot
Copy link
Member

:shipit: @carlocab has triggered a merge.

@carlocab
Copy link
Member Author

carlocab commented Aug 5, 2022

Bottles are being uploaded. Assuming the GitHub runner doesn't run out of space, I expect this to land within the next hour or so.

@carlocab carlocab deleted the gcc-12 branch August 5, 2022 15:07
@carlocab
Copy link
Member Author

carlocab commented Aug 5, 2022

🚢

carlocab added a commit to carlocab/homebrew-core that referenced this pull request Aug 5, 2022
This fails to build when Python3 is not available. See Homebrew#106755.
@iMichka iMichka mentioned this pull request Aug 5, 2022
6 tasks
@iMichka
Copy link
Member

iMichka commented Aug 5, 2022

Here is the PR for patchelf on Linux: #107388

@iMichka
Copy link
Member

iMichka commented Aug 5, 2022

Let's work on the rest of the issues found on Linux:

@carlocab carlocab mentioned this pull request Aug 5, 2022
6 tasks
@carlocab
Copy link
Member Author

carlocab commented Aug 5, 2022

gdb: #107389

qt just hardcodes a reference to gcc-11 as its compiler. We can't afford a qt rebuild whenever gcc gets a major version bump. gcc-current? 😂

My guess is that this covers the rest of the qt dependents too: anything that starts with q, plus treefrog.

verilator is not a Qt-dependent, but it also hardcodes g++-11.

llvm@13 looks like #107018.

@iMichka iMichka mentioned this pull request Aug 5, 2022
6 tasks
BrewTestBot pushed a commit that referenced this pull request Aug 5, 2022
Follow up of #106755

Closes #107388.

Signed-off-by: Carlo Cabrera <30379873+carlocab@users.noreply.github.com>
Signed-off-by: BrewTestBot <1589480+BrewTestBot@users.noreply.github.com>
@bradcray
Copy link

bradcray commented Aug 5, 2022

@iMichka / @carlocab : Just noting that I see the Chapel checkbox on the list above and am checking to see if I can find someone on our team to help with it, if that would be helpful.

@carlocab
Copy link
Member Author

carlocab commented Aug 5, 2022

That would be very helpful, @bradcray, thanks so much. To be clear, the macOS builds are fine. It's the Linux build that seems to have issues.

The error log is here: https://github.com/Homebrew/homebrew-core/runs/7661609625?check_suite_focus=true#step:11:4370

@cho-m
Copy link
Member

cho-m commented Aug 5, 2022

chapel test also fails on macOS in other PRs (#107088, #107164). Error seems similar to Linux

[Fail] Compilation output:
mv: rename /private/tmp/chpl-brew.deleteme-U5CzFi/hello6-taskpar-dist.tmp to /private/tmp/chapel-test-20220804-87278-h3rn08/.chpl/chapel-test-tZwPUP/hello6-taskpar-dist: No such file or directory
error: mv /private/tmp/chpl-brew.deleteme-U5CzFi/hello6-taskpar-dist.tmp /private/tmp/chapel-test-20220804-87278-h3rn08/.chpl/chapel-test-tZwPUP/hello6-taskpar-dist

EDIT: Currently no error on macOS 10.15 with llvm@11.

@bradcray
Copy link

bradcray commented Aug 5, 2022

@carlocab: Saw that, thanks for making sure.
@cho-m: We've been seeing and working on diagnosing related failures on some local M1 builds lately, so hopefully it's the same root issue for all cases. Thanks for making us aware of those cases, which I hadn't been aware of.

BrewTestBot pushed a commit that referenced this pull request Aug 5, 2022
This fails to build when Python3 is not available. See #106755.

Closes #107385.

Signed-off-by: BrewTestBot <1589480+BrewTestBot@users.noreply.github.com>
iMichka added a commit to iMichka/homebrew-core that referenced this pull request Aug 7, 2022
BrewTestBot pushed a commit that referenced this pull request Aug 10, 2022
Follow up of #106755

Closes #107390.

Signed-off-by: Carlo Cabrera <30379873+carlocab@users.noreply.github.com>
Signed-off-by: BrewTestBot <1589480+BrewTestBot@users.noreply.github.com>
@github-actions github-actions bot added the outdated PR was locked due to age label Sep 11, 2022
@github-actions github-actions bot locked as resolved and limited conversation to collaborators Sep 11, 2022
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
automerge-skip `brew pr-automerge` will skip this pull request CI-linux-self-hosted Build on Linux self-hosted runner CI-long-timeout [DEPRECATED] Use longer GitHub Actions CI timeout. CI-no-fail-fast Continue CI tests despite failing GitHub Actions matrix builds. long build Set a long timeout for formula testing no long build conflict Do not allow merging other pull requests when files conflict with this one outdated PR was locked due to age
Projects
None yet
Development

Successfully merging this pull request may close these issues.

7 participants