chrisa / node-dtrace-provider

Native DTrace probes for node.js apps
Other
319 stars 70 forks source link

Reapplied changes for FreeBSD #85

Closed No9 closed 7 years ago

No9 commented 7 years ago

This patch reintroduces FreeBSD support for this library.

It updates the the build script to use %s instead of %q the latter is not supported by default on FreeBSD.

It requires that the base source for dtrace is installed on the build machine. This can be found in the standard base.txz for the release that being built on.

It also removes the malloc.h include as that has been integrated into stdlib.h

outputs on FreeBSD 10.3 and 11

# npm test

> dtrace-provider@0.7.0 test /usr/home/anton/code/builds/node-dtrace-provider
> tap test/*test.js

ok test/32probe-char.test.js .......................... 35/35
ok test/32probe.test.js ............................. 531/531
ok test/add-probes.test.js ............................ 49/49
ok test/basic.test.js ................................... 4/4
ok test/create-destroy.test.js ........................ 13/13
ok test/disambiguation.test.js .......................... 7/7
ok test/dtrace-test.js .................................. 1/1
ok test/enabled-disabled.test.js ...................... 13/13
ok test/enabledagain.test.js ............................ 6/6
ok test/fewer-args-json.test.js ......................... 4/4
ok test/fewer-args.test.js .............................. 5/5
ok test/gc.test.js ...................................... 3/3
ok test/json-args.test.js ............................... 5/5
ok test/more-args.test.js ............................... 4/4
ok test/multiple-json-args.test.js ...................... 3/3
ok test/notenabled.test.js .............................. 2/2
total ............................................... 685/685

ok
davepacheco commented 7 years ago

Thanks! Do I understand correctly that this patch re-adds FreeBSD support for this module? Which versions are supported? What happens on other versions? Does it depend on libusdt#15 also?

No9 commented 7 years ago

Hey @davepacheco yes this patch re-adds FreeBSD support. I have tested it on FreeBSD 10.3 and FreeBSD 11 with https://github.com/chrisa/libusdt/pull/15 and it passes.

davepacheco commented 7 years ago

Thanks. If this depends on the newer libusdt, shouldn't it include a submodule update as well?

No9 commented 7 years ago

Hey @davepacheco Happy to reinclude the reference if that makes sense I took it out as it seemed to be conflicting with changes from @melloc and keeping the two in sync as other patches landed was a tad fustrating Let me know either way

melloc commented 7 years ago

@No9 Can you update the submodule to chrisa/libusdt@471ae4a? Also, can you include a comment explaining that FreeBSD's printf doesn't support %q?

No9 commented 7 years ago

Hi @melloc Added libusdt reference and updated the commit message as requested.

melloc commented 7 years ago

Merged as 942e227.