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

Bug: v1.2.1 has carriage return in bin/cli.js shebang line #44

Closed
l0b0 opened this issue Dec 13, 2021 · 8 comments
Closed

Bug: v1.2.1 has carriage return in bin/cli.js shebang line #44

l0b0 opened this issue Dec 13, 2021 · 8 comments

Comments

@l0b0
Copy link
Collaborator

l0b0 commented Dec 13, 2021

To Reproduce
Steps to reproduce the behavior:

cd "$(mktemp --directory)"
nix-shell --packages yarn --pure --run 'yarn add stac-node-validator@1.2.1
./node_modules/.bin/stac-node-validator'`

Expected behavior

The command should run without issue.

Actual behavior

/usr/bin/env: 'node\r': No such file or directory
/usr/bin/env: use -[v]S to pass options in shebang lines

Additional context

The command works when installed with npm. Presumably it converts newlines at install time to avoid this.

I could automate the release on merge to master if you add the relevant credentials to the repo secrets.

Discovered in this PR.

@m-mohr
Copy link
Collaborator

m-mohr commented Dec 13, 2021

Hmm, I would have expected that the .editorconfig is there to avoid such things. Otherwise the .editorconfig seems pretty useless to me if the editor still adds \r\n instead of \n.

Also, we should add a CI test for this.

Will try to fix and re-release.

m-mohr added a commit that referenced this issue Dec 13, 2021
m-mohr added a commit that referenced this issue Dec 13, 2021
@l0b0
Copy link
Collaborator Author

l0b0 commented Dec 13, 2021

.editorconfig is working fine - the file in the repository doesn't have \r. So it must be your release process that's changing the file. This is why I suggested automating releases.

@m-mohr
Copy link
Collaborator

m-mohr commented Dec 13, 2021

Well, I'm on Windows so this doesn't come completely unexpected.

Trying to get a test for it, but can't reproduce the code snippet you gave above on Windows yet.

m-mohr added a commit that referenced this issue Dec 13, 2021
m-mohr added a commit that referenced this issue Dec 13, 2021
@m-mohr
Copy link
Collaborator

m-mohr commented Dec 13, 2021

The addition of a .gitattributes file that enforces lf for /bin/cli.js should help, will release 1.2.2 shortly.

m-mohr added a commit that referenced this issue Dec 13, 2021
* Add test for CLI execution #44

* Set cli.js to be always have lf line endings #44, update files to comply to .editorconfig
@blacha
Copy link

blacha commented Dec 13, 2021

Looking at previous releases something changed in the release process between releases 1.1.0 and 1.2.0

grabbing the packaged tars from https://registry.npmjs.org/stac-node-validator/ for these two releases shows the \r was added into the file

image

@m-mohr
Copy link
Collaborator

m-mohr commented Dec 13, 2021

Nothing in the release process has changed (intentionally) on my side, the process I do is the same. Must be some tooling that has changed, but how shall I know that if whatever tool changes something in-between?!

@m-mohr
Copy link
Collaborator

m-mohr commented Dec 13, 2021

v1.2.2 has been released, the line ending was lf during the release so that should work now.
Additionally, I've added the .gitattributes file so that non-Linux users should always get the correct lf line ending after git checkout.
Sorry for any inconvenience caused.

@m-mohr m-mohr closed this as completed Dec 13, 2021
@l0b0
Copy link
Collaborator Author

l0b0 commented Dec 14, 2021

Fix confirmed, thanks!

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

Successfully merging a pull request may close this issue.

3 participants