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

bash is being build against system ncurses on mac limiting usage of newer terminfo #158667

Closed
4 tasks done
jindraj opened this issue Dec 31, 2023 · 17 comments · Fixed by #206855
Closed
4 tasks done

bash is being build against system ncurses on mac limiting usage of newer terminfo #158667

jindraj opened this issue Dec 31, 2023 · 17 comments · Fixed by #206855
Labels
bug Reproducible Homebrew/homebrew-core bug maintainer feedback Additional maintainers' opinions may be needed outdated PR was locked due to age stale No recent activity

Comments

@jindraj
Copy link
Contributor

jindraj commented Dec 31, 2023

My "brew doctor output" says Your system is ready to brew.

no it doesn't but it is not related to the issue at all.

I have resolved all warnings from brew doctor and that did not fix my problem.

no, it wouldn't help at all.

brew config

: 4.2.1-36-gbc566b5
ORIGIN: https://github.com/Homebrew/brew
HEAD: bc566b5cd0d540a8afda2bbaac0c395823d66ec0
Last commit: 5 hours ago
Core tap JSON: 31 Dec 22:29 UTC
Core cask tap JSON: 31 Dec 22:29 UTC
HOMEBREW_PREFIX: /usr/local
HOMEBREW_CASK_OPTS: []
HOMEBREW_EDITOR: nvim
HOMEBREW_GITHUB_API_TOKEN: set
HOMEBREW_MAKE_JOBS: 8
HOMEBREW_SORBET_RUNTIME: set
Homebrew Ruby: 3.1.4 => /usr/local/Homebrew/Library/Homebrew/vendor/portable-ruby/3.1.4/bin/ruby
CPU: octa-core 64-bit kabylake
Clang: 15.0.0 build 1500
Git: 2.43.0 => /usr/local/bin/git
Curl: 8.4.0 => /usr/bin/curl
macOS: 14.2.1-x86_64
CLT: 15.1.0.0.1.1700200546
Xcode: N/A

brew doctor

Please note that these warnings are just used to help the Homebrew maintainers
with debugging if you file an issue. If everything you use Homebrew for is
working fine: please don't worry or file an issue; just ignore this. Thanks!

Warning: Some installed formulae are deprecated or disabled.
You should find replacements for the following formulae:
  dog

Warning: You have unlinked kegs in your Cellar.
Leaving kegs unlinked can lead to build-trouble and cause formulae that depend on
those kegs to fail to run properly once built. Run `brew link` on these:
  qmk
  pycparser
  cffi
  pillow
  tfenv
  python-cryptography
  libtorrent-rakshasa
  gnupg

Warning: Homebrew's "sbin" was not found in your PATH but you have installed
formulae that put executables in /usr/local/sbin.
Consider setting your PATH for example like so:
  echo 'export PATH="/usr/local/sbin:$PATH"' >> /Users/jindraj/.bash_profile

Verification

  • My "brew doctor output" says Your system is ready to brew. and am still able to reproduce my issue.
  • I ran brew update and am still able to reproduce my issue.
  • I have resolved all warnings from brew doctor and that did not fix my problem.
  • I searched for recent similar issues at https://github.com/Homebrew/homebrew-core/issues?q=is%3Aissue and found no duplicates.

What were you trying to do (and why)?

I want truecolor support in my terminal, I'm using bash as my shell interpret and iTerm2 terminal emulator on macOS. To do so I've set TERM=xterm-direct, but since then the prompt wrapping is broken. I limited the issue to be within the shell, since it works fine on remote machines using the exact terminal emulator from my local. The issue is that homebrew bash is built against system ncurses, which is quite old and so is the terminfo database which lacks the xterm-direct record. As a test I've compiled bash from source against the homebrew ncurses and it works fine.

More details

bash doesn't know how to handle xterm-direct TERM because ncurses supplied with macOS against which it is compiled are quite old and do not have xterm-direct in their terminfo.

brew bash

otool -L /usr/local/opt/bash/bin/bash

    /usr/local/opt/bash/bin/bash:
	/usr/lib/libncurses.5.4.dylib (compatibility version 5.4.0, current version 5.4.0)
	/usr/lib/libiconv.2.dylib (compatibility version 7.0.0, current version 7.0.0)
	/System/Library/Frameworks/CoreFoundation.framework/Versions/A/CoreFoundation (compatibility version 150.0.0, current version 2048.1.255)
	/usr/lib/libSystem.B.dylib (compatibility version 1.0.0, current version 1336.0.0)

asciicast

bash compiled from source against brew ncurses

otool -L /usr/local//bin/bash

/usr/local/bin/bash:
	/usr/local/opt/ncurses/lib/libncursesw.6.dylib (compatibility version 6.0.0, current version 6.0.0)
	/usr/local/opt/gettext/lib/libintl.8.dylib (compatibility version 13.0.0, current version 13.0.0)
	/System/Library/Frameworks/CoreFoundation.framework/Versions/A/CoreFoundation (compatibility version 150.0.0, current version 2202.0.0)
	/usr/local/opt/libiconv/lib/libiconv.2.dylib (compatibility version 9.0.0, current version 9.1.0)
	/usr/lib/libSystem.B.dylib (compatibility version 1.0.0, current version 1336.61.1)

asciicast

What happened (include all command output)?

as described above, bash wraps prompt line pooly when TERM is set to xterm-direct and it's compiled against macOS supplied ncurses.

What did you expect to happen?

I'd like bash formula to have the homebrew ncurses as an dependency, be compiled against it.

Step-by-step reproduction instructions (by running brew commands)

N/A
@jindraj jindraj added the bug Reproducible Homebrew/homebrew-core bug label Dec 31, 2023
@SMillerDev
Copy link
Member

I'd like bash formula to have the homebrew ncurses as an dependency, be compiled against it.

You can make a pull request, but since homebrew prefers to build against system tools on macOS I don't think this will be favoured over the current approach.

@jindraj
Copy link
Contributor Author

jindraj commented Jan 1, 2024

I understand the effort to keep the deps on system level. In this specific scenario the system ncurses is 20y old. Other shells like zsh or fish do have ncurses in deps.
PR #158683

@iMichka
Copy link
Member

iMichka commented Jan 7, 2024

If we accepted it for zsh or fish, I would expect us to accept it for bash too. That would be more consistent.

We have more than 80 formulae that depends_on "ncurses" (brew version) on macOS, whereas around 250 use system ncurses. I am unsure how consistent we have been here over the years.
Thoughts @Homebrew/core ?

@iMichka iMichka added the maintainer feedback Additional maintainers' opinions may be needed label Jan 7, 2024
@carlocab
Copy link
Member

carlocab commented Jan 8, 2024

bash is a little different from the formulae we've switched to Homebrew ncurses recently, though. It has many more users, and it's also trickier to work around being broken. Note also that ncurses seems to have been updated in newer versions of macOS (the bundled one is still old though, I think).

That said, I'm not necessarily opposed to doing the same for bash. I just want to make sure we've considered it carefully before doing so.

@cho-m
Copy link
Member

cho-m commented Jan 8, 2024

The other examples were more on the topic of overall consistency (brew vs system). For this, we either need to just accept the inconsistency or the only option is brew ncurses due to those issues.

We would need to go farther back for examples with similar user count as bash like the ones already mentioned in thread, e.g.

It sounds like bash situation is similar to the above.

@MikeMcQuaid
Copy link
Member

Note also that ncurses seems to have been updated in newer versions of macOS (the bundled one is still old though, I think).

Everything in macOS is "old". I'd love if we can avoid using dependencies universally like these unless we've verified the bug applies on the newest macOS with the system nurses there.

@fxcoudert
Copy link
Member

Documentation of TERM=xterm-direct says:

To get this to work, at this date (2019), you need three things:

  • The ncurses library version 6.1 or newer must be installed.

So given the macOS ncurses library is older, I'm tempted to say it's not a bug, it's a lack of feature in the system.

@carlocab
Copy link
Member

carlocab commented Jan 9, 2024

So given the macOS ncurses library is older, I'm tempted to say it's not a bug, it's a lack of feature in the system.

This feels like splitting hairs. I'm open to saying that it's a bug but one that's too costly to try to fix. What would make me say it's worth trying to fix would be:

  1. the bug is present even with the ncurses that comes with the latest version of macOS (as per Mike's comment)
  2. multiple users (say, 3+) report having issues with bash being linked to the older system-provided ncurses

@iMichka
Copy link
Member

iMichka commented Jan 23, 2024

What do we need to get this moving forward? This was a great discussion with interesting insights, but what is the conclusion?

All in all I feel that:

  • The issue is a bug, and that bug needs to be solved
  • The cost of adding ncurses here seem low
  • I think consistence with other formulae makes sense here

Should we discuss this at the AGM in Brussels in 2 weeks? Or should this go to the TSC for a quick vote ASAP?

@p-linnane
Copy link
Member

Utilizing system ncurses is limiting newer functionality. I feel like that's a good reason to use brewed ncurses.

@MikeMcQuaid
Copy link
Member

  • The issue is a bug, and that bug needs to be solved

The issue is a bug affecting a specific configuration which is a small proportion of people using the bash formula.

  • The cost of adding ncurses here seem low

The cost is adding an additional dependency to bash to literally everyone who uses the formula, increasing the installation time of bash, the CI time for ncurses and the perception that "Homebrew is slow".

  • I think consistence with other formulae makes sense here

I agree consistency is desirable and we should aim to document this consistency. That said, I still think it's worth considering "should we add this dependency" on a case-by-case basis.

Utilizing system ncurses is limiting newer functionality. I feel like that's a good reason to use brewed ncurses.

As noted above: I don't think that's a slam-dunk answer; it is contextual depending on how many people benefit from the new functionality vs. face the costs of the additional dependency.

Should we discuss this at the AGM in Brussels in 2 weeks? Or should this go to the TSC for a quick vote ASAP?

Neither: we can discuss this here. I am not blocking or strongly opposed to adding the dependency. I just ask that the above be considered here.

Adding a new dependency is always "easy", but it also has costs.

the bug is present even with the ncurses that comes with the latest version of macOS (as per Mike's comment)

I am assuming given the reporter is on 14.2.1-x86_64 that this applies e.g. also on ARM and on all latest macOS versions but would love a maintainer to reproduce this on ARM and/or on macOS 14, too.

@jindraj
Copy link
Contributor Author

jindraj commented Jan 29, 2024

I am assuming given the reporter is on 14.2.1-x86_64 that this applies e.g. also on ARM and on all latest macOS versions but would love a maintainer to reproduce this on ARM and/or on macOS 14, too.

Yes I'm on macOS 14.2.1 and it behaves the same on both archs.

@iMichka
Copy link
Member

iMichka commented Feb 12, 2024

Looks like this got stuck as we could not reach a decision easily.

What if we add the dependency, but add a comment in the formula linking to this issue, asking to consider reverting back to system ncurses once Apple has updated? At least we would keep track of why we added it.

@MikeMcQuaid
Copy link
Member

I am fine with adding the dependency 👍🏻

iMichka added a commit to iMichka/homebrew-core that referenced this issue Feb 22, 2024
See Homebrew#158667

This is an EXCEPTION to our rules: do not take this as a sign to add brewed ncurses everywhere.
We will drop brewed ncurses here once Apple has updated to a newer version
Copy link
Contributor

github-actions bot commented Mar 5, 2024

This issue has been automatically marked as stale because it has not had recent activity. It will be closed if no further activity occurs.

@github-actions github-actions bot added the stale No recent activity label Mar 5, 2024
@fxcoudert
Copy link
Member

Closing: a decision has been reached, but the PR is stuck for now for technical reasons. We'll continue to work on it.

@github-actions github-actions bot added the outdated PR was locked due to age label Apr 4, 2024
@github-actions github-actions bot locked as resolved and limited conversation to collaborators Apr 4, 2024
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
bug Reproducible Homebrew/homebrew-core bug maintainer feedback Additional maintainers' opinions may be needed outdated PR was locked due to age stale No recent activity
Projects
None yet
Development

Successfully merging a pull request may close this issue.

8 participants