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

feat: support windows shebang #366

Merged
merged 2 commits into from
Oct 18, 2021
Merged

feat: support windows shebang #366

merged 2 commits into from
Oct 18, 2021

Conversation

vagusX
Copy link
Contributor

@vagusX vagusX commented Oct 14, 2021

部分项目的 bin 文件的 shebang 行用了 CRLF,因此使用 npminstall 时会报错 env: node\r: No such file or directory,但是在 npm 下是可以的,因为 npm 内部用了 fixBin 来解决这个问题,即在尝试修正 shebang 行的中的 \r\n\n


npm 那边是 shim-bin 处理也会调用 fixBin 去修复 \r\n 的问题,但貌似 windows 下是没有必要的 https://github.com/npm/bin-links/blob/0d336269938bf47a1532bb8e234e2ac2f8230327/lib/shim-bin.js#L75

因此这个 PR 只处理了非 window 的情况

References

Some issues from those packages:

@vagusX vagusX marked this pull request as ready for review October 14, 2021 14:47
Copy link
Contributor

@killagu killagu left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

@vagusX vagusX requested a review from killagu October 15, 2021 10:57
@killagu killagu merged commit 202c9fe into cnpm:master Oct 18, 2021
@killagu
Copy link
Contributor

killagu commented Oct 18, 2021

await utils.forceSymlink(src, dest);
// ensure that bin are executable and not containing
// windows line-endings(CRLF) on the hashbang line
await fixBin(src, 0o755);
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

看起来这就是之前不支持 windows 的关键原因了。。。

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants