Skip to content
This repository has been archived by the owner on Aug 11, 2022. It is now read-only.

bin scripts should always have Unix line endings #12371

Closed
othiym23 opened this issue Apr 15, 2016 · 7 comments
Closed

bin scripts should always have Unix line endings #12371

othiym23 opened this issue Apr 15, 2016 · 7 comments

Comments

@othiym23
Copy link
Contributor

othiym23 commented Apr 15, 2016

See #2097 for context and discussion. When publishing from Windows, depending on how developers set up Git, it's possible that binary scripts will end up in the tarball with CR LF line endings, instead of the LF expected by Unix / OS X. This can cause failures when those scripts are invoked. The logic that Git applies to figure out whether a file is binary or not is surprisingly subtle, and this is the sort of thing that can cause really hard to debug issues if not handled correctly. This is the heuristic @iarna and I have come up with for figuring out which files need their line endings cleaned up:

  1. Is it linked to from bin?
  2. Is the first line a shebang?
    cannot perform relative import from bin #2 is necessary in the case in which bin links to a native binary, which we don't want to mangle during the publish or installation processes.

To be a complete solution, this would need to be some kind of stream transform that gets applied at both publish and install time – publish, so that newly-published packages have the correct line endings, and install, so that non-Windows users running installs with the fixed version of the CLI aren't bitten by legacy packages with the wrong line endings in their scripts.

@othiym23 othiym23 changed the title bin scripts should always be published with Unix line endings bin scripts should always have Unix line endings Apr 15, 2016
enlight added a commit to enlight/electron-inspector that referenced this issue Sep 15, 2016
NPM publishes files without converting line endings. Files in the bin
directory are supposed to be run directly by the shell, and if they
contain Windows line endings then macOS fails to interpret the hash-bang
correctly. There's an open issue in the NPM repo regarding this
(npm/npm#12371).
@ikatun
Copy link

ikatun commented Apr 7, 2017

Just add "prepublish": "dos2unix index.js" line to your "scripts" in package.json.

Also make sure that dos2unix is added to the dev dependencies (npm install --saveDev dos2unix).

Note:
Prepublish script is also executed when doing local install (npm install path/to/your/local/module)

@s-h-a-d-o-w
Copy link

s-h-a-d-o-w commented Apr 11, 2017

dos2unix doesn't offer a CLI, so that won't work.

What does work however is the tool mentioned here: #2097 (comment)

To do the one-way conversion that you seem to describe, one would simply have to do:

    "prepublish": "crlf --set=LF bin/*"

If one wants to convert back to CRLF after publishing, to get the "Git for Windows" behavior (see: #2097 (comment)), one would need to use some dirty hacks to ensure that it doesn't break anything on non-Windows machines (i.e. line endings should be/stay LF on e.g. Mac and publishing shouldn't fail due to using IF to detect whether the OS is Windows):

    "prepublish": "true || IF a==a crlf --set=LF bin/*",
    "postpublish": "true || IF a==a crlf --set=CRLF bin/*",

It's not pretty but by failing silently on anything that isn't Windows (unless one runs it in some weird bash that maybe doesn't know "true ||" or that does understand "IF"), it avoids having to e.g. create a separate script that does OS checking and that one would need to include in every single package.

@marcosscriven
Copy link

The fix for this issue unfortunately caused a new one #19151

@janwidmer
Copy link

janwidmer commented Apr 23, 2018

Is there any new solution to solve the publish problem besides using prebublish and postpublish in combination with the clrf plugin?

We are experiencing the original problem descriped here #2097 with .githook files being generated by a yeoman generator

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

Successfully merging a pull request may close this issue.

8 participants