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

with-poll flag is passed to configure #744

Merged
merged 1 commit into from
Oct 31, 2024

Conversation

aaronjensen
Copy link
Contributor

Fixes #738

@aaronjensen
Copy link
Contributor Author

Still working on testing this.

@aaronjensen aaronjensen marked this pull request as draft October 30, 2024 18:19
@aaronjensen aaronjensen marked this pull request as ready for review October 30, 2024 18:51
@aaronjensen
Copy link
Contributor Author

Tested with 31. From what I could see, this likely never worked.

Copy link
Owner

@d12frosted d12frosted left a comment

Choose a reason for hiding this comment

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

good catch

@d12frosted d12frosted merged commit df3693f into d12frosted:master Oct 31, 2024
12 checks passed
@d12frosted
Copy link
Owner

You don't need to pass it. We apply a patch:

  local_patch "poll", sha: "59e876f82e6fd8e4583bc2456339eda4f989c86b1e16a02b0726702e95f60825" if build.with? "poll"

or am I missing something?

@aaronjensen
Copy link
Contributor Author

aaronjensen commented Oct 31, 2024

@d12frosted AFAICT, the patch adds a build option that defaults to off, so we need to specify it to enable the new code:

https://github.com/d12frosted/homebrew-emacs-plus/blob/master/patches/emacs-29/poll.patch#L9-L20

@aaronjensen aaronjensen deleted the with-poll-fix branch October 31, 2024 06:45
@d12frosted
Copy link
Owner

Oh. In that case we need to keep the option, but revert the deletion of the following line:

args << "--with-poll" if build.with? "poll"

@aaronjensen
Copy link
Contributor Author

@d12frosted not sure I understand... I moved the line because it was in the wrong place. Do we need it twice for some reason?

@d12frosted
Copy link
Owner

Oh sorry, my bad. Looked at the wrong file 🤦 . Your fix seems legit - adding to args needs to be done BEFORE autogen / configure 😅

And you are right, the commit that added this option never actually enable it 🤦

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.

[General]: with-poll not working
2 participants