Skip to content

*binutils 2.42#161274

Merged
BrewTestBot merged 16 commits intoHomebrew:masterfrom
chenrui333:binutils-2.42
Apr 2, 2024
Merged

*binutils 2.42#161274
BrewTestBot merged 16 commits intoHomebrew:masterfrom
chenrui333:binutils-2.42

Conversation

@chenrui333
Copy link
Copy Markdown
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 HOMEBREW_NO_INSTALL_FROM_API=1 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 HOMEBREW_NO_INSTALL_FROM_API=1 brew install --build-from-source <formula>)? If this is a new formula, does it pass brew audit --new <formula>?

@chenrui333 chenrui333 added the CI-linux-self-hosted Build on Linux self-hosted runner label Jan 30, 2024
@chenrui333
Copy link
Copy Markdown
Member Author

 output.cc:32:10: fatal error: 'uchar.h' file not found
 #include <uchar.h>
          ^~~~~~~~~

@chenrui333 chenrui333 added the build failure CI fails while building the software label Jan 30, 2024
@FtZPetruska
Copy link
Copy Markdown
Contributor

Looks like the issue has already been reported upstream: https://sourceware.org/bugzilla/show_bug.cgi?id=31320

From personal testing, the following path was enough to get it to build on my personal machine (Sonoma, arm64):

diff --git a/gold/output.cc b/gold/output.cc
index ead67f20363..98b2f25c18b 100644
--- a/gold/output.cc
+++ b/gold/output.cc
@@ -29,7 +29,9 @@
 #include <unistd.h>
 #include <sys/stat.h>
 #include <algorithm>
+#ifndef __APPLE__
 #include <uchar.h>
+#endif
 
 #ifdef HAVE_SYS_MMAN_H
 #include <sys/mman.h>
diff --git a/gold/stringpool.cc b/gold/stringpool.cc
index d8f38cfabc1..e04909edad1 100644
--- a/gold/stringpool.cc
+++ b/gold/stringpool.cc
@@ -25,7 +25,9 @@
 #include <cstring>
 #include <algorithm>
 #include <vector>
+#ifndef __APPLE__
 #include <uchar.h>
+#endif
 
 #include "output.h"
 #include "parameters.h"

Using this suggestion from the thread did not work for me:

#ifdef __APPLE__
#include <stdint.h>
typedef uint_least16_t char16_t;
typedef uint_least32_t char32_t;
#else
#include <uchar.h>
#endif
output.cc:34:24: error: cannot combine with previous 'type-name' declaration specifier
typedef uint_least16_t char16_t;
                       ^
output.cc:35:24: error: cannot combine with previous 'type-name' declaration specifier
typedef uint_least32_t char32_t;
                       ^

@fxcoudert fxcoudert added the CI-no-fail-fast Continue CI tests despite failing GitHub Actions matrix builds. label Mar 5, 2024
@fxcoudert
Copy link
Copy Markdown
Member

Rerunning to see which SDKs have the header and which don't.

@fxcoudert
Copy link
Copy Markdown
Member

Signed-off-by: Rui Chen <rui@chenrui.dev>
@fxcoudert
Copy link
Copy Markdown
Member

I've made some tests in #165082 and I'm not sure there is a way for macOS 12. Newer binutils require C++11 with uchar.h available, and macOS 12 doesn't seem to have that.

Signed-off-by: Rui Chen <rui@chenrui.dev>
Signed-off-by: Rui Chen <rui@chenrui.dev>
Signed-off-by: Rui Chen <rui@chenrui.dev>
Signed-off-by: Rui Chen <rui@chenrui.dev>
Signed-off-by: Rui Chen <rui@chenrui.dev>
Signed-off-by: Rui Chen <rui@chenrui.dev>
Signed-off-by: Rui Chen <rui@chenrui.dev>
@fxcoudert fxcoudert removed the build failure CI fails while building the software label Mar 5, 2024
@fxcoudert
Copy link
Copy Markdown
Member

Upstream effectively decided to raise their requirements and excluded Monterey and earlier. Not sure why it only manifests for some targets.

@iMichka iMichka added the automerge-skip `brew pr-automerge` will skip this pull request label Mar 6, 2024
Copy link
Copy Markdown
Member

@iMichka iMichka left a comment

Choose a reason for hiding this comment

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

Would this break some reverse deps on Monterey? I see a few formula that would still need it on macOS

@iMichka iMichka removed the automerge-skip `brew pr-automerge` will skip this pull request label Mar 6, 2024
@github-actions
Copy link
Copy Markdown
Contributor

github-actions bot commented Mar 6, 2024

🤖 An automated task has requested bottles to be published to this PR.

@iMichka
Copy link
Copy Markdown
Member

iMichka commented Mar 6, 2024

Whoops I approved to fast, I wanted to discuss the Monterey situation first, sorry. I canceled the upload job.

@fxcoudert
Copy link
Copy Markdown
Member

Would this break some reverse deps on Monterey? I see a few formula that would still need it on macOS

Neither arm-linux-gnueabihf-binutils nor x86_64-linux-gnu-binutils are dependencies of anything in homebrew-core.

@Bo98
Copy link
Copy Markdown
Member

Bo98 commented Mar 6, 2024

Unlike C, char16_t and char32_t are builtin types in C++. They are not defined in uchar.h so the include seems unnecessary entirely unless they're catering for C99-compatible compilers that are not C++11 compliant (though https://sourceware.org/git/gitweb.cgi?p=binutils-gdb.git;h=941d02eaae0557c80c9e4130478e584a8b284494 seems to suggest otherwise).

@Bo98
Copy link
Copy Markdown
Member

Bo98 commented Mar 6, 2024

For example, see:

It would be needed on GCC 4.1 which supports C99 but not C++11, but since GCC 4.1 doesn't accept -std=c++0x anyway such compiler would be blocked early: https://sourceware.org/git/gitweb.cgi?p=binutils-gdb.git;h=941d02eaae0557c80c9e4130478e584a8b284494.

So the correct patch for me is to remove those includes, given they are not compatible with libc++ prior to 2022 (15.0): llvm/llvm-project@311ff39 and https://sourceware.org/bugzilla/show_bug.cgi?id=30867 shows they are willing to support GCC from 2013.

I imagine this issue also affects some of the BSDs too.

@cho-m cho-m added the upstream issue An upstream issue report is needed label Mar 19, 2024
@fxcoudert
Copy link
Copy Markdown
Member

As stated above in response to:

Would this break some reverse deps on Monterey? I see a few formula that would still need it on macOS

Neither arm-linux-gnueabihf-binutils nor x86_64-linux-gnu-binutils are dependencies of anything in homebrew-core.

Maybe upstream will accept a patch to improve the situation, but right now they effectively decided to require it, so I think we should just follow that.

@fxcoudert fxcoudert requested a review from iMichka March 28, 2024 13:11
@fxcoudert fxcoudert removed the upstream issue An upstream issue report is needed label Mar 28, 2024
@github-actions
Copy link
Copy Markdown
Contributor

github-actions bot commented Apr 1, 2024

🤖 An automated task has requested bottles to be published to this PR.

@github-actions
Copy link
Copy Markdown
Contributor

github-actions bot commented Apr 1, 2024

⚠️ Bottle publish failed.

@github-actions
Copy link
Copy Markdown
Contributor

github-actions bot commented Apr 2, 2024

@github-actions github-actions bot added the CI-published-bottle-commits The commits for the built bottles have been pushed to the PR branch. label Apr 2, 2024
@BrewTestBot BrewTestBot enabled auto-merge April 2, 2024 16:31
@BrewTestBot BrewTestBot added this pull request to the merge queue Apr 2, 2024
Merged via the queue into Homebrew:master with commit 7c199ea Apr 2, 2024
@github-actions github-actions bot added the outdated PR was locked due to age label Jul 10, 2024
@github-actions github-actions bot locked as resolved and limited conversation to collaborators Jul 10, 2024
@chenrui333 chenrui333 deleted the binutils-2.42 branch July 27, 2024 14:00
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

CI-linux-self-hosted Build on Linux self-hosted runner CI-no-fail-fast Continue CI tests despite failing GitHub Actions matrix builds. CI-published-bottle-commits The commits for the built bottles have been pushed to the PR branch. outdated PR was locked due to age

Projects

None yet

Development

Successfully merging this pull request may close these issues.

8 participants