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

Issue with line endings #14

Closed
ksmth opened this issue Dec 13, 2013 · 24 comments
Closed

Issue with line endings #14

ksmth opened this issue Dec 13, 2013 · 24 comments

Comments

@ksmth
Copy link

ksmth commented Dec 13, 2013

I'm running OSX 10.9 and when I try running codepainter after installing it through npm install -g codepainter I recieve an error saying that node\r can't be found. After changing the line endings from Windows to Unix the error disappears. I tried creating a fork, but somehow git doesn't pick up the changes of the line endings.

@jednano
Copy link
Owner

jednano commented Dec 13, 2013

Do you know what file it was that was throwing the error?

The reason it wasn't picking up the changes to the line endings is, I believe, due to the .gitattributes file at the root of the project. Remove that file and it'll pick up the changes.

I want to move the file to a different location though. Thing is, I need the test input and sample files to maintain inconsistent line endings. All other files need not do this. Not quite sure how to handle the .gitattributes file to make this happen though.

@ksmth
Copy link
Author

ksmth commented Dec 13, 2013

It was bin/codepaint. Maybe you could work around this issue with an .editorconfig file instead of that .gitattributes file? As far as I know you can put a file in each folder that need a different setup. → http://editorconfig.org

@jednano
Copy link
Owner

jednano commented Dec 13, 2013

Haha – yeah, I'm quite familiar with editorconfig. I'm one of the contributors. That doesn't prevent people from committing code that has inconsistent line endings though. That's really what I'm trying to prevent here.

@jednano
Copy link
Owner

jednano commented Dec 13, 2013

And BTW, I already have an .editorconfig file in place and it's set to have lf line endings everywhere outside of the tests folder. I'm also doing a regex search on bin/codepaint and I see no \r characters anywhere in the file. Maybe it has something to do with the #! at the top? Can you try playing with that? I'm not so savvy about Macs.

@ksmth
Copy link
Author

ksmth commented Dec 13, 2013

So, I have just run the following command:

perl -p -e 's[\r\n][WIN\n];' codepaint

And it outputs:

#!/usr/bin/env nodeWIN
WIN
var clc = require('cli-color');WIN
var crypto = require('crypto');WIN
var editorconfig = require('editorconfig');WIN
var extend = require('node.extend');WIN
var fs = require('fs');WIN
var glob = require('glob');WIN
var path = require('path');WIN
var program = require('gitlike-cli');WIN
WIN
...

So my best guess is, that \r is in fact the default line ending character for windows and is interpreted as an actual character by /usr/bin/env.

@jednano
Copy link
Owner

jednano commented Dec 13, 2013

I cannot reproduce this.

Try this – download the raw codepaint file from this link and inspect the file, bypassing git checkout. I'm not seeing any Windows \r\n line endings in this file. In fact, there are only lf linux line endings.

@ksmth
Copy link
Author

ksmth commented Dec 16, 2013

That's totally true. Using that file, I can't reproduce this issue either. When I install it through npm, I do get the error though. I'm totally lost on how to fix this.

@jednano
Copy link
Owner

jednano commented Dec 16, 2013

Shot in the dark -- what version of codepainter does it say you have installed in package.json?

@ksmth
Copy link
Author

ksmth commented Dec 16, 2013

version 0.3.24

@jednano
Copy link
Owner

jednano commented Dec 16, 2013

It seems highly unlikely that Git or GitHub would be the culprit here for changing line endings on checkout, but it definitely sounds like that is what's happening, right? Maybe NPM is to blame?

I also tried downloading the source directly from the 0.3.24 release here but I also don't see any \r\n Windows line endings.

There were issues with line endings before, which @treyhunner brought to my attention, I believe. This is why I installed a .gitattributes file, but these issues have since been resolved. It would be a shame if somehow the files on GitHub's server are tainted.

@ksmth
Copy link
Author

ksmth commented Dec 16, 2013

Tomorrow I'll have access to another mac developer machine. I'm going to try to reproduce it there and report back as soon as I know more.

@atroche
Copy link

atroche commented Dec 19, 2013

Not sure if this is useful, but:

❯❯❯ npm install codepainter
❯❯❯ codepaint
env: node\r: No such file or directory

OS X 10.8.5.

@jednano
Copy link
Owner

jednano commented Dec 19, 2013

@atroche thanks, but it's a bit misleading. I worry that you have multiple versions running if you didn't install via npm install -g codepainter. Are you sure the right one is running?

So we have this failing on Mac OS X 10.8.5 and 10.9.

Help me out here guys! I don't have a Mac at all. Any ideas?

@tleish
Copy link

tleish commented Dec 19, 2013

Also broken for me on Mac OSX 10.9.

$ codepainter --version
env: node\r: No such file or directory

LOCAL FIX
To fix this for me, I installed dos2unix and used it to remove all the windows CRLF line in codepainter:

$ brew install dos2unix
$ cd [/path/to/codepainter/]
$ find . -type f -print0 | xargs -0 dos2unix
$ codepainter --version
0.3.24

ADDITIONAL INFORMATION

I checked the files from git and from the npm package for windows carriage returns with the following results.

GIT CLONE: FILES WITH WINDOWS CRLF

$ find . -type f -exec file "{}" ";" | grep CRLF
./test/cases/end_of_line/crlf/expected.js: ASCII text, with CRLF line terminators
./test/cases/end_of_line/crlf/sample.js: UTF-8 Unicode (with BOM) text, with CRLF, LF line terminators
./test/cases/end_of_line/lf/input.js: ASCII text, with CRLF line terminators
./test/cases/end_of_line/lf/sample.js: UTF-8 Unicode (with BOM) text, with CRLF, LF line terminators

NPM PACKAGE: FILES WITH WINDOWS CRLF

$ find . -type f -exec file "{}" ";" | grep CRLF
./0.3.24/package/codepainter.js: ASCII text, with CRLF line terminators
./0.3.24/package/lib/Error.js: ASCII text, with CRLF line terminators
./0.3.24/package/lib/Inferrer.js: UTF-8 Unicode (with BOM) text, with CRLF line terminators
./0.3.24/package/lib/MultiInferrer.js: UTF-8 Unicode (with BOM) text, with CRLF line terminators
./0.3.24/package/lib/Object.js: ASCII text, with CRLF line terminators
./0.3.24/package/lib/Pipe.js: ASCII text, with CRLF line terminators
./0.3.24/package/lib/Rule.js: ASCII text, with CRLF line terminators
./0.3.24/package/lib/rules/end_of_line.js: ASCII text, with CRLF line terminators
./0.3.24/package/lib/rules/indent_style_and_size.js: ASCII C++ program text, with CRLF line terminators
./0.3.24/package/lib/rules/index.js: ASCII text, with CRLF line terminators
./0.3.24/package/lib/rules/insert_final_newline.js: ASCII text, with CRLF line terminators
./0.3.24/package/lib/rules/quote_type.js: ASCII C++ program text, with CRLF line terminators
./0.3.24/package/lib/rules/space_after_anonymous_functions.js: ASCII C++ program text, with CRLF line terminators
./0.3.24/package/lib/rules/space_after_control_statements.js: ASCII text, with CRLF line terminators
./0.3.24/package/lib/rules/spaces_around_operators.js: ASCII text, with CRLF line terminators
./0.3.24/package/lib/rules/spaces_in_brackets.js: ASCII text, with CRLF line terminators
./0.3.24/package/lib/rules/trim_trailing_whitespace.js: ASCII text, with CRLF line terminators
./0.3.24/package/lib/Serializer.js: ASCII text, with CRLF line terminators
./0.3.24/package/lib/styles/codepainter.json: ASCII text, with CRLF line terminators
./0.3.24/package/lib/styles/hautelook.json: ASCII text, with CRLF line terminators
./0.3.24/package/lib/styles/idiomatic.json: ASCII text, with CRLF line terminators
./0.3.24/package/lib/styles/mediawiki.json: ASCII text, with CRLF line terminators
./0.3.24/package/lib/Tokenizer.js: ASCII English text, with CRLF line terminators
./0.3.24/package/lib/Transformer.js: ASCII text, with CRLF line terminators
./0.3.24/package/lib/util/index.js: ASCII text, with CRLF line terminators
./0.3.24/package/lib/util/string.js: ASCII text, with CRLF line terminators
./0.3.24/package/LICENSE: ASCII English text, with very long lines, with CRLF line terminators
./0.3.24/package/README.md: UTF-8 Unicode English text, with CRLF line terminators

So, while github doesn't have windows line feeds, the npm package does. From my understanding, this may occur using npm publish from a windows machine.

REPORTED ISSUE:
npm publish from Windows handles CRLF incorrectly
https://github.com/isaacs/npm/issues/2097

POSSIBLE WORKAROUND
Developing node modules, npm and git. how to publish npm packages from a windows machine whilst preserving the unix style line endings
http://foldingair.blogspot.com/2013/05/developing-node-modules-npm-and-git-how.html

@jednano
Copy link
Owner

jednano commented Dec 20, 2013

@tleish thank you so much for the extensive coverage on this issue. Unfortunately, I think your solution only works if the local files have Windows line endings when you run npm publish, which mine do not, so....

I tried this – and please someone tell me if it works now – but version 0.3.25 was created like so:

$ npm version patch -m "Patch version: 0.3.25"
$ git push
$ git push --tags
$ npm publish https://github.com/jedmao/codepainter/archive/v0.3.25.tar.gz
npm http GET https://github.com/jedmao/codepainter/archive/v0.3.25.tar.gz
npm http 200 https://github.com/jedmao/codepainter/archive/v0.3.25.tar.gz
npm http PUT https://registry.npmjs.org/codepainter
npm http 409 https://registry.npmjs.org/codepainter
npm http GET https://registry.npmjs.org/codepainter
npm http 200 https://registry.npmjs.org/codepainter
npm http PUT https://registry.npmjs.org/codepainter/-/codepainter-0.3.25.tgz/-re
v/85-1b6e883d356e3e8e6d49478818fc55bb
npm http 201 https://registry.npmjs.org/codepainter/-/codepainter-0.3.25.tgz/-re
v/85-1b6e883d356e3e8e6d49478818fc55bb
npm http PUT https://registry.npmjs.org/codepainter/0.3.25/-tag/latest
npm http 201 https://registry.npmjs.org/codepainter/0.3.25/-tag/latest
+ codepainter@0.3.25

Hopefully, this has resolved the issue. Someone, please try-out version 0.3.25 and tell me if it's resolved. Thanks!

@ksmth
Copy link
Author

ksmth commented Dec 20, 2013

Works perfectly fine for me now.

@jednano
Copy link
Owner

jednano commented Dec 20, 2013

BTW, I find it interesting that Travis CI doesn't run into this issue. I need to figure out a way to automate this for every publish >_<

@jednano jednano closed this as completed Dec 20, 2013
@tleish
Copy link

tleish commented Dec 20, 2013

Fixed for me as well. Uninstalled codepainter, cleared cache, and re-installed codepainter.

I checked the package, and no more windows CRLF

$ find . -type f -exec file "{}" ";" | grep CRLF
$ (no files listed, which is good)
$ codepainter --version
0.3.25

No more need for post-install fixing.

Thanks for the figuring out the fix. Curious, how is what you did different from what you were previosly doing?

@jednano
Copy link
Owner

jednano commented Dec 20, 2013

@tleish, previously I was just doing $ npm publish after pushing my tag, without specifying a tarball. NPM probably just takes the URL from the package.json repository URL in this case – not sure. For whatever reason though, pointing directly to the tarball bypasses whatever issue NPM was presenting. It just makes the publish process more involved. I'm going to update my bash script to have a way to automate this. I had it setup previously where I could just type patch and it does all the version updates and tag stuff for me in one shot.

@spion
Copy link

spion commented Feb 10, 2014

Wont generating something like

#!/path/to/nodebinary
require('./path/to/realbinary.js')

work much better? It will also solve the issue with node being named differently on certain distros.

Possible backward-compatibility breakage: require.main (undocumented), module.parent (documented) but it may be possible to resolve these by not directly using require...

Another alternative is to just fix the shebang newline of all files listed in bin that have one, but on install. Doing it at install time rather than at packaging time will also allow npm to replace the path to the node binary to use the correct name if a solution is found that can handle the myriad of different ways of writing a correct shebang

@jednano
Copy link
Owner

jednano commented Feb 10, 2014

This issue is not with shebang, but the newline characters in the bin file. Your solution would still have two newline characters; thus, still fail.

@spion
Copy link

spion commented Feb 10, 2014

I actually posted on the wrong issue; I meant to post on the npm issue. Whoops, sorry about that!

By the way, afaik its enough to only replace the shebang line's newline character(s) to get the shell to recognize the binary, which is what my second suggestion is.

@jednano
Copy link
Owner

jednano commented Feb 10, 2014

Sorry, but even that doesn't change anything. My newline characters are \n on my local machine, but they still get changed somehow. Only publishing the tarball fixes the issue.

@jednano
Copy link
Owner

jednano commented Feb 10, 2014

@spion what NPM issue?

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

No branches or pull requests

5 participants