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

Reapplied changes for FreeBSD #85

Closed
wants to merge 2 commits into
base: master
from

Conversation

Projects
None yet
3 participants
@No9
Contributor

No9 commented Oct 29, 2016

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

@No9 No9 referenced this pull request Oct 29, 2016

Closed

Install fails on FreeBSD #30

@davepacheco

This comment has been minimized.

Show comment
Hide comment
@davepacheco

davepacheco Nov 7, 2016

Collaborator

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?

Collaborator

davepacheco commented Nov 7, 2016

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

This comment has been minimized.

Show comment
Hide comment
@No9

No9 Nov 10, 2016

Contributor

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

Contributor

No9 commented Nov 10, 2016

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

@davepacheco

This comment has been minimized.

Show comment
Hide comment
@davepacheco

davepacheco Nov 11, 2016

Collaborator

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

Collaborator

davepacheco commented Nov 11, 2016

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

@No9

This comment has been minimized.

Show comment
Hide comment
@No9

No9 Nov 13, 2016

Contributor

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

Contributor

No9 commented Nov 13, 2016

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

This comment has been minimized.

Show comment
Hide comment
@melloc

melloc Nov 13, 2016

Collaborator

@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?

Collaborator

melloc commented Nov 13, 2016

@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 No9 changed the title from Reapplied changes for FreeBSD without reference to libusdt to Reapplied changes for FreeBSD Nov 14, 2016

@No9

This comment has been minimized.

Show comment
Hide comment
@No9

No9 Nov 14, 2016

Contributor

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

Contributor

No9 commented Nov 14, 2016

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

@melloc

This comment has been minimized.

Show comment
Hide comment
@melloc

melloc May 18, 2017

Collaborator

Merged as 942e227.

Collaborator

melloc commented May 18, 2017

Merged as 942e227.

@melloc melloc closed this May 18, 2017

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