eslint / eslint-release

The ESLint release tool
Other
24 stars 11 forks source link

Handle globs in pkg.files #17

Open btmills opened 7 years ago

btmills commented 7 years ago

During the "Fixing line endings" step, eslint-release expects only exact file paths in pkg.files, not globs. Because of this, including lib/*.js in pkg.files causes eslint-release to throw an exception (included below) during the release process.

The other @eslint projects use lib and index.js, for example, but eslint-plugin-markdown includes a .eslintrc.yml file in the lib/ directory, which we don't want to include in the build.

Discovered in https://github.com/eslint/eslint-plugin-markdown/pull/67 due to a failed Jenkins build:

Fixing line endings
fs.js:839
  return binding.lstat(pathModule._makeLong(path));
                 ^

Error: ENOENT: no such file or directory, lstat 'lib/*.js'
    at Error (native)
    at Object.fs.lstatSync (fs.js:839:18)
    at /var/lib/jenkins/workspace/Releases/eslint-plugin-markdown Release/eslint-plugin-markdown/node_modules/eslint-release/lib/release-ops.js:398:19
    at Array.filter (native)
    at Object.release (/var/lib/jenkins/workspace/Releases/eslint-plugin-markdown Release/eslint-plugin-markdown/node_modules/eslint-release/lib/release-ops.js:397:28)
    at Object.<anonymous> (/var/lib/jenkins/workspace/Releases/eslint-plugin-markdown Release/eslint-plugin-markdown/node_modules/eslint-release/bin/eslint-prerelease.js:35:12)
    at Module._compile (module.js:409:26)
    at Object.Module._extensions..js (module.js:416:10)
    at Module.load (module.js:343:32)
    at Function.Module._load (module.js:300:12)
not-an-aardvark commented 7 years ago

It looks the error occurs in the part of the process where we convert all line endings to unix.

Is it actually necessary to do this? It seems like it would be better to just use the linebreak-style rule and/or .gitattributes rather than changing the contents of the files as part of the release process.

ilyavolodin commented 7 years ago

We added conversion process due to the fact that .gitattributes do not work on the release. They only kick in when you are checking something into git. Since we don't check anything in, but instead publish to NPM, there's a good chance that if your git is not setup correctly and you are running on Windows, you might release broken package. That's what happened to me on the very first release I did with ESLint. It might not be a problem anymore, since we are doing release in Jenkins that's running on Linux box.

not-an-aardvark commented 7 years ago

I think .gitattributes should work on checkout, or else this suggestion is wrong.

Regardless of what git does, if we enable linebreak-style then that rule will catch any issue that appears (although it would still be annoying to have to fix it).

ilyavolodin commented 7 years ago

Sorry, you are right, it does work on checkout, but because builds generate files from time to time, .gitattributes do not apply to them. ESLint build generates couple of json files, and on my first release I was doing in on Windows, they were generated with Windows line endings, which broke ESLint for anyone running on OSX/Linux.

not-an-aardvark commented 7 years ago
ilyavolodin commented 7 years ago

I honestly remember that incident pretty vaguely. It's been a very long time ago (version 0.22.0). All I remember is that my local git installation was setup to keep lind-endings as is (not to change them to Unix), and it cause OSX/Linux to throw errors on that version of ESLint. Here's original issue: https://github.com/eslint/eslint/issues/2641

not-an-aardvark commented 7 years ago

Oh, I know the issue -- based on https://github.com/eslint/eslint/issues/2641#issuecomment-107102866, it looks like it was the shebang in bin/eslint.js that caused the problem. Bash interprets an \r at the end of a shebang as part of the shebang.

Since bin/eslint.js is in the repository, I think .gitattributes should be able to convert its linebreaks correctly. Also, if we enable linebreak-style, we can make it so that the build aborts if the shebang is invalid.