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

Restore accepting custom make command with extra options as the make env variable #4271

Merged
merged 1 commit into from Jan 11, 2021

Conversation

terceiro
Copy link
Contributor

@terceiro terceiro commented Jan 11, 2021

What was the end-user or developer problem that led to this PR?

Debian's gem2deb passes a set of build variables to Rubygems extension builders using ENV['make']. These options are used to produce hardened binaries, as well as to help with reproducible builds.

After ad632bb, it no longer works.

What is your fix for the problem, implemented in this PR?

Consider this case and shellsplit it before passing it to process spawning helpers.

Make sure the following tasks are checked

Debian's gem2deb passes a set of build variables to Rubygems extension
builders using ENV['make']. These options are used to produce hardened
binaries, as well as to help with reproducible builds.

This fixes a regression introduced by
ad632bb.
@terceiro terceiro changed the title Gem::Ext::Builder: accept custom command with extra options Gem::Ext::Builder: accept custom make command with extra options Jan 11, 2021
@deivid-rodriguez deivid-rodriguez changed the title Gem::Ext::Builder: accept custom make command with extra options Restore accepting custom make command with extra options as the make env variable Jan 11, 2021
Copy link
Member

@deivid-rodriguez deivid-rodriguez left a comment

Choose a reason for hiding this comment

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

Thanks @terceiro, looks great! Sorry for the regression.

@deivid-rodriguez deivid-rodriguez merged commit fdc5cfa into rubygems:master Jan 11, 2021
@deivid-rodriguez
Copy link
Member

deivid-rodriguez commented Jan 11, 2021

@terceiro I'm planning to get this fix out in about a week.

@deivid-rodriguez
Copy link
Member

@terceiro I was thinking about this, and while I'm happy to fix this regression and get you back on track,I wonder if this is the right thing to do. I would've never expected people to pass command line flags as part of ENV["make"], hence the regression 😅. I was only expecting people to configure the specific path or make variant in there.

Is this something you do often, and that other libraries usually support? Would it be better that we respected a separate environment variable for this, such as, MAKEFLAGS or any other variable commonly used/documented for this?

@terceiro
Copy link
Contributor Author

@deivid-rodriguez in my experience most projects that support overriding commands with environment variables like this will usually handle a command with extra arguments, but I don't really have data to prove it.

Maybe we could use MAKEFLAGS as well, in this case I think rubygems would not need to do anything.

@deivid-rodriguez
Copy link
Member

Yeah, I think you're right. I just remembered how ruby-core customizes the gem CLI command for its environment: https://github.com/ruby/ruby/blob/0036648a420f945624898568bb82bc5f83195d12/tool/runruby.rb#L115.

Thanks for the feedback, your fix is good 👍.

deivid-rodriguez added a commit that referenced this pull request Jan 18, 2021
Restore accepting custom make command with extra options as the `make` env variable

(cherry picked from commit fdc5cfa)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants