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

instrument.js hangs sometimes in node v5 #71

Closed
esbena opened this issue Dec 2, 2015 · 3 comments
Closed

instrument.js hangs sometimes in node v5 #71

esbena opened this issue Dec 2, 2015 · 3 comments

Comments

@esbena
Copy link
Contributor

esbena commented Dec 2, 2015

Possibly related: #58

instrument.js hangs in node v5

To reproduce:

$ node --version
v5.0.0
$ mkdir test
$ cd test
$ echo '{}' > package.json
$ npm i jalangi2
$ npm i minimist
$ node node_modules/jalangi2/src/js/commands/instrument.js --outputDir out node_modules/minimist
^C # it hangs..
$ rm -rf node_modules/minimist/test
$ node node_modules/jalangi2/src/js/commands/instrument.js --outputDir out node_modules/minimist
instrumenting out/minimist2/index_orig_.js
instrumenting out/minimist2/example/parse_orig_.js
done!

It seems that the call to ncp on line 454 of instrument.js never calls the callback.
That can be shown with the following change:

ncp(inputDir, copyDir, {transform: transform}, function(){console.log('ncp done')});
@msridhar
Copy link
Contributor

msridhar commented Dec 2, 2015

Does it work with v4? In the short-term probably downgrading node is your best bet. I think the right fix is to get rid of our dependence on ncp entirely. One simple solution is to do the following instead:

  1. First just do a simple cp -r to the output directory (there must be a robust library for this).
  2. Go through the output directory and replace files with their instrumented versions.

I think that will be a lot more robust and conceptually simple than what we do now.

@esbena
Copy link
Contributor Author

esbena commented Dec 2, 2015

https://www.npmjs.com/package/graceful-ncp is a drop-in replacement for ncp, just use require('graceful-ncp') instead.
The issue is fixed if instrument.js is modified to use that instead of ncp.

I will make a pull-request when I have tested this fix for some time.

esbena added a commit to esbena/jalangi2 that referenced this issue Dec 2, 2015
esbena added a commit to esbena/jalangi2 that referenced this issue Dec 9, 2015
@msridhar
Copy link
Contributor

Fixed by 619d5d2

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

2 participants