build changes cwd

[breavyn]

Issue Type

• Bug Report
• Feature Request
• Other

Current/Missing Behaviour

Calling nwbuild with mode: "build" and manageManifest will change the processes cwd to use the package manager. This requires the caller to save and restore the cwd when using nwbuild.

nw-builder/src/bld.js

Line 233 in 8f4e07d

process.chdir(

A similar issue exists here also.

nw-builder/src/bld.js

Line 421 in 8f4e07d

const buildNativeAddon = ({ cacheDir, version, platform, arch, outDir, nodeVersion }) => {

Expected/Proposed Behaviour

The package manager should be executed with the needed cwd, without changing the calling processes cwd.

I can submit a PR if desired.

[ayushmanchhabra]

@breavyn PR would be great. But before that would like to understand how this is a bug? How does saving and restoring the cwd lead to buggy behaviour with respect to the built application?

[breavyn]

The nwjs application itself is built correctly. However, silently changing the cwd is hostile to users trying to call nwbuild from a script. It causes any further filesystem related operations to have unintended outcomes.

I propose either not changing the cwd in the first place, which is a trivial change. Or clearly documenting that this is intended behaviour, suggesting the user save and restore the cwd themselves if needed.

const cwd = process.cwd();
await nwbuild({...});
process.chdir(cwd);

Keeping in mind changing this behaviour would break scripts relying on the cwd change.

[ayushmanchhabra]

@breavyn I understand now, please go ahead with the pull request.

Keeping in mind changing this behaviour would break scripts relying on the cwd change.

I think as long as the application builds correctly in managed manifest mode, it should be fine.