octave: bump revision (gcc) #84245

siko1056 · 2021-08-30T07:49:01Z

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>?

The gcc of the "octave" bottle is outdated. gcc version 11.1.0 is hard coded inside, while an updated system uses gcc version 11.2.0 already. This issue is fixed by rebuilding the bottle with bumping the revision.

The stale gcc link causes very frequently used Octave packages to not install correctly. Entering the first line in the Octave command-line results in the following error messages, clearly indicating the problem of a stale gcc compiler.

>> pkg install -forge control

ld: warning: directory not found for option '-L/usr/local/opt/gcc/lib/gcc/11/gcc/x86_64-apple-darwin20/11.1.0'
ld: warning: directory not found for option '-L/usr/local/opt/gcc/lib/gcc/11/gcc/x86_64-apple-darwin20/11.1.0/../../..'
ld: library not found for -lgfortran
clang: error: linker command failed with exit code 1 (use -v to see invocation)
make: *** [__control_slicot_functions__.oct] Error 1
tar -xzf slicot.tar.gz
/usr/local/Cellar/octave/6.3.0/bin/mkoctfile-6.3.0 -Wall -Wno-deprecated-declarations  __control_helper_functions__.cc
mkdir sltmp
mv slicot/src/*.f ./sltmp
mv slicot/src_aux/*.f ./sltmp
if [ "1" = "1" ]; then \
                echo "copy routines using DGGES"; \
                cp SB04OD.fortran ./sltmp/SB04OD.f; \
                cp SG03AD.fortran ./sltmp/SG03AD.f; \
                cp SG03BD.fortran ./sltmp/SG03BD.f; \
        fi;
copy routines using DGGES
cp AB08NX.fortran ./sltmp/AB08NX.f
cp AG08BY.fortran ./sltmp/AG08BY.f
cp SB01BY.fortran ./sltmp/SB01BY.f
cp SB01FY.fortran ./sltmp/SB01FY.f
cp SB06ND.fortran ./sltmp/SB06ND.f
cp TB01MD.fortran ./sltmp/TB01MD.f
cp TB01ND.fortran ./sltmp/TB01ND.f
cp TB01ZD.fortran ./sltmp/TB01ZD.f
cp TG04BX.fortran ./sltmp/TG04BX.f
cp ODLTZM.fortran ./sltmp/ODLTZM.f
cp makefile.slicot ./sltmp/makefile
cd sltmp; /usr/local/Cellar/octave/6.3.0/bin/mkoctfile-6.3.0 -w -c MA02ID.f; rm MA02ID.f; /usr/local/Cellar/octave/6.3.0/bin/mkoctfile-6.3.0 -c *.f;
ar -rc slicotlibrary.a ./sltmp/*.o
rm -rf sltmp slicot
LDFLAGS="-F/usr/local/opt/qt@5/lib  -L/usr/local/opt/openblas/lib -lopenblas  -L/usr/local/opt/gcc/lib/gcc/11/gcc/x86_64-apple-darwin20/11.1.0 -L/usr/local/opt/gcc/lib/gcc/11/gcc/x86_64-apple-darwin20/11.1.0/../../.. -lgfortran -lquadmath -lm" \
    /usr/local/Cellar/octave/6.3.0/bin/mkoctfile-6.3.0 -Wall -Wno-deprecated-declarations__control_slicot_functions__.cc common.cc slicotlibrary.a

error: pkg: error running 'make' for the control package.
error: called from
    configure_make at line 110 column 9
    install at line 196 column 7
    pkg at line 568 column 9

A better approach would probably be to automatically rebuild this bottle if gcc is updated. However, this seems to be not desired and I do not understand the following logic, which tried to solve this issue:

homebrew-core/Formula/octave.rb

Lines 125 to 129 in 250d6e6

    
           # Avoid revision bumps whenever fftw's, gcc's or OpenBLAS' Cellar paths change 
        
           inreplace "src/mkoctfile.cc" do |s| 
        
             s.gsub! Formula["fftw"].prefix.realpath, Formula["fftw"].opt_prefix 
        
             s.gsub! Formula["gcc"].prefix.realpath, Formula["gcc"].opt_prefix 
        
           end

Detailed discussion: Homebrew/discussions#1986

cho-m · 2021-08-30T07:57:21Z

Probably should see if there is a way for current -L/usr/local/opt/gcc/lib/gcc/11/gcc/x86_64-apple-darwin20/11.1.0/../../.. to instead traverse the ..s and use -L/usr/local/opt/gcc/lib/gcc/11, where libgfortan is available.

Otherwise, at minimum, there needs to be a test case added to detect this. When gcc is updated, dependents like octave will be tested, so the only way to know when to do a revision bump is if the test fails.

SMillerDev · 2021-08-30T08:07:37Z

However, this seems to be not desired and I do not understand the following logic, which tried to solve this issue

This should replace the versioned path /usr/local/Cellar/11.1.0/gcc/ with the unversioned /usr/local/opt/gcc/, which should solve that issue. Unfortunately it seems that there is a version a little further in the include path too.

Like mentioned in the discussion, could you add a test that fails if GCC gets updated to a version that octave doesn't have baked in?

siko1056 · 2021-08-30T08:56:27Z

Thank you @cho-m and @SMillerDev for the comments. I will work on a small example and extend this PR.

carlocab · 2021-08-30T10:56:50Z

One alternative here is to force GCC to stop encoding its full version in its subdirectories. I vaguely recall a configure-time option that enables this.

This will break any existing users that rely on those paths, but they get broken with each version bump anyway.

carlocab · 2021-08-30T11:12:37Z

#84248

SMillerDev · 2021-08-30T11:22:12Z

Regardless of the fix, it would still be good to have a check for it.

From `./configure --help`: --with-gcc-major-version-only use only GCC major number in filesystem paths Some formulae rely on paths that encode GCC's full version [1] (e.g. `octave`, see Homebrew#84245), so we should stop trying to create these paths so that these formulae don't get broken on GCC version bumps. [1] e.g. `include/c++/11.2.0`, `lib/gcc/11/gcc/x86_64-apple-darwin20/11.2.0`

The previous commit reconfigures GCC to stop using its full version in file system paths. We need to bump this revision to pull in the fix. Closes Homebrew#84245.

siko1056 · 2021-08-31T05:09:13Z

Thank you @carlocab for the quick help and input 🙂

With 55cad12 I added two minimal compilations tests. The second is the most minimal version of the problem I could think of.

Please tell me if my "rusty" ruby syntax knowledge for long strings is adequate and I will polish the PR if necessary 😉

Formula/octave.rb

siko1056 · 2021-08-31T05:59:53Z

Thanks for the suggestions @carlocab , your ruby syntax looks indeed cleaner 😉

Formula/octave.rb

siko1056 · 2021-09-01T01:44:23Z

@carlocab Are further changes necessary here? The test failure seems to be related to a general compilation problem of Octave on macOS 11 arm64, the remaining tests were cancelled 😓

SMillerDev · 2021-09-01T07:10:28Z

I think we want to wait for #84248 to run this test again

carlocab · 2021-09-01T09:52:17Z

I think we want to wait for #84248 to run this test again

Actually, I think it's the reverse. We want to merge this first so that we can check that the test here fails after the revision bump to GCC in #84248.

If the test doesn't catch the change in paths from the GCC change then it's not the test we need.

siko1056 · 2021-09-13T07:11:04Z

@SMillerDev @carlocab is there anything I can help on this item?

carlocab · 2021-09-19T04:24:43Z

Fix the failing test, as merging this as is will lead to all the bottles going away.

siko1056 · 2021-10-06T05:38:08Z

My basic wish to bump the revision had already been done b74de19 and my original problem is resolved with a fresh bottle. However I do not know if #84248 will be a problem with the next gcc update again.

Is the test I wrote still of any interest? Maybe running the test suite again might succeed now 😇

Otherwise feel free to close this report.

Thank you all very much for your time and effort to resolve this issue 🙂

iMichka · 2021-11-12T18:20:03Z

@siko1056 I think the test might still be of interest. It depends on how much work it is do make it run. Do you need any help to get a green CI here?

siko1056 · 2021-11-13T12:54:44Z

Thanks for your reply @iMichka . The last test run is quite some time ago. If it was run again, I think it should pass now. Can you trigger the test suit run again?

The more interesting question is, does it fail if GCC links are stale again, i.e. the purpose of the test 😉

siko1056 · 2021-11-25T07:21:04Z

@iMichka I made some updates and only the first time the test suite was run. Can you trigger another run? Many thanks in advance 🙂

iMichka · 2021-11-25T19:55:00Z

You have a syntax error:

Formula/octave.rb:149:1: C: [Correctable] Use 2 spaces for indentation in a heredoc.
        mkoctfile('-v', '-std=c++11', 'oct_demo.cc'); ...

siko1056 · 2021-11-28T06:29:11Z

Thanks @iMichka, I fixed the problem and some tests pass now. However, some tests timed out and on "ubuntu-latest" the "mkoctfile" configuration seems not to be setup correctly.

If the workers are less busy, can the check be re-run? 😓

siko1056 · 2021-11-28T13:23:35Z

Thank you @carlocab for the extra attempt. All checks except unbuntu are working, enough to merge? 😇

carlocab · 2021-11-28T13:34:26Z

Not quite:

Merging PRs with brew test failures breaks bottles.
You still have a merge commit on your branch, so BrewTestBot won't be able to merge this (Homebrew/core has a linear history requirement).

We can probably fix the first one by adding appropriate -L flags. Something like -L#{lib}/octave/#{version} should work. Failing that, we can skip the last part of the test on Linux with return if OS.linux?, but we should really try to fix it first.

You can fix the second one by rebasing your branch against origin/master. Please squash all your commits too when you do so.

siko1056 · 2021-11-28T14:14:40Z

Thanks for the constructive comments @carlocab .

Fixed item 2: I rebased my patch-1-branch and sqashed everything into a single commit.
Item 1: I don't know, why mkoctfile "forgets" all during the installation set "-L" flags? So far I only use Homebrew for macOS, but I try to use it for my Linux and figure out what is going on...

iMichka · 2021-11-29T21:13:44Z

octave: no graphical display found

This is because we run the tests in a docker container on Linux.

Can we try to run all the octave calls in the test block with the -W flag, which should disable the GUI part of octave?

siko1056 · 2021-11-30T01:29:24Z

Thanks for checking @iMichka . In general Octave does not really care if there is no display available and just turns of GUI related features as the output suggests

octave: no graphical display found
octave: disabling GUI features

The problem for the Linux compilation check is this error:

/home/linuxbrew/.linuxbrew/bin/ld: cannot find -loctinterp
/home/linuxbrew/.linuxbrew/bin/ld: cannot find -loctave

resulting from
https://github.com/Homebrew/homebrew-core/runs/4345829738?check_suite_focus=true#step:7:55

/home/linuxbrew/.linuxbrew/bin/g++-11
  -I/home/linuxbrew/.linuxbrew/Cellar/octave/6.4.0_1/include/octave-6.4.0/octave/..
  -I/home/linuxbrew/.linuxbrew/Cellar/octave/6.4.0_1/include/octave-6.4.0/octave
  -I/home/linuxbrew/.linuxbrew/Cellar/octave/6.4.0_1/include
  -pthread -fopenmp -Os -w -pipe -march=native  -std=c++11
  -o oct_demo.oct  /tmp/oct-pF7pis.o
  -shared -Wl,-Bsymbolic
  -L/home/linuxbrew/.linuxbrew/lib
  -L/home/linuxbrew/.linuxbrew/Cellar/octave/6.4.0_1/lib
  -loctinterp -loctave

I try to find out the contents of those -L directories on the builder machine. They should contain liboctinterp.so and liboctave.so to sucessfully compile the test file.

carlocab · 2021-11-30T01:39:26Z

liboctave.so and liboctinterp.so are in /home/linuxbrew/.linuxbrew/Cellar/octave/6.4.0_1/lib/octave/6.4.0, which is why I suggested you add the flag -L#{lib}/octave/#{version}

You don't need to do ls in CI to check, you can just have a look at the Linux bottle for yourself with

curl -L -H 'Authorization: Bearer QQ==' https://ghcr.io/v2/homebrew/core/octave/blobs/sha256:61a160547f61806c5c7458c7b5cd1e421fa6661c94c0e345e072d1acca10c9ae | tar -x

For example:

❯ curl -sSL -H 'Authorization: Bearer QQ==' https://ghcr.io/v2/homebrew/core/octave/blobs/sha256:61a160547f61806c5c7458c7b5cd1e421fa6661c94c0e345e072d1acca10c9ae | tar -x
❯ fd liboctave
octave/6.4.0/include/octave-6.4.0/octave/liboctave-build-info.h
octave/6.4.0/lib/octave/6.4.0/liboctave.so
octave/6.4.0/lib/octave/6.4.0/liboctave.so.8
octave/6.4.0/lib/octave/6.4.0/liboctave.so.8.0.2
octave/6.4.0/share/octave/6.4.0/etc/tests/liboctave
❯ fd liboctinterp
octave/6.4.0/include/octave-6.4.0/octave/liboctinterp-build-info.h
octave/6.4.0/lib/octave/6.4.0/liboctinterp.so
octave/6.4.0/lib/octave/6.4.0/liboctinterp.so.9
octave/6.4.0/lib/octave/6.4.0/liboctinterp.so.9.0.0

siko1056 · 2021-11-30T01:50:49Z

@carlocab Thank you again for this enlightning comment 🙂 Homebrew's build system is much better than I expected, great work you do 👏

Now I (hopefully) understand your previous comment regarding the -L#{lib}/octave/#{version} and lets wait for the builders to finish ☕

And sorry for wasting build time with the ls approach.

siko1056 · 2021-11-30T03:54:27Z

It seems to have worked 🎉 Thank you for all your helpful comments.

Ready to merge? 😇

Also, bump the revision for GCC.

carlocab

Thanks for working on this!

iMichka · 2021-11-30T06:37:14Z

Nice, thanks!

BrewTestBot · 2021-11-30T08:07:58Z

🤖 A scheduled task has triggered a merge.

BrewTestBot added the java Java use is a significant feature of the PR or issue label Aug 30, 2021

carlocab mentioned this pull request Aug 30, 2021

gcc: don't encode full version in paths #84248

Closed

6 tasks

carlocab reviewed Aug 31, 2021

View reviewed changes

Formula/octave.rb Outdated Show resolved Hide resolved

carlocab reviewed Aug 31, 2021

View reviewed changes

Formula/octave.rb Outdated Show resolved Hide resolved

carlocab reviewed Aug 31, 2021

View reviewed changes

Formula/octave.rb Outdated Show resolved Hide resolved

carlocab reviewed Aug 31, 2021

View reviewed changes

Formula/octave.rb Outdated Show resolved Hide resolved

SMillerDev added the in progress Stale bot should stay away label Sep 1, 2021

SMillerDev added the CI-no-fail-fast Continue CI tests despite failing GitHub Actions matrix builds. label Sep 1, 2021

myurkin mentioned this pull request Nov 3, 2021

Packages of ADDA adda-team/adda#282

Open

BrewTestBot added the automerge-skip `brew pr-automerge` will skip this pull request label Nov 25, 2021

carlocab added the CI-long-timeout [DEPRECATED] Use longer GitHub Actions CI timeout. label Nov 28, 2021

BrewTestBot removed the automerge-skip `brew pr-automerge` will skip this pull request label Nov 28, 2021

octave: add basic compilation test

23cfb7c

Also, bump the revision for GCC.

carlocab approved these changes Nov 30, 2021

View reviewed changes

iMichka approved these changes Nov 30, 2021

View reviewed changes

BrewTestBot closed this in abeb5e5 Nov 30, 2021

siko1056 deleted the patch-1 branch November 30, 2021 08:44

github-actions bot added the outdated PR was locked due to age label Dec 31, 2021

github-actions bot locked as resolved and limited conversation to collaborators Dec 31, 2021

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

octave: bump revision (gcc) #84245

octave: bump revision (gcc) #84245

siko1056 commented Aug 30, 2021

cho-m commented Aug 30, 2021

SMillerDev commented Aug 30, 2021

siko1056 commented Aug 30, 2021

carlocab commented Aug 30, 2021

carlocab commented Aug 30, 2021

SMillerDev commented Aug 30, 2021

siko1056 commented Aug 31, 2021

siko1056 commented Aug 31, 2021

siko1056 commented Sep 1, 2021

SMillerDev commented Sep 1, 2021

carlocab commented Sep 1, 2021

siko1056 commented Sep 13, 2021

carlocab commented Sep 19, 2021

siko1056 commented Oct 6, 2021

iMichka commented Nov 12, 2021

siko1056 commented Nov 13, 2021

siko1056 commented Nov 25, 2021

iMichka commented Nov 25, 2021

siko1056 commented Nov 28, 2021

siko1056 commented Nov 28, 2021

carlocab commented Nov 28, 2021

siko1056 commented Nov 28, 2021

iMichka commented Nov 29, 2021

siko1056 commented Nov 30, 2021

carlocab commented Nov 30, 2021 •

edited

Loading

siko1056 commented Nov 30, 2021 •

edited

Loading

siko1056 commented Nov 30, 2021

carlocab left a comment

iMichka commented Nov 30, 2021

BrewTestBot commented Nov 30, 2021

	# Avoid revision bumps whenever fftw's, gcc's or OpenBLAS' Cellar paths change
	inreplace "src/mkoctfile.cc" do \|s\|
	s.gsub! Formula["fftw"].prefix.realpath, Formula["fftw"].opt_prefix
	s.gsub! Formula["gcc"].prefix.realpath, Formula["gcc"].opt_prefix
	end

octave: bump revision (gcc) #84245

octave: bump revision (gcc) #84245

Conversation

siko1056 commented Aug 30, 2021

cho-m commented Aug 30, 2021

SMillerDev commented Aug 30, 2021

siko1056 commented Aug 30, 2021

carlocab commented Aug 30, 2021

carlocab commented Aug 30, 2021

SMillerDev commented Aug 30, 2021

siko1056 commented Aug 31, 2021

siko1056 commented Aug 31, 2021

siko1056 commented Sep 1, 2021

SMillerDev commented Sep 1, 2021

carlocab commented Sep 1, 2021

siko1056 commented Sep 13, 2021

carlocab commented Sep 19, 2021

siko1056 commented Oct 6, 2021

iMichka commented Nov 12, 2021

siko1056 commented Nov 13, 2021

siko1056 commented Nov 25, 2021

iMichka commented Nov 25, 2021

siko1056 commented Nov 28, 2021

siko1056 commented Nov 28, 2021

carlocab commented Nov 28, 2021

siko1056 commented Nov 28, 2021

iMichka commented Nov 29, 2021

siko1056 commented Nov 30, 2021

carlocab commented Nov 30, 2021 • edited Loading

siko1056 commented Nov 30, 2021 • edited Loading

siko1056 commented Nov 30, 2021

carlocab left a comment

Choose a reason for hiding this comment

iMichka commented Nov 30, 2021

BrewTestBot commented Nov 30, 2021

carlocab commented Nov 30, 2021 •

edited

Loading

siko1056 commented Nov 30, 2021 •

edited

Loading