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(msvs): do not fix paths in action command arguments #84

Merged
merged 1 commit into from
Dec 16, 2020

Conversation

targos
Copy link
Member

@targos targos commented Nov 29, 2020

BREAKING CHANGE: On Windows, arguments passed to the "action" commands are no longer transformed to replace slashes with backslashes.

BREAKING CHANGE: On Windows, arguments passed to the "action" commands are no longer transformed to replace slashes with backslashes.
@targos
Copy link
Member Author

targos commented Nov 29, 2020

Node.js Windows integration is expected to fail because js2c.py has to take that behavior into account. nodejs/node@5533033 is the fix for it.

@targos
Copy link
Member Author

targos commented Nov 29, 2020

This change is necessary to update to V8 8.8, as it expects the paths passed to torque to always have forward slashes.

@targos
Copy link
Member Author

targos commented Dec 13, 2020

@nodejs/gyp / @nodejs/node-gyp what do you think?

@gengjiawen
Copy link
Member

Anything we can do to fix windows build on this repo. Merge this will most likely breaks CI for other PR.

@targos
Copy link
Member Author

targos commented Dec 16, 2020

it's kind of a chicken and egg problem. We have to land something first either here or in nodejs/node, and CI will be broken until the other change is landed. I think it's ok to break 1 workflow here, if we don't take too much time to update node afterwards

@gengjiawen
Copy link
Member

it's kind of a chicken and egg problem. We have to land something first either here or in nodejs/node, and CI will be broken until the other change is landed. I think it's ok to break 1 workflow here, if we don't take too much time to update node afterwards

Sounds good to me. Let's merge it :)

@gengjiawen gengjiawen merged commit fc22f83 into nodejs:master Dec 16, 2020
@targos targos deleted the fix-actions-windows branch December 16, 2020 08:50
targos added a commit to targos/node-gyp that referenced this pull request Aug 22, 2021
This is a revert of nodejs/gyp-next#84
with a change to the _FixPath function allowing to change the
separator used.

Fixes: nodejs/gyp-next#120
Fixes: nodejs#2485
gengjiawen pushed a commit that referenced this pull request Aug 22, 2021
* fix(msvs): fix paths again in action command arguments

This is a revert of #84
with a change to the _FixPath function allowing to change the
separator used.

Fixes: #120
Fixes: nodejs/node-gyp#2485
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.

2 participants