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

binutils: Enable libiberty in the build #168531

Conversation

SimonKagstrom
Copy link

  • [ x] Have you followed the guidelines for contributing?
  • [ x] Have you ensured that your commits follow the commit style guide?
  • [x ] Have you checked that there aren't other open pull requests for the same formula update/change?
  • [x ] 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>?

See Issue #168521 . To be able to use libbfd to develop programs, libiberty is needed for non-trivial tasks. This PR enables libiberty during the build.

Tested on my local x86_64 iMac.

@carlocab carlocab requested a review from fxcoudert April 10, 2024 15:11
Copy link
Contributor

Thanks for contributing to Homebrew! 🎉 It looks like you're having trouble with a CI failure. See our contribution guide for help. You may be most interested in the section on dealing with CI failures. You can find the CI logs in the Checks tab of your pull request.

@fxcoudert
Copy link
Member

I don't see how this fixes the issue. sframe_decoder_get_abi_arch is from libsframe, not libiberty, according to the doc. Also, libiberty is used internally by a lot of GNU projects (GCC, GDB, binutils), so this creates potential for conflict.

@SimonKagstrom
Copy link
Author

There's a very long list of linker errors with the default binutils, some from libiberty and some from libsframe and some from libopcodes. I just happened to paste the wrong ones in the original bug report :-)

If you're interested, this is the project I'm trying to build:

https://github.com/SimonKagstrom/emilpro/tree/next-gen

it succeeds on MacOS after my change, and runs successfully.

@SimonKagstrom
Copy link
Author

And I don't quite understand the potential conflict, perhaps you could elaborate on that? The libraries are built statically, at least on MacOS, so won't be loaded by gdb or any other binary. I'm very much a beginner at Homebrew though, so there might be something I misunderstand.

But anyway, I'd like this change to be able to build my applications (and any other which uses meaningful parts of libbfd) on MacOS in addition to Linux.

@SimonKagstrom
Copy link
Author

For reference, omitting libiberty when linking leads to thousands of lines of these linker errors:

-- Build files have been written to: /Users/ska/projects/build/emilpro
[1/1] Linking CXX executable qt/emilpro/emilpro
FAILED: qt/emilpro/emilpro
: && /Applications/Xcode.app/Contents/Developer/Toolchains/XcodeDefault.xctoolchain/usr/bin/c++ -g -isysroot /Applications/Xcode.app/Contents/Developer/Platforms/MacOSX.platform/Developer/SDKs/MacOSX14.4.sdk -mmacosx-version-min=14.1 [...] && :
Undefined symbols for architecture x86_64:
  "__hex_value", referenced from:
      _ihex_object_p in libbfd.a[26](ihex.o)
      _ihex_get_section_contents in libbfd.a[26](ihex.o)
      _ihex_scan in libbfd.a[26](ihex.o)
      _ihex_scan in libbfd.a[26](ihex.o)
      _srec_object_p in libbfd.a[27](srec.o)
      _srec_get_section_contents in libbfd.a[27](srec.o)
      _srec_get_section_contents in libbfd.a[27](srec.o)
      _srec_get_section_contents in libbfd.a[27](srec.o)
      _srec_get_section_contents in libbfd.a[27](srec.o)
      ...
  "__objalloc_alloc", referenced from:
      _bfd_hash_table_init_n in libbfd.a[13](hash.o)
      _bfd_hash_lookup in libbfd.a[13](hash.o)
      _bfd_hash_insert in libbfd.a[13](hash.o)
 [...]

among other symbols needed by libbfd.

@SimonKagstrom
Copy link
Author

@fxcoudert any more comments?

@SimonKagstrom
Copy link
Author

Bumping again @fxcoudert and @carlocab :-)

As I said, I'm developing an application which uses libbfd as a library. It builds and runs beautifully on MacOS (as well as Linux) with this small patch,

Screenshot 2024-04-28 at 20 15 11

so I'd really think it would be splendid if it could be merged.

@fxcoudert
Copy link
Member

And I don't quite understand the potential conflict, perhaps you could elaborate on that?

libiberty is used in binutils, gdb, gcc and a couple of others. So why ship it as part of binutils (which few people use on macOS anyway, since it doesn't have a linker, for example) and not gdb or gcc? We can't ship it several times, it would conflict.

binutils and gdb are most commonly tools that people use, rather than libraries to link against. So your use case is not relevant to the majority of Homebrew users. I would recommend you simply maintain your own tap for it: https://docs.brew.sh/How-to-Create-and-Maintain-a-Tap

@fxcoudert fxcoudert closed this Apr 28, 2024
@SimonKagstrom
Copy link
Author

But is it really so @fxcoudert? libiberty is a static library like the rest of the libbfd libraries:

Simons-iMac-7:emilpro ska$ ls /usr/local/opt/binutils/lib/
libbfd.a	libctf-nobfd.a	libctf.a	libiberty.a	libopcodes.a	libsframe.a

so I don't really see how it could conflict? gdb runs just fine by the way:

Simons-iMac-7:emilpro ska$ gdb qt/emilpro/emilpro
GNU gdb (GDB) 14.2
Copyright (C) 2023 Free Software Foundation, Inc.
License GPLv3+: GNU GPL version 3 or later <http://gnu.org/licenses/gpl.html>
This is free software: you are free to change and redistribute it.
There is NO WARRANTY, to the extent permitted by law.
Type "show copying" and "show warranty" for details.
This GDB was configured as "x86_64-apple-darwin23.2.0".

The current recipe for binutils contains libbfd.a, which is practically unusable since it requires libiberty to do any real work. This patch just corrects that.

Yes, I can obviously use my local patch to build, but it makes it quite a hassle for anyone else to use my application, or for that matter integrate it into a CI.

I really hope you can reconsider this.

@github-actions github-actions bot added the outdated PR was locked due to age label May 29, 2024
@github-actions github-actions bot locked as resolved and limited conversation to collaborators May 29, 2024
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
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants