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

Remove mkmf as a dependency #5

Merged
merged 1 commit into from
Jun 1, 2016

Conversation

botandrose
Copy link
Collaborator

Requiring mkmf has some unfortunate side-effects, like polluting the top-level namespace causing naming collisions with e.g. ffi gem (see pbhogan/scrypt#47). In fact, it turns out that Ruby core recommends only requiring mkmf in extconf.rb, and not anywhere else (see https://bugs.ruby-lang.org/issues/12370#note-4).

Therefore, this PR removes the mkmf dependency, and relies on Open3.popenx methods to be able to find the executables themselves via the system PATH. This also simplifies things a bit, removing the need for the elm_executable variable.

What do you think?

@fbonetti
Copy link
Owner

fbonetti commented Jun 1, 2016

I was wondering why you removed mkmf in your last PR but this makes perfect sense now. Thanks for opening this. I'll release this as a patch.

@fbonetti fbonetti merged commit 3294492 into fbonetti:master Jun 1, 2016
@botandrose
Copy link
Collaborator Author

🎉

@botandrose botandrose deleted the remove_mkmf_as_a_dependency branch June 1, 2016 03:42
@fbonetti
Copy link
Owner

fbonetti commented Jun 1, 2016

@botandrose botandrose restored the remove_mkmf_as_a_dependency branch November 16, 2017 00:19
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.

3 participants