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-arm-embedded 9-2019-q4-major (new formula) #46584

Closed

Conversation

dunkmann00
Copy link

@dunkmann00 dunkmann00 commented Nov 10, 2019

This formula installs the GNU Arm Embedded Toolchain for Arm Cortex-M
and Cortex-R processors. This used to be included in Homebrew Cask but
was removed because according to Homebrew policy, the binaries should be
built and not simply linked to, since it is an open-source CLI tool
(Homebrew/homebrew-cask#56802).

  • Have you followed the guidelines for contributing?
  • 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 <formula>)?

Formula/gcc-arm-embedded.rb Outdated Show resolved Hide resolved
Formula/gcc-arm-embedded.rb Show resolved Hide resolved
Formula/gcc-arm-embedded.rb Outdated Show resolved Hide resolved
@chenrui333 chenrui333 added the new formula PR adds a new formula to Homebrew/homebrew-core label Nov 10, 2019
@dunkmann00
Copy link
Author

So I've run into a bit of a problem. I changed all of the resources into depends_on and that works fine so long as I symlink them into the same place as the build-toolchain.sh script thinks they should be. But when I ran brew audit it told me that expat and zlib are included on MacOS and should be uses_from_macos instead. The problem with this is that they then can't be symlinked the same way. I'm not sure what would be the best way to proceed. I have two ideas that I wanted to run by both of you (@fxcoudert , @SMillerDev) to see if either of these are acceptable/what you think is most appropriate:

  • Use depends_on for both lib and expat even though I'm getting an error from audit
  • Modify build-toolchain.sh and host it somewhere, like GitHub, and pull it in as a resource. I can adjust the file to use the correct library locations, and to use the system included libraries where appropriate.

Do either of those sound appropriate? Or if you have any other suggestions those would be welcome as well. Thanks!

@SMillerDev
Copy link
Member

Generally you should be able to tell a build script where it can find dependencies. If this is not the case here, please file an issue or pull request upstream suggesting to change that. If that doesn't work you might want to replace the paths in the build script instead using inreplace in the formula.

@drewcassidy
Copy link

It looks like the URL has changed as well. trying to use this results in

  DownloadError: Failed to download resource "gcc-arm-embedded"
Download failed: https://developer.arm.com/-/media/Files/downloads/gnu-rm/9-2019q4/RC2.1/src%202.0/gcc-arm-none-eabi-9-2019-q4-major-src.tar.bz2

@dunkmann00
Copy link
Author

It seems there was some problem on their end (bug report). I'll update with the new URL when I push the changes for the dependencies. I'll try to do that tonight or tomorrow.

@klaygomes
Copy link

Last message says the problem was fixed.

@dunkmann00
Copy link
Author

Correct, they fixed it and the URL is now different.

This formula installs the GNU Arm Embedded Toolchain for Arm Cortex-M
and Cortex-R processors. This used to be included in Homebrew Cask but
was removed because according to Homebrew policy, the binaries should be
built and not simply linked to, since it is an open-source CLI tool
(Homebrew#56802).
depends_on "mpfr"

def install
ENV.deparallelize
Copy link
Member

Choose a reason for hiding this comment

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

All the components (binutils, gcc) should build fine in parallel. What is the problem?

Copy link
Author

Choose a reason for hiding this comment

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

The build fails when it is run in parallel like this. There are specific makefiles that can be run in parallel and the build will succeed on Catalina, but not on High Sierra and Mojave if they use APFS. This bug seems to be related to an APFS issue that looks like it is fixed on Catalina.

I referenced this on a response to a prior change you requested (#46584 (comment)).

Copy link
Member

Choose a reason for hiding this comment

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

I'm sorry, we have some many PRs open, it's sometimes hard to keep track. Right now, a 4-hour build for a formula with minor usage is not something we can have in Homebrew core, I'm sorry.

Regarding parallel building, if you go to the GCC bugzilla issue, you'll see it was reported by me, actually… 😄 and it was fixed, so it should definitely not be an issue anymore. We build many GCC versions in parallel, in the gcc* formulas.

Copy link
Author

Choose a reason for hiding this comment

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

I didn't even realize that lol! What're the odds. And I understand, I do think this formula would get some usage due to the gcc arm embedded compiler being needed for compiling CircuitPython, but I see your point. I am hoping I can get this formula to a place that is acceptable as it would be my first formula contribution, which is somewhat exciting.

I will change the formula and put back the inreplace for parallel building. However, I cannot test this on my own machine for the High Sierra and Mojave builds. So I will just have to see what happens with the test bot. Is this okay?


test do
# 'error Unsupported architecture' is thrown by XCode preprocessor, removing CPATH fixes this
ENV.delete "CPATH"
Copy link
Member

Choose a reason for hiding this comment

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

How about a simpler test? Compiling a simple C program, for example.

@fxcoudert
Copy link
Member

After 4 hours, I killed the CI job. Non-parallel make is not going to be acceptable.

- Add parallel execution for makefiles.
@SMillerDev
Copy link
Member

Killed again, please fix the parallel build before pushing again @dunkmann00

@dunkmann00
Copy link
Author

The last commit did address the parallel build. While it didn't make everything run in parallel, it would make most of the build do so. When I build it on my machine it takes around 1.5 hours. If the amount of time that it takes the test bot is unacceptable then I am not sure how to proceed.

I understand you don't want certain builds occupying too much of the test bot's time, but I waited until the build queue was empty, and this is not a formula that would be updated frequently.

I kindly request for you to allow it to run for a little while longer? If this is not something you are willing to allow I would like to ask if it would once again be acceptable to add the formula for a prebuilt version of the gcc arm embedded toolchain into homebrew-cask? I reference this issue in my original pull request comment and I will reference it here again for convenience Homebrew/homebrew-cask#56802

@SMillerDev
Copy link
Member

It still defines that it should not be running in parallel as the first line in the install block. @fxcoudert opinions?

@fxcoudert
Copy link
Member

Apparently it still uses parallelisation inside. @BrewTestBot test this please

I'm still concerned about the fragility of this formula, but… given the effort, probably it is worth merging.

@iMichka
Copy link
Member

iMichka commented Dec 28, 2019

The high-sierra build failed with:

In file included from /private/tmp/gcc-arm-embedded-20191228-70898-1uu1300/gcc-arm-none-eabi-9-2019-q4-major/src/gcc/libstdc++-v3/libsupc++/exception:143,
                 from /private/tmp/gcc-arm-embedded-20191228-70898-1uu1300/gcc-arm-none-eabi-9-2019-q4-major/src/gcc/libstdc++-v3/libsupc++/new:40,
                 from /private/tmp/gcc-arm-embedded-20191228-70898-1uu1300/gcc-arm-none-eabi-9-2019-q4-major/src/gcc/libstdc++-v3/libsupc++/bad_alloc.cc:26:
/private/tmp/gcc-arm-embedded-20191228-70898-1uu1300/gcc-arm-none-eabi-9-2019-q4-major/build-native/gcc-final/arm-none-eabi/thumb/v8-m.base/nofp/libstdc++-v3/include/bits/exception_ptr.h:37:10: fatal error: bits/exception_defines.h: No such file or directory
   37 | #include <bits/exception_defines.h>
      |          ^~~~~~~~~~~~~~~~~~~~~~~~~~
compilation terminated.
make[8]: *** [bad_alloc.lo] Error 1
make[8]: *** Waiting for unfinished jobs....
make[7]: *** [all-recursive] Error 1
make[6]: *** [all] Error 2
make[5]: *** [multi-do] Error 1
make[4]: *** [all-multi] Error 2
make[3]: *** [all-recursive] Error 1
make[2]: *** [all] Error 2
make[1]: *** [all-target-libstdc++-v3] Error 2
make: *** [all] Error 2

@iMichka
Copy link
Member

iMichka commented Dec 28, 2019

I killed the build to free up some CI time for other things. High-Sierra failed (see above), and Catalina and Mojave were running for more than 3 hours, so I think it's better to fix the first failure first. I also find the 3 hours build time suspicious. Even building llvm from source takes less time.

@travisgriggs
Copy link

I am new to homebrew. I use these toolchains in binary form from a lot. I was excited to use brew to install this toolchain. But there wasn't a cask/formula to do so. Wanting to learn by doing, I thought maybe I'd have a go. But it turns that @dunkmann00 was ahead of me. But the logic seems really circular. There's apparently a bit of complexity about getting this built right. But somebody else already does that job and makes a binary available. It used to be a cask. But then it got removed and turned into a formula. But the formula's not ready. As a newcomer, I was a bit surprised to find this kind of pedanticism. I'm used to that in the Debian world of 10 years ago.

Anyway, when in Rome... so... can I help this effort in anyway? Can I at least test this formula and benefit from it in it's not-yet-approved state? I am running Catalina...

@SMillerDev
Copy link
Member

You can always make a tap where you keep this formula as it currently is, or where you distribute the binary form. But for it to be in the core tap it needs to build in parallel and needs to fix the failures.

@travisgriggs
Copy link

Anyway, when in Rome... so... can I help this effort in anyway? Can I at least test this formula and benefit from it in it's not-yet-approved state? I am running Catalina...

I did the pull thing and ran it. Installs and works fine. But still takes a long time:

🍺  /usr/local/Cellar/gcc-arm-embedded/9-2019-q4-major: 2,937 files, 419.7MB, built in 92 minutes 42 seconds

@jonchang
Copy link
Contributor

We have a bit more CI time so I’ve restarted the build.

@fxcoudert
Copy link
Member

At this stage, it seems the build is too fragile, and too specific, for us to maintain in Homebrew core. The software was added back as a cask: Homebrew/homebrew-cask#75091

@fxcoudert fxcoudert closed this Jan 6, 2020
@lock lock bot added the outdated PR was locked due to age label Feb 5, 2020
@lock lock bot locked as resolved and limited conversation to collaborators Feb 5, 2020
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
new formula PR adds a new formula to Homebrew/homebrew-core outdated PR was locked due to age
Projects
None yet
Development

Successfully merging this pull request may close these issues.

9 participants