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

Use brew's readline in macOS autotools GitHub builds #3579

Merged
merged 1 commit into from
Nov 12, 2024

Conversation

d-torrance
Copy link
Member

It's keg-only to avoid conflicting with libedit provided by the system, so we need to specify the paths in order for it to be detected. Otherwise, we end up building it every time!

It's keg-only to avoid conflicting with libedit provided by the
system, so we need to specify the paths in order for it to be
detected.  Otherwise, we end up building it every time!
@d-torrance
Copy link
Member Author

Before

From https://github.com/Macaulay2/M2/actions/runs/11762892834/job/32766260824:

checking for readline/readline.h... yes
checking for library containing rl_set_prompt... -lreadline
checking for library containing rl_completion_matches... none required
checking for library containing readline... none required
checking for library containing add_history... none required
checking whether readline library is new enough (version at least 6)... no, will build it
configure: readline library will be compiled

After

From: https://github.com/Macaulay2/M2/actions/runs/11789232513/job/32837650836?pr=3579

checking for readline/readline.h... yes
checking for library containing rl_set_prompt... -lreadline
checking for library containing rl_completion_matches... none required
checking for library containing readline... none required
checking for library containing add_history... none required
checking whether readline library is new enough (version at least 6)... yes

@mahrud
Copy link
Member

mahrud commented Nov 12, 2024

I don't like a setup in which users "in the know" know which flags they need to provide for the build to succeed and others either get stuck or have to build extra libraries.

I also don't care how autotools works really so I'll abstain from reviewing, but my preferred fix would be something like this:

PATHS ${HOMEBREW_PREFIX}/opt/readline /opt/local /usr/local /usr

If you feel like your fix is good enough then feel free to merge.

@mahrud mahrud removed their request for review November 12, 2024 11:01
@d-torrance
Copy link
Member Author

Ooh, I like how cmake checks the brew directories by default! This PR came out of my experiments putting together the macOS dependencies portion of the autotools wiki page, and poor macOS users have to add a ton of flags manually because of all these keg-only brew packages.

I'll go ahead and merge this for now to avoid building readline unnecessarily, but I would like to work toward a better solution soon.

@d-torrance d-torrance merged commit 1684b70 into Macaulay2:development Nov 12, 2024
5 checks passed
@mahrud
Copy link
Member

mahrud commented Nov 12, 2024

I should say, it only checks the brew directories by default for readline and gdbm. At this point it's probably safe to look there for everything there also, but cmake doesn't do that yet.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants