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

feat(recipes): add Linux usdt build #19

Merged
merged 1 commit into from
Aug 10, 2020
Merged

Conversation

mmarchini
Copy link
Contributor

No description provided.

python \
ccache \
xz-utils \
devtoolset-7 \
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I tried devtoolset-6, but there was an unmet dependency on cloudlinux (devtoolset-6-dwz, which is not available on https://repo.cloudlinux.com/cloudlinux/7/sclo/devtoolset-6/x86_64/)

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

we're moving to devtoolset-8 for Node 14+ so maybe you should just jump to that

Copy link
Contributor Author

@mmarchini mmarchini Apr 8, 2020

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Even for older Node.js versions?

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It should be safe, devtoolsets are nice that way, they'll pin you to the libc of the OS version. We treat them as fixed because there may be variations that break for folks who are expecting stability in a major version.

Since you're starting off with something fresh, you get to set the rules! You could say "oh, you can't use this on that OS, too old" if you wanted. So now's your chance to set the bar high so it doesn't feel like it's way too low in 3 years time.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Makes sense, will update the PR tomorrow.

@rvagg
Copy link
Member

rvagg commented Apr 8, 2020

so what's the summary tagline for this one? "adds dtrace probes"? I assume it's useful for getting tracing data down into v8 that you can't get with a standard binary?

@mmarchini
Copy link
Contributor Author

Add USDT probes* to allow tracing of V8 GC calls and some HTTP events with compatible tracers (systemtap and BPF tracing tools).

* I think we're past the point where calling them dtrace probes makes sense on Linux

RUN groupadd --gid 1000 node \
&& adduser --gid 1000 --uid 1000 node

COPY cloudlinux.repo /etc/yum.repos.d/cloudlinux.repo
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@sam-github where are you getting devtoolset-8 from for the new stuff in Build? I guess we should try and be consistent here if @mmarchini opts to bump to it here.

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

devtoolset-8 is package, its yum installable:

nodejs/build#2262

@mmarchini
Copy link
Contributor Author

I thought we merged this PR already 🙃

I'll try to test it again this weekend to see if we can move forward with it.

@mmarchini mmarchini merged commit 0c1a234 into nodejs:master Aug 10, 2020
@mmarchini mmarchini deleted the usdt branch August 10, 2020 04:26
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 this pull request may close these issues.

3 participants