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

Unable to build under windows - error at test:bin task #1233

Closed
blikblum opened this issue Jul 9, 2016 · 14 comments · Fixed by renovatebot/renovate#216
Closed

Unable to build under windows - error at test:bin task #1233

blikblum opened this issue Jul 9, 2016 · 14 comments · Fixed by renovatebot/renovate#216

Comments

@blikblum
Copy link
Contributor

blikblum commented Jul 9, 2016

Cloned handlebars.js repository under windows 10 64bit, node 4.2.1

Installed dependencies npm install

Ran grunt. Initially there was an error in jison. After changing the eol of handlebars.I to unix it worked.

Again ran grunt

Got the following error:
Running "test:bin" task
Fatal error: Command failed: C:\WINDOWS\system32\cmd.exe /s /c "./bin/handlebars -a spec/artifacts/empty.handlebars"

To run ./bin/handlebars file under windows is necessary to execute node ./bin/handlebars

@kpdecker
Copy link
Collaborator

I don't have a windows box to test on, but if you submit a PR that fixes that issue, glad to accept.

@stevenvachon
Copy link

stevenvachon commented Aug 21, 2016

Test with AppVeyor CI.

This should get it going quickly and easily: https://github.com/stevenvachon/hidefile/blob/master/appveyor.yml

@blikblum
Copy link
Contributor Author

Done in #1244

There still a issue with istanbul i could not find how to fix

@nknapp
Copy link
Collaborator

nknapp commented May 14, 2017

#1346 should fix this

@blikblum
Copy link
Contributor Author

blikblum commented May 14, 2017

Edit: sorry i think that was already merged. will try the PR

Hi updated and still getting an error:

Running "test:cov" task
D:\repositories\handlebars.js\node_modules\.bin\istanbul:2
basedir=$(dirname "$(echo "$0" | sed -e 's,\\,/,g')")
          ^^^^^^^
SyntaxError: missing ) after argument list
    at Object.exports.runInThisContext (vm.js:76:16)
    at Module._compile (module.js:542:28)
    at Object.Module._extensions..js (module.js:579:10)
    at Module.load (module.js:487:32)
    at tryModuleLoad (module.js:446:12)
    at Function.Module._load (module.js:438:3)
    at Module.runMain (module.js:604:10)
    at run (bootstrap_node.js:394:7)
    at startup (bootstrap_node.js:149:9)
    at bootstrap_node.js:509:3
Fatal error: 1 tests failed

There's a istanbul.cmd file in .bin folder but is executing the istanbul bash script instead file.

I change to call istanbul.cmd but gives another error:

D:\repositories\handlebars.js\node_modules\.bin\istanbul.cmd:1
(function (exports, require, module, __filename, __dirname) { @IF EXIST "%~dp0\n
ode.exe" (

Since fork is for a node.js module i tried spawn also without luck

Using node 6.9.1, windows 10

@nknapp
Copy link
Collaborator

nknapp commented May 14, 2017

Have you tried what you wrote and then notice that the PR was not merged yet? Or did the PR-commit not work either?

I have merged the PR now, so maybe you could try again.

@blikblum
Copy link
Contributor Author

The mentioned issues were before merging. I tried the PR but got eslint errors. Will try after the merge

@blikblum
Copy link
Contributor Author

I manually applied the changes to master branch and worked fine. I can do a PR if necessary.

The 4.x build failed with an error in jison but this is another issue

@nknapp
Copy link
Collaborator

nknapp commented May 14, 2017 via email

@blikblum
Copy link
Contributor Author

blikblum commented May 14, 2017

Is the jison error related to line endings?

I don't know. There was an error previously i don't remember exactly:

Running "parser" task
D:\repositories\handlebars.js\node_modules\jison\lib\jison\util\lex-parser.js:13
3
    throw new Error(str);
    ^

Error: Parse error on line 2:
function strip(raw
---------------------^
Expecting '%%', 'ACTION', 'NAME', 'START_INC', 'START_EXC', 'START_COND', got '{
'

nknapp added a commit that referenced this issue May 14, 2017
@nknapp
Copy link
Collaborator

nknapp commented May 14, 2017

@blikblum After changing the eol of handlebars.I to unix it worked. (That is what you found out about a year ago. A year can be a very long time, I know that.)

I have added a .gitattributes-entry to fix that on Windows. Deletion and `git reset --hard' may be necessary to apply the changed line-endings.

nknapp added a commit to nknapp/handlebars.js that referenced this issue May 14, 2017
Fixes handlebars-lang#1233

NodeJS files cannot be executed directly on Windows.
@nknapp
Copy link
Collaborator

nknapp commented May 14, 2017

About the original issue: It still exists, but I'll integrate your suggestion, maybe slightly different, because on Linux it would still be good to call the script without the "node "-prefix

@nknapp
Copy link
Collaborator

nknapp commented Aug 22, 2017

I have update branch issue-1233, wihch has "appveyor.com" configured as CI-service.
For some reason, tests in env browser and node are not executed in the Windows environment while they are very well on Linux.

I would be cool if someone could look into that.

@nknapp nknapp reopened this Aug 22, 2017
nknapp added a commit that referenced this issue Aug 23, 2017
Closes #1233

- Handle path-separators properly. Use "path.sep" instead of "/".
  Or use "require.resolve()" if possible
- Use "execFile" instead of "exec" to run the Handlebars executable.
  This prevents problems due to (missing) shell escaping.
- Use explicit call to "node" in order to run the executable on Windows.
- Add "appveyor"-CI in order to run regular tests on Windows.
@nknapp nknapp closed this as completed in 5b76f04 Aug 23, 2017
@nknapp
Copy link
Collaborator

nknapp commented Oct 17, 2017

Released in 4.0.11

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.

4 participants