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

failure "empty command evaluates to undefined" with nodejs 4.8.2 #4502

Closed
jonassmedegaard opened this issue Apr 15, 2017 · 11 comments · Fixed by #4510
Closed

failure "empty command evaluates to undefined" with nodejs 4.8.2 #4502

jonassmedegaard opened this issue Apr 15, 2017 · 11 comments · Fixed by #4510

Comments

@jonassmedegaard
Copy link

jonassmedegaard commented Apr 15, 2017

Hi,

Coffeescript 1.12.5 fails when built with Nodejs 4.8.2 (the version in Debian unstable/testing).
with the following error message and stacktrace:

bin/cake build:full
failed 1 and passed 834 tests in 23.65 seconds 

  empty command evaluates to undefined 
  AssertionError: Expected undefined to equal coffee> 
    at exports.eq (/build/coffeescript-1.12.5~dfsg/test/support/helpers.coffee:16:34)
    at /build/coffeescript-1.12.5~dfsg/test/repl.coffee:1:1
    at Function. (/build/coffeescript-1.12.5~dfsg/test/repl.coffee:1:1)
    at global.test (/build/coffeescript-1.12.5~dfsg/Cakefile:1:1)
    at testRepl (/build/coffeescript-1.12.5~dfsg/test/repl.coffee:1:1)
    at Object. (/build/coffeescript-1.12.5~dfsg/test/repl.coffee:1:1)
    at Object. (/build/coffeescript-1.12.5~dfsg/test/repl.coffee:1:1)
    at Module._compile (module.js:1:1)
    at Object.exports.run (/build/coffeescript-1.12.5~dfsg/lib/coffee-script/coffee-script.js:1:1)
    at runTests (/build/coffeescript-1.12.5~dfsg/Cakefile:1:1)
    at testBuiltCode (/build/coffeescript-1.12.5~dfsg/Cakefile:1:1)
    at /build/coffeescript-1.12.5~dfsg/Cakefile:1:1
    at ChildProcess. (/build/coffeescript-1.12.5~dfsg/Cakefile:1:1)
    at emitTwo (events.js:1:1)
    at ChildProcess.emit (events.js:1:1)
    at Process.ChildProcess._handle.onexit (internal/child_process.js:1:1)
 
  function () {
      return fn(input, output, repl);
    }

If instead using Nodejs 6.10.2 (in Debian experimental) the testsuite succeeds.

@GeoffreyBooth
Copy link
Collaborator

The module includes built files, in the lib folder. There’s no need to build CoffeeScript yourself to run the compiler. Just run bin/coffee.

For development purposes it is assumed that you’re running the latest version of Node. We’re not supporting development cake commands in older runtimes.

@jonassmedegaard
Copy link
Author

jonassmedegaard commented Apr 15, 2017 via email

@GeoffreyBooth
Copy link
Collaborator

The compiler runs in Node > 0.8.0. As in, using the built code (e.g. bin/coffee) should work in old versions of Node. If that isn’t the case, please open an issue.

Running the cake commands (building a new compiler, running the tests etc.) assume current Node.

@jonassmedegaard
Copy link
Author

jonassmedegaard commented Apr 15, 2017 via email

@GeoffreyBooth
Copy link
Collaborator

Let me be more specific. The newest versions of CoffeeScript support things like generator functions that are not supported in old versions of Node. To test that support, the tests run generators via the runtime, and therefore the tests require a runtime that supports generators. So yes, running the tests requires a current version of Node.

Simply building or running the compiler, however, does not. If you type simply cake you’ll see that cake build:full builds the compiler and then runs the tests. But just running cake build, to build the compiler and stop, does work in Node 4.6.2.

Furthermore the JavaScript output that is generated by cake build is checked into the repo. It’s in the lib folder. This is what we expect downstream tools to use, and this is what bin/coffee and both top-level .js files point to. There’s additionally a merged version of the JavaScript sources in docs/v1/browser-compiler/coffee-script.js. I would consider all of them to be the CoffeeScript “source,” as they’re all in the repo.

The browser compiler, in particular, doesn’t even depend on Node; it runs in browsers, and the v1 browser compiler should run in very old versions of Node. Generally downstream tools that import CoffeeScript, like the Ruby coffee-script gem, use the browser compiler for maximum compatibility.

@jonassmedegaard
Copy link
Author

jonassmedegaard commented Apr 16, 2017 via email

@GeoffreyBooth
Copy link
Collaborator

recent build process and corresponding tests require features only available in recent Nodejs.

No. The build process does not require recent Node. The build process works in Node 4.6.2, if not earlier. That’s what happens when you run the command cake build. (Type just cake to see explanations of all the commands.)

The tests fail in Node 4.6.2, presumably because they’re expecting the runtime to support features that Node 4.6.2 doesn’t have. We could, I suppose, add some checks to filter out the tests that don’t make sense to run under Node 4. Or you could, since you’re patching CoffeeScript anyway 😉

But I guess it depends what you mean by “build.” Does build simply mean compiling the CoffeeScript into JavaScript, or does it mean doing that and running the tests (and having them all pass)? If the former, then you’re done; the code already builds in Node 4. If the latter, I suggest you create another patch that comments out or deletes any offending tests.

I also wonder what “require all code to be built from source” means for a language like JavaScript. It’s not like cake build produces a platform-dependent binary, like compiling C code. The JavaScript produced by cake build is both a generated output, yes, but they’re also very much source files: the code in lib is human-readable. Lots of projects have only .js files as their source. Considering that the CoffeeScript project’s JavaScript files are source files, platform-independent, human-readable and checked into the repo, why wouldn’t you consider these files the “source” for the purposes of your policy?

The engines: node key in package.json refers to the Node runtime required to run the JavaScript files by another Node project that is including this module, or when running the coffee binary at the command line. It just isn’t meant to refer to the minimum Node runtime required for building the code or running the tests. If you do npm install coffee-script and look inside the resulting node_modules/coffee-script folder, you’ll notice that there are only JavaScript files in there. That’s all we distribute, and that’s all that gets hosted on NPM or used by other Node modules.

@jonassmedegaard
Copy link
Author

jonassmedegaard commented Apr 16, 2017 via email

GeoffreyBooth added a commit to GeoffreyBooth/coffeescript that referenced this issue Apr 17, 2017
…n that was fixed in Node 5.11.0; just disable the test for Node < 6. Fixes jashkenas#4502.
@GeoffreyBooth GeoffreyBooth reopened this Apr 17, 2017
GeoffreyBooth added a commit that referenced this issue Apr 17, 2017
…n that was fixed in Node 5.11.0; just disable the test for Node < 6. Fixes #4502. (#4510)
@GeoffreyBooth
Copy link
Collaborator

@jonassmedegaard It turns out there was a regression in Node sometime before 5.11.0, when it was fixed. Only one of our tests was affected, and it wasn’t a terribly important one (basically, what should be echoed when someone presses enter in the REPL without entering any code first). In #4510 I just disabled that test for Node < 6. The master branch should build and test successfully in Node 4 now.

@jonassmedegaard
Copy link
Author

jonassmedegaard commented Apr 17, 2017 via email

@GeoffreyBooth
Copy link
Collaborator

Duplicate of #4161.

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

Successfully merging a pull request may close this issue.

2 participants