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

Fix IndexError when parsing GYP files. #1267

Merged
merged 1 commit into from
Oct 20, 2017
Merged

Fix IndexError when parsing GYP files. #1267

merged 1 commit into from
Oct 20, 2017

Conversation

bnoordhuis
Copy link
Member

@bnoordhuis bnoordhuis commented Aug 19, 2017

GYP automatically turns variables ending in _dir, _file or _path into
absolute paths but didn't check for empty strings.

It interacted badly with variables inherited through the environment
from npm, the scripts-prepend-node-path=false setting in particular
because it is turned into npm_config_script_prepend_node_path=.

Fixes: #1217
CI: https://ci.nodejs.org/job/nodegyp-test-pull-request/32/

@refack
Copy link
Contributor

refack commented Aug 20, 2017

@bnoordhuis you want to upstream it?

GYP automatically turns variables ending in _dir, _file or _path into
absolute paths but didn't check for empty strings.

It interacted badly with variables inherited through the environment
from npm, the `scripts-prepend-node-path=false` setting in particular
because it is turned into `npm_config_script_prepend_node_path=`.

Fixes: #1217
PR-URL: #1267
Reviewed-By: Gibson Fahnestock <gibfahn@gmail.com>
Reviewed-By: Refael Ackermann <refack@gmail.com>
@bnoordhuis bnoordhuis closed this Oct 20, 2017
@bnoordhuis bnoordhuis deleted the fix1217 branch October 20, 2017 13:36
@bnoordhuis bnoordhuis merged commit 05d2002 into nodejs:master Oct 20, 2017
@bnoordhuis
Copy link
Member Author

you want to upstream it?

Do you think it's useful? It's not like we pull in changes from upstream anymore.

@refack
Copy link
Contributor

refack commented Oct 20, 2017

you want to upstream it?

Do you think it's useful? It's not like we pull in changes from upstream anymore.

I try to for node core. And there's #1183.
(Plus they have great try-bots)

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.

None yet

3 participants