Skip to content
This repository has been archived by the owner on Apr 14, 2021. It is now read-only.

Shellsplit build config #7023

Merged
1 commit merged into from
Apr 4, 2019
Merged

Shellsplit build config #7023

1 commit merged into from
Apr 4, 2019

Conversation

deivid-rodriguez
Copy link
Member

@deivid-rodriguez deivid-rodriguez commented Mar 8, 2019

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

The problem was #6940. Build configurations with multiple whitepaced parameter are no longer properly handled.

What was your diagnosis of the problem?

My diagnosis was not mine, was @jeremy's: Since shellscaping / shelljoining build arguments to the rubygems gem installer, these are no longer properly passed

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

My fix is not mine, it's @jeremy's: shellsplit settings before passing them.

Why did you choose this fix out of the possible options?

I chose this fix because it works and it sounds like the proper followup to rubygems/rubygems#2441.

Fixes #6940.

@jeremy
Copy link
Contributor

jeremy commented Mar 8, 2019

Might be nice to have stored these args as an array in the first place, too! With backward compatibility support for single-string build args. Separate PR though, I imagine.

For this PR, could broaden test coverage in spec/install/gems/native_extensions_spec.rb as well, since that tests actually passing the build args to extconf.rb. That uses a single build arg now, but should be proven to work with multiple args.

@deivid-rodriguez
Copy link
Member Author

I added the suggested spec, sorry for the delay!

@deivid-rodriguez
Copy link
Member Author

Feedback was addressed and @hsbt already approved the initial version, so going in!

@bundlerbot r+

ghost pushed a commit that referenced this pull request Apr 4, 2019
7023: Shellsplit build config r=deivid-rodriguez a=deivid-rodriguez

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

The problem was #6940. Build configurations with multiple whitepaced parameter are no longer properly handled.

### What was your diagnosis of the problem?

My diagnosis was not mine, was @jeremy's: Since [shellscaping / shelljoining build arguments to the rubygems gem installer](#6940), these are no longer properly passed

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

My fix is not mine, it's @jeremy's: shellsplit settings before passing them.

### Why did you choose this fix out of the possible options?

I chose this fix because it works and it sounds like the proper followup to rubygems/rubygems#2441.

Fixes #6940.

Co-authored-by: David Rodríguez <[email protected]>
@ghost
Copy link

ghost commented Apr 4, 2019

Build succeeded

@ghost ghost merged commit 70cb2f0 into master Apr 4, 2019
@ghost ghost deleted the shellsplit_build_config branch April 4, 2019 20:54
This pull request was closed.
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

bundle config build.<gem> doesn't send multiple parameters to extconf.rb correctly
3 participants