Skip to content
This repository has been archived by the owner on Jul 24, 2024. It is now read-only.

Accept process.execPath as-is #1325

Merged
merged 1 commit into from
Jan 20, 2016
Merged

Accept process.execPath as-is #1325

merged 1 commit into from
Jan 20, 2016

Conversation

saper
Copy link
Member

@saper saper commented Jan 3, 2016

Remove red tape surrounding process.execPath
and use it as-is. In particular, do not try
to canonicalize the path returned by
process.execPath as this might be relative
in the restricted Linux environments
(for example using overlayfs).

We should be prepared to accept relative
or otherwise broken process.execPath and
have a faith it works (otherwise our
build scripts wouldn't be invoked
in the first place).

#1323

Remove red tape surrounding process.execPath
and use it as-is. In particular, do not try
to canonicalize the path returned by
process.execPath as this might be relative
in the restricted Linux environments
(for example using overlayfs).

We should be prepared to accept relative
or otherwise broken process.execPath and
have a faith it works (otherwise our
build scripts wouldn't be invoked
in the first place).

sass#1323
@saper
Copy link
Member Author

saper commented Jan 3, 2016

After a little consideration this is not a bug in the node's fs module either; process.execPath should be considered existing relative to the OS-dependent PATH resolution mechanism and not relative to the current directory wherever our scripts may be executed. In other words, we try to fs.lstatSync('node') with a current directory that does not have a file named node at all and we correctly get ENOENT in return.

@xzyfer
Copy link
Contributor

xzyfer commented Jan 5, 2016

Are you sure this isn't introducing a regression? I recall this being added for specific reason.

#668

@saper
Copy link
Member Author

saper commented Jan 5, 2016

I am never sure, I always check. Without libuv/libuv#293 (or similar) merged we are just getting the working path to the executable - we do not care here at all whether it is symlinked or pulled out of the moon. Even relative path will work here as long as it is in some directory on PATH. (See even worse problem on OpenBSD libuv/libuv#652). We just want to call "node", preferably the one we are called with. There is no superportable way to do this 100% correctly, but this approach will work.

Next time I'll boot my Windows box I'll check of course.

@Mischi
Copy link

Mischi commented Jan 10, 2016

👍

I've verified that this PR would fix node-sass on OpenBSD if process.execPath is relative.

@xzyfer
Copy link
Contributor

xzyfer commented Jan 20, 2016

On further thought this makes sense.

xzyfer added a commit that referenced this pull request Jan 20, 2016
@xzyfer xzyfer merged commit fa40bda into sass:master Jan 20, 2016
@saper saper mentioned this pull request Feb 14, 2016
@xzyfer xzyfer modified the milestone: next.patch Sep 4, 2016
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants