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: install npm_config_platform=win32 #61

Merged
merged 2 commits into from
Apr 30, 2020

Conversation

dimadeveatii
Copy link
Contributor

@dimadeveatii dimadeveatii commented Apr 29, 2020

Closes #60

Summary
This PR fixes an issue when trying to install electron on a linux machine with environment variable npm_config_platform=win32.

Motivation
There is support for building electron apps on linux for windows. The npm_config_platform is an environment variable that is usually used to instruct npm packages about the platform for which custom or pre-built binaries should be downloaded.

For example, running: npm_config_platform=win32 npm i electron-chromedriver should download the Windows chromedriver.exe even when I run it on a linux machine. This is currently broken and attempting to run it on linux fails with an error.

There is not much use in downloading the Windows chromedriver on linux. However, if you have a native dependency like sharp and try to install all packages with npm_config_platform=win32 npm i in order to create a Windows build, you still get the same error from electron-chromedriver package.

@dimadeveatii dimadeveatii requested a review from a team as a code owner April 29, 2020 11:18
@codebytere
Copy link
Member

codebytere commented Apr 29, 2020

@dimadeveatii please fill out the pull request outlining the problem that prompted this fix as well a brief explanation for how this fixes that problem. We will be able to review this once that has been done.

@codebytere codebytere changed the title fix: install npm_config_platform=win32 #60 fix: install npm_config_platform=win32 Apr 29, 2020
@dimadeveatii
Copy link
Contributor Author

Thank you @codebytere, will do that. Thought that linking the issue is enough.

@codebytere
Copy link
Member

codebytere commented Apr 30, 2020

@dimadeveatii no worries - it's more just to make sure the history chains are over-communicated for posterity. Thanks!

@codebytere codebytere merged commit 3d0e75f into electron:master Apr 30, 2020
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.

Running with npm_config_platform=win32 on linux fails
2 participants