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: remove texinfo dependency #109186

Merged
merged 1 commit into from
Aug 30, 2022
Merged

Conversation

Bo98
Copy link
Member

@Bo98 Bo98 commented Aug 30, 2022

The texinfo dependency tree is far too large for a critical component of Homebrew on older Linuxes.

It brings in the entire Python dependency tree and greatly increases the risk of including C++-only dependencies, which themselves require a modern GCC.

The dependency should also be completely unnecessary, but upstream's tarball is bugged.

@Bo98 Bo98 added CI-skip-dependents Pass --skip-dependents to brew test-bot. CI-no-bottles Merge without publishing bottles labels Aug 30, 2022
@Bo98 Bo98 force-pushed the binutils-texinfo branch 2 times, most recently from 7f9d86b to b321594 Compare August 30, 2022 02:46
@Bo98 Bo98 force-pushed the binutils-texinfo branch from b321594 to c5ef725 Compare August 30, 2022 03:36
uses_from_macos "zlib"

link_overwrite "bin/gold"
link_overwrite "bin/ld.gold"
link_overwrite "bin/dwp"

def install
# Workaround https://sourceware.org/bugzilla/show_bug.cgi?id=28909
touch "gas/doc/.dirstamp", mtime: Time.utc(2022, 1, 1)
make_args = OS.mac? ? [] : ["MAKEINFO=true"] # for gprofng
Copy link
Member

Choose a reason for hiding this comment

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

How useful even is gprofng? I'm a little keen on disabling it so we can also skip bison.

Copy link
Member Author

@Bo98 Bo98 Aug 30, 2022

Choose a reason for hiding this comment

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

No idea. Though at least bison is lighter.

texinfo dependency is indicative of a bug, since doc files should be pregenerated and we're not using autotools here.

Copy link
Member

Choose a reason for hiding this comment

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

Does bison appear elsewhere in the Linux bootstrap dep tree? If not, we should just get rid of it.

Copy link
Member Author

Choose a reason for hiding this comment

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

It does not.

@danielnachun
Copy link
Contributor

So I guess the concern here is that these dependencies drag in a large tree for build dependencies? I have no strong feelings either way given that binutils is relocatable on Linux and will not typically need to be built outside of CI. However if these extra dependencies create problems in terms of dependency trees they should obviously be dropped.

@Bo98
Copy link
Member Author

Bo98 commented Aug 30, 2022

Yeah, texinfo should also never really be required if upstream had a functional tarball. I'll probably submit a patch upstream at some point.

For the record:

`brew deps --tree texinfo`
texinfo
├── ncurses
│   ├── pkg-config
│   └── gpatch
├── perl
│   ├── berkeley-db
│   │   └── [email protected]
│   │       └── ca-certificates
│   ├── gdbm
│   └── expat
└── gettext
    ├── libxml2
    │   ├── [email protected]
    │   │   ├── pkg-config
    │   │   ├── gdbm
    │   │   ├── mpdecimal
    │   │   ├── [email protected]
    │   │   │   └── ca-certificates
    │   │   ├── readline
    │   │   │   └── ncurses
    │   │   │       ├── pkg-config
    │   │   │       └── gpatch
    │   │   ├── sqlite
    │   │   │   ├── readline
    │   │   │   │   └── ncurses
    │   │   │   │       ├── pkg-config
    │   │   │   │       └── gpatch
    │   │   │   └── zlib
    │   │   ├── xz
    │   │   ├── bzip2
    │   │   ├── expat
    │   │   ├── libffi
    │   │   ├── libxcrypt
    │   │   ├── ncurses
    │   │   │   ├── pkg-config
    │   │   │   └── gpatch
    │   │   ├── unzip
    │   │   │   ├── zip
    │   │   │   │   └── bzip2
    │   │   │   └── bzip2
    │   │   └── zlib
    │   ├── [email protected]
    │   │   ├── pkg-config
    │   │   ├── gdbm
    │   │   ├── mpdecimal
    │   │   ├── [email protected]
    │   │   │   └── ca-certificates
    │   │   ├── readline
    │   │   │   └── ncurses
    │   │   │       ├── pkg-config
    │   │   │       └── gpatch
    │   │   ├── sqlite
    │   │   │   ├── readline
    │   │   │   │   └── ncurses
    │   │   │   │       ├── pkg-config
    │   │   │   │       └── gpatch
    │   │   │   └── zlib
    │   │   ├── xz
    │   │   ├── bzip2
    │   │   ├── expat
    │   │   ├── libffi
    │   │   ├── libxcrypt
    │   │   ├── ncurses
    │   │   │   ├── pkg-config
    │   │   │   └── gpatch
    │   │   ├── unzip
    │   │   │   ├── zip
    │   │   │   │   └── bzip2
    │   │   │   └── bzip2
    │   │   ├── zlib
    │   │   └── libnsl
    │   │       ├── pkg-config
    │   │       └── libtirpc
    │   │           └── krb5
    │   │               ├── [email protected]
    │   │               │   └── ca-certificates
    │   │               └── bison
    │   │                   └── m4
    │   ├── pkg-config
    │   ├── icu4c
    │   ├── readline
    │   │   └── ncurses
    │   │       ├── pkg-config
    │   │       └── gpatch
    │   └── zlib
    └── ncurses
        ├── pkg-config
        └── gpatch

will not typically need to be built outside of CI

One of the reasons build deps always matter (besides bootstrapping from scratch etc.) is because of the non-x86_64 case. Which will get even more relevant if we're going to be using a newer GCC than what most people have.

@Bo98 Bo98 enabled auto-merge (squash) August 30, 2022 04:09
@danielnachun
Copy link
Contributor

Does texinfo actually need gettext? Or is that also a spurious dependency?

@Bo98 Bo98 merged commit 7b968f0 into Homebrew:master Aug 30, 2022
@Bo98
Copy link
Member Author

Bo98 commented Aug 30, 2022

Or is that also a spurious dependency?

Could be. Haven't had a proper look at that.

@Bo98 Bo98 deleted the binutils-texinfo branch August 30, 2022 04:20
@github-actions github-actions bot added the outdated PR was locked due to age label Sep 30, 2022
@github-actions github-actions bot locked as resolved and limited conversation to collaborators Sep 30, 2022
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
CI-no-bottles Merge without publishing bottles CI-skip-dependents Pass --skip-dependents to brew test-bot. outdated PR was locked due to age
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants