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.

Sign up for GitHub

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

Jump to bottom

src: use NODE_BUILTIN_MODULE_CONTEXT_AWARE() macro #17071

Merged
merged 1 commit into from
Nov 17, 2017
Merged

src: use NODE_BUILTIN_MODULE_CONTEXT_AWARE() macro #17071

merged 1 commit into from
Nov 17, 2017

Conversation

bnoordhuis
Copy link
Member

@bnoordhuis bnoordhuis commented Nov 16, 2017 •
edited
Loading

Commit d217b28 ("async_hooks: add trace events to async_hooks") used
NODE_MODULE_CONTEXT_AWARE_BUILTIN() instead.

After commit 8680bb9 ("src: explicitly register built-in modules") it
no longer works for static and shared library builds so remove it.

CI: https://ci.nodejs.org/job/node-test-pull-request/11478/

@nodejs-github-bot nodejs-github-bot added the c++ Issues and PRs that require attention from people who are familiar with C++. label Nov 16, 2017
@bnoordhuis bnoordhuis mentioned this pull request Nov 16, 2017
Closed
3 tasks
@bnoordhuis
Copy link
Member Author

Anyone have a problem with me landing this < 48 hours? It'd be nice to have the static library build working again.

@cjihrig
Copy link
Contributor

cjihrig commented Nov 16, 2017

This has a number of sign offs. I'd be OK with fast tracking to fix the build.

@bnoordhuis
src: use NODE_BUILTIN_MODULE_CONTEXT_AWARE() macro
d6ac8a4 
Commit d217b28 ("async_hooks: add trace events to async_hooks")
used `NODE_MODULE_CONTEXT_AWARE_BUILTIN()` instead.

After commit 8680bb9 ("src: explicitly register built-in modules")
it no longer works for static library builds so remove it.

PR-URL: nodejs#17071
Reviewed-By: Anna Henningsen <anna@addaleax.net>
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Timothy Gu <timothygu99@gmail.com>
@bnoordhuis bnoordhuis closed this Nov 17, 2017
@bnoordhuis bnoordhuis deleted the fix-library-build branch November 17, 2017 01:40
@bnoordhuis bnoordhuis merged commit d6ac8a4 into nodejs:master Nov 17, 2017
@MylesBorins
Copy link
Contributor

MylesBorins commented Dec 11, 2017 •
edited
Loading 


edit: this now lands cleanly, removing label

MylesBorins pushed a commit that referenced this pull request Dec 12, 2017
@bnoordhuis @MylesBorins
src: use NODE_BUILTIN_MODULE_CONTEXT_AWARE() macro
4bc30ba 
Commit d217b28 ("async_hooks: add trace events to async_hooks")
used `NODE_MODULE_CONTEXT_AWARE_BUILTIN()` instead.

After commit 8680bb9 ("src: explicitly register built-in modules")
it no longer works for static library builds so remove it.

PR-URL: #17071
Reviewed-By: Anna Henningsen <anna@addaleax.net>
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Timothy Gu <timothygu99@gmail.com>
MylesBorins pushed a commit that referenced this pull request Dec 12, 2017
@bnoordhuis @MylesBorins
src: use NODE_BUILTIN_MODULE_CONTEXT_AWARE() macro
0db1f87 
Commit d217b28 ("async_hooks: add trace events to async_hooks")
used `NODE_MODULE_CONTEXT_AWARE_BUILTIN()` instead.

After commit 8680bb9 ("src: explicitly register built-in modules")
it no longer works for static library builds so remove it.

PR-URL: #17071
Reviewed-By: Anna Henningsen <anna@addaleax.net>
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Timothy Gu <timothygu99@gmail.com>
@MylesBorins MylesBorins mentioned this pull request Dec 12, 2017
Merged
@gibfahn
Copy link
Member

gibfahn commented Dec 19, 2017

Depends on a semver-minor (#15538 and #16565)

This was referenced Jan 16, 2018
Closed
Closed
gibfahn pushed a commit to gibfahn/node that referenced this pull request Jan 17, 2018
@bnoordhuis @gibfahn
src: use NODE_BUILTIN_MODULE_CONTEXT_AWARE() macro
c411842 
Commit d217b28 ("async_hooks: add trace events to async_hooks")
used `NODE_MODULE_CONTEXT_AWARE_BUILTIN()` instead.

After commit 8680bb9 ("src: explicitly register built-in modules")
it no longer works for static library builds so remove it.

PR-URL: nodejs#17071
Backport-PR-URL: nodejs#18179
Reviewed-By: Anna Henningsen <anna@addaleax.net>
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Timothy Gu <timothygu99@gmail.com>
MylesBorins pushed a commit that referenced this pull request Jan 19, 2018
@bnoordhuis @MylesBorins
src: use NODE_BUILTIN_MODULE_CONTEXT_AWARE() macro
aa63e02 
Commit d217b28 ("async_hooks: add trace events to async_hooks")
used `NODE_MODULE_CONTEXT_AWARE_BUILTIN()` instead.

After commit 8680bb9 ("src: explicitly register built-in modules")
it no longer works for static library builds so remove it.

Backport-PR-URL: #18179
PR-URL: #17071
Reviewed-By: Anna Henningsen <anna@addaleax.net>
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Timothy Gu <timothygu99@gmail.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@cjihrig cjihrig cjihrig approved these changes

@TimothyGu TimothyGu TimothyGu approved these changes

@addaleax addaleax addaleax approved these changes

@jasnell jasnell jasnell approved these changes

Assignees
No one assigned
Labels
c++ Issues and PRs that require attention from people who are familiar with C++.
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
9 participants
@bnoordhuis @cjihrig @MylesBorins @gibfahn @jasnell @addaleax @TimothyGu @BethGriggs @nodejs-github-bot