7.7.1 Proposal #11638

italoacasas · 2017-03-01T20:50:08Z

2017-03-01, Version 7.7.1 (Current), @italoacasas

Notable changes

Node.js 7.7.0 contains a bug that will prevent all native modules from building, this patch should fix the issue. Apologies to everyone who was affected by 7.7.0.

CI

Commits

[c8e34b61f6] - build: add missing src/tracing header files (Daniel Bevenius) #10851
[96f55f9e59] - src: move trace_event.h include to internal header (Ben Noordhuis) #10959
[30c80cbe6f] - src: fix TracingController cleanup (Jason Ginchereau) #10623
[b89b2a7d36] - src: always initialize tracing controller in agent (Matt Loring) #10507
[54e55e05ca] - test: make test-intl-no-icu-data more robust (Michaël Zasso) #10992
[7b253eb3ed] - test: increase strictness for test-trace-event (Rich Trott) #11065
[3dc4a5f1f4] - tracing: fix -Wunused-private-field warning (Santiago Gimeno) #10416
[8a918bf411] - tracing: fix -Wreorder warning (Santiago Gimeno) #10416

Initialize `InternalTraceBuffer::id_` the last. PR-URL: #10416 Reviewed-By: James M Snell <jasnell@gmail.com> Reviewed-By: Anna Henningsen <anna@addaleax.net> Reviewed-By: Colin Ihrig <cjihrig@gmail.com>

Remove `external_buffer_` from `InternalTraceBuffer` as it seems not to be used anywhere. PR-URL: #10416 Reviewed-By: James M Snell <jasnell@gmail.com> Reviewed-By: Anna Henningsen <anna@addaleax.net> Reviewed-By: Colin Ihrig <cjihrig@gmail.com>

PR-URL: #10507 Reviewed-By: Colin Ihrig <cjihrig@gmail.com> Reviewed-By: James M Snell <jasnell@gmail.com>

This fixes an incorrect deletion of the `TracingController` instance, which in some environments could cause an error about an invalid pointer passed to `free()`. The `TracingController` instance is actually owned by a `unique_ptr` member of the platform, so calling `platform::SetTracingController(nullptr)` is the correct way to delete it. But before that, the `TraceBuffer` must be deleted in order for the tracing loop to exit; that is accomplished by calling `TracingController::Initialize(nullptr)`. PR-URL: #10623 Reviewed-By: Matthew Loring <mattloring@google.com> Reviewed-By: Michaël Zasso <targos@protonmail.com>

I noticed that only one header from src/tracing is included in the sources list in node.gyp. Not sure if this is intentional or not so I wanted to bring it up just in case this was overlooked. PR-URL: #10851 Reviewed-By: Ben Noordhuis <info@bnoordhuis.nl> Reviewed-By: Colin Ihrig <cjihrig@gmail.com> Reviewed-By: James M Snell <jasnell@gmail.com> Reviewed-By: Italo A. Casas <me@italoacasas.com>

Move the include from src/node.h to src/node_internals.h. trace_event.h is not shipped in binary-only and headers-only tarballs, making it currently impossible to build add-ons against them. PR-URL: #10959 Refs: nodejs/citgm#226 (comment) Reviewed-By: Colin Ihrig <cjihrig@gmail.com> Reviewed-By: James M Snell <jasnell@gmail.com> Reviewed-By: Matthew Loring <mattloring@google.com> Reviewed-By: Richard Lau <riclau@uk.ibm.com> Reviewed-By: Stephen Belanger <admin@stephenbelanger.com>

Change test-trace-event such that it checks that all expected values are within the same trace object rather than scattered across multiple trace objects. PR-URL: #11065 Reviewed-By: Anna Henningsen <anna@addaleax.net> Reviewed-By: Luigi Pinca <luigipinca@gmail.com> Reviewed-By: Colin Ihrig <cjihrig@gmail.com> Reviewed-By: Evan Lucas <evanlucas@me.com> Reviewed-By: James M Snell <jasnell@gmail.com>

In V8 5.6, String#toLocaleUpperCase can work even when no ICU data is loaded. Use another method to check the --icu-data-dir option pointing to an empty directory. PR-URL: #10992 Reviewed-By: Ben Noordhuis <info@bnoordhuis.nl>

richardlau · 2017-03-01T21:06:32Z

doc/changelogs/CHANGELOG_V7.md

+
+### Notables changes
+
+Node.js 7.7.0 contains a bug that will prevent all native modules from loading, this patch should fix the issue. Apologies to everyone who was affected by 7.7.0.


Replace "loading" with either "compiling" or "building". Any prebuilt modules would have been fine.

rmg · 2017-03-01T21:08:22Z

Since v7.7.0 was passing on citgm because it was only testing the build output and not the release packaged binaries, is there a way to run this proposed release against citgm using rc-style binary packages?

italoacasas · 2017-03-01T21:10:34Z

@rmg I'm going to test the binaries manually with native modules.

cc: @MylesBorins

is there a way to run this proposed release against citgm using rc-style binary packages?

italoacasas · 2017-03-01T22:20:05Z

@targos I see errors in CITGM related with native modules. Any idea?

richardlau · 2017-03-01T22:26:19Z

What do the errors look like?

italoacasas · 2017-03-01T22:27:51Z

@richardlau

warn: module name:        | request             
warn: version:            | 2.79.0              
warn: error:              | Install Failed      
warn: error:              | undefined                                                                                                                                                                                                                                                                                   
warn:                     | > execSync@1.0.2 install /tmp/fdccdfb4-d49d-48b4-bff3-bfbd848bcd7b/request/node_modules/execSync                                                                                                                                                                                            
warn:                     | > node install.js                                                                                                                                                                                                                                                                           
warn:                     |                                                                                                                                                                                                                                                                                             
warn:                     | [execsync v1.0.2] Attempting to compile native extensions.                                                                                                                                                                                                                                  
warn:                     | [execSync v1.0.2]                                                                                                                                                                                                                                                                           
warn:                     | Native code compile failed!!

italoacasas · 2017-03-01T22:33:56Z

~~But, I'm testing manually, building native modules and is working.. 😒~~

Notable changes: Node.js 7.7.0 contains a bug that will prevent all native modules from building, this patch should fix the issue. Apologies to everyone who was affected by 7.7.0. PR-URL: #11638

MylesBorins · 2017-03-01T23:01:40Z

The issue with CITGM is that we grab the headers from the repo when we build native modules. The problem that we ran into was with the header files that were created with the build assets. Technically this wouldn't have been easy to test even locally before the builds were promoted. More than likely what we need to make sure that this doesn't happen in the future is a series of tests that are run during or before promotion.

…

On Wed, Mar 1, 2017, 5:34 PM Italo A. Casas ***@***.***> wrote: But, I'm testing manually, building native modules and is working.. 😒 — You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub <#11638 (comment)>, or mute the thread <https://github.com/notifications/unsubscribe-auth/AAecV4vsjp2ehLyDtf3cum39kb_l7DEFks5rhfJagaJpZM4MQLzF> .

richardlau · 2017-03-01T23:26:39Z

In theory one could run the install.py script and install just the headers into an empty directory (this is what make tar-headers does so would be a reasonable approximation) and use that directory instead of the full source tree to compile against. But yeah the best solution is likely tests between building the release and prior to promotion.

richardlau · 2017-03-01T23:41:27Z

I don't know if it's CitGM or execSync but that's not very helpful error output.

MylesBorins · 2017-03-02T00:14:14Z

I worked with an RC and can confirm that node-gyp is working fine with v7.7.1

https://gist.github.com/MylesBorins/d983701a27608239ecc65c119ab66dc4

MylesBorins

rubber stamp LGTM

I can confirm that this release will work fine with node-gyp

PR-URL: nodejs#11638

Notable changes: Node.js 7.7.0 contains a bug that will prevent all native modules from building, this patch should fix the issue. Apologies to everyone who was affected by 7.7.0. PR-URL: nodejs#11638

PR-URL: nodejs/node#11638

Notable changes: Node.js 7.7.0 contains a bug that will prevent all native modules from building, this patch should fix the issue. Apologies to everyone who was affected by 7.7.0. PR-URL: nodejs/node#11638 Signed-off-by: Ilkka Myller <ilkka.myller@nodefield.com>

PR-URL: nodejs/node#11638

santigimeno and others added 7 commits March 1, 2017 10:52

tracing: fix -Wreorder warning

8a918bf

Initialize `InternalTraceBuffer::id_` the last. PR-URL: #10416 Reviewed-By: James M Snell <jasnell@gmail.com> Reviewed-By: Anna Henningsen <anna@addaleax.net> Reviewed-By: Colin Ihrig <cjihrig@gmail.com>

src: always initialize tracing controller in agent

b89b2a7

PR-URL: #10507 Reviewed-By: Colin Ihrig <cjihrig@gmail.com> Reviewed-By: James M Snell <jasnell@gmail.com>

nodejs-github-bot added build Issues and PRs related to build files or the CI. c++ Issues and PRs that require attention from people who are familiar with C++. trace_events Issues and PRs related to V8, Node.js core, and userspace code trace events. v7.x labels Mar 1, 2017

test: make test-intl-no-icu-data more robust

54e55e0

In V8 5.6, String#toLocaleUpperCase can work even when no ICU data is loaded. Use another method to check the --icu-data-dir option pointing to an empty directory. PR-URL: #10992 Reviewed-By: Ben Noordhuis <info@bnoordhuis.nl>

richardlau requested changes Mar 1, 2017

View reviewed changes

italoacasas force-pushed the v7.7.1-proposal branch from 336010b to 248c869 Compare March 1, 2017 21:13

richardlau approved these changes Mar 1, 2017

View reviewed changes

2017-03-01, Version 7.7.1 (Current)

5c2e415

Notable changes: Node.js 7.7.0 contains a bug that will prevent all native modules from building, this patch should fix the issue. Apologies to everyone who was affected by 7.7.0. PR-URL: #11638

italoacasas force-pushed the v7.7.1-proposal branch from 248c869 to 5c2e415 Compare March 1, 2017 22:58

MylesBorins approved these changes Mar 2, 2017

View reviewed changes

italoacasas added a commit to italoacasas/node that referenced this pull request Mar 2, 2017

Working on v7.7.2

b20bc13

PR-URL: nodejs#11638

italoacasas merged commit 5c2e415 into v7.x Mar 2, 2017

italoacasas added a commit to italoacasas/nodejs.org that referenced this pull request Mar 2, 2017

blog: release post for v7.7.1

446b273

PR-URL: nodejs/node#11638

italoacasas mentioned this pull request Mar 2, 2017

blog: release post for v7.7.1 nodejs/nodejs.org#1165

Merged

MylesBorins pushed a commit to nodejs/nodejs.org that referenced this pull request Mar 2, 2017

blog: release post for v7.7.1 (#1165)

50a1ae8

PR-URL: nodejs/node#11638

italoacasas added a commit to italoacasas/nodejs.org that referenced this pull request Mar 2, 2017

blog: update 7.7.1 release

86d6016

PR-URL: nodejs/node#11638

sam-github deleted the v7.7.1-proposal branch March 6, 2017 16:59

italoacasas added a commit to nodejs/nodejs.org that referenced this pull request Mar 14, 2017

blog: update 7.7.1 release (#1166)

41832f4

PR-URL: nodejs/node#11638

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

7.7.1 Proposal #11638

7.7.1 Proposal #11638

italoacasas commented Mar 1, 2017 •

edited

Loading

richardlau Mar 1, 2017

rmg commented Mar 1, 2017

italoacasas commented Mar 1, 2017

italoacasas commented Mar 1, 2017

richardlau commented Mar 1, 2017

italoacasas commented Mar 1, 2017 •

edited

Loading

italoacasas commented Mar 1, 2017 •

edited

Loading

MylesBorins commented Mar 1, 2017 via email

richardlau commented Mar 1, 2017

richardlau commented Mar 1, 2017

MylesBorins commented Mar 2, 2017

MylesBorins left a comment


		### Notables changes

		Node.js 7.7.0 contains a bug that will prevent all native modules from loading, this patch should fix the issue. Apologies to everyone who was affected by 7.7.0.

7.7.1 Proposal #11638

7.7.1 Proposal #11638

Conversation

italoacasas commented Mar 1, 2017 • edited Loading

2017-03-01, Version 7.7.1 (Current), @italoacasas

Notable changes

CI

Commits

richardlau Mar 1, 2017

Choose a reason for hiding this comment

rmg commented Mar 1, 2017

italoacasas commented Mar 1, 2017

italoacasas commented Mar 1, 2017

richardlau commented Mar 1, 2017

italoacasas commented Mar 1, 2017 • edited Loading

italoacasas commented Mar 1, 2017 • edited Loading

MylesBorins commented Mar 1, 2017 via email

richardlau commented Mar 1, 2017

richardlau commented Mar 1, 2017

MylesBorins commented Mar 2, 2017

MylesBorins left a comment

Choose a reason for hiding this comment

italoacasas commented Mar 1, 2017 •

edited

Loading

italoacasas commented Mar 1, 2017 •

edited

Loading

italoacasas commented Mar 1, 2017 •

edited

Loading