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

Release proposal: v6.2.1 #7108

Closed
wants to merge 485 commits into from
Closed

Release proposal: v6.2.1 #7108

wants to merge 485 commits into from

Conversation

rvagg
Copy link
Member

@rvagg rvagg commented Jun 2, 2016

Replaces #6934, Jeremiah's not doing too well atm and I haven't done a >= v4 release for a while!

View changelog at https://github.com/nodejs/node/blob/v6.2.1-proposal/doc/changelogs/CHANGELOG_V6.md

CI is happy @ https://ci.nodejs.org/job/node-test-commit/3624/


Notable changes

  • buffer: Ignore negative lengths in calls to Buffer() and Buffer.allocUnsafe(). This fixes a possible security concern (reported by Feross Aboukhadijeh) where user input is passed unchecked to the Buffer constructor or allocUnsafe() as it can expose parts of the memory slab used by other Buffers in the application. Note that negative lengths are not supported by the Buffer API and user input to the constructor should always be sanitised and type-checked. (Anna Henningsen) #7051
  • npm: Upgrade npm to 3.9.3 (Kat Marchán) #7030
    • npm/npm@42d71be npm/npm#12685 When using npm ls <pkg> without a semver specifier, npm ls would skip any packages in your tree that matched by name, but had a prerelease version in their package.json. (@zkat)
    • npm/npm@f04e05 npm/npm#10013 read-package-tree@5.1.4: Fixes an issue where npm install would fail if your node_modules was symlinked. (@iarna)
    • b894413 #12372 Changing a nested dependency in an npm-shrinkwrap.json and then running npm install would not get up the updated package. This corrects that. (@misterbyrne)
    • This release includes npm@3.9.0, which is the result of our Windows testing push -- the test suite (should) pass on Windows now. We're working on getting AppVeyor to a place where we can just rely on it like Travis.
  • tty: Explicitly opt-in to blocking mode for stdio on OS X. A bug fix in libuv 1.9.0, introduced in Node.js v6.0.0, exposed problems with Node's use of non-blocking stdio, particularly on OS X which has a small output buffer. This change should fix CLI applications that have been having problems with output since Node.js v6.0.0 on OS X. The core team is continuing to address stdio concerns that exist across supported platforms and progress can be tracked at Tracking issue: stdio problems #6980. (Jeremiah Senkpiel) #6895
  • V8: Upgrade to V8 5.0.71.52. This includes a fix that addresses problems experienced by users of node-inspector since Node.js v6.0.0, see Internal Error: Illegal Access  node-inspector/node-inspector#864 for details. (Michaël Zasso) #6928

addaleax and others added 30 commits May 17, 2016 08:08
Until now, the docs stated that `process.noDeprecation` could be set
at runtime, but before any modules were loaded. That was not true,
because `lib/internal/util.js` was loaded during the process startup
process, so setting the flag at runtime was pointless.

Minimal test case:

    process.noDeprecation = true;
    process.EventEmitter;

This patch moves checking `process.noDeprecation` to the place where
it was actually used.

PR-URL: #6683
Reviewed-By: Ben Noordhuis <info@bnoordhuis.nl>
Reviewed-By: Jeremiah Senkpiel <fishrock123@rocketmail.com>
Reviewed-By: Sakthipriyan Vairamani <thechargingvolcano@gmail.com>
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
Prevent util.inspect of throwing on date object with invalid date value.
It changed to output result of toString method call.

PR-URL: #6504
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Evan Lucas <evanlucas@me.com>
Reviewed-By: Anna Henningsen <anna@addaleax.net>
The custom linting rule for argument alignment in multi-line function
calls previously ignored template strings in an effort to avoid false
positives. This isn't really necessary. Enforce for template strings and
adjust whitespace in three tests to abide. (Insert "The test abides"
joke of your choosing here.)

PR-URL: #6720
Reviewed-By: Roman Reiss <me@silverwind.io>
Reviewed-By: Ben Noordhuis <info@bnoordhuis.nl>
ESLint 2.9.0 fixes some bugs that resulted in minor issues not being
caught by ESLint 2.7.0. Update instances of our code that will be
flagged when we upgrade to ESLint 2.9.0.

PR-URL: #6498
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
Reviewed-By: Roman Reiss <me@silverwind.io>
ESLint 2.9.0 fixes some minor bugs that we have been experiencing and
introduces some new rules that we may wish to consider.

PR-URL: #6498
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
Reviewed-By: Roman Reiss <me@silverwind.io>
* bump to ICU 57.1 - update URL / hash

Fixes: #6058
PR-URL: #6088
Reviewed-By: James M Snell <jasnell@gmail.com>
…-icu

* Change configure default to "small-icu" (Intl on, English only)
* add "--without-intl" and "vcbuild without-intl" options, equivalent
to --with-intl=none
* update BUILDING.md with above changes
* Checks in tools that generate the deps/icu-small source directory
from ICU source
* Tools and process for updating ICU documented in tools/icu/README.md

Fixes: #3476
PR-URL: #6088
Reviewed-By: James M Snell <jasnell@gmail.com>
* this commit has "small" ICU 57.1.
See other related commit for tools to generate this commit.

Fixes: #3476
PR-URL: #6088
Reviewed-By: James M Snell <jasnell@gmail.com>
Add a hack js-yaml module to the doctool dependencies that simply
loads the one that’s included with eslint.

This helps avoiding to check in the whole dependency tree into
the core repo.

PR-URL: #6495
Reviewed-By: Robert Jefe Lindstaedt <robert.lindstaedt@gmail.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Jeremiah Senkpiel <fishrock123@rocketmail.com>
This commit introduces the Documentation YAML metadata concept to the
documentation.

- Parses added or Added for HTML
- Parses all data for JSON

Ref: #3867
Ref: #3713
Ref: #6470
PR-URL: #6495
Reviewed-By: Robert Jefe Lindstaedt <robert.lindstaedt@gmail.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Jeremiah Senkpiel <fishrock123@rocketmail.com>
Introduced in commit 3f69ea5 ("tools: update marked dependency"), it
stopped the embedded addons in the documentation from getting built.

PR-URL: #6652
Reviewed-By: Ben Noordhuis <info@bnoordhuis.nl>
Add checks that make sure the doctool parses metadata correctly.

PR-URL: #6495
Reviewed-By: Robert Jefe Lindstaedt <robert.lindstaedt@gmail.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Jeremiah Senkpiel <fishrock123@rocketmail.com>
The current makefile runs both `cctest` and `build-addons` in parallel
under the assumption that both rely on `all`. Unfortunately
`build-addons` does not rely on all, and there is an edge case where
by it is possible to call `build-addons` while compilation is still
happening.

This patch takes the simplest route by forcing `build-addons` and
`cctest` to run in sequence like the other test targets. This ensures
that `build-addons` will never be run during compilation.

It would be possible to modify `build-addons` to rely on `all` but it
would be a much more aggressive change to the MAKEFILE for a fairly
minor perf bump, as cctest is so fast.

PR-URL: #6723
Reviewed-By: Ben Noordhuis <info@bnoordhuis.nl>
Allow multiple `added:` version entries, since semver-minors
can trickle down to previous major versions, and thus
features may have been added in multiple versions.

Also include `deprecated:` entries and apply the same logic
to them for consistency.

Stylize the added HTML as `Added in:` and `Deprecated since:`.

PR-URL: #6495
Reviewed-By: Robert Jefe Lindstaedt <robert.lindstaedt@gmail.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Jeremiah Senkpiel <fishrock123@rocketmail.com>
Update module marked. Customize renderer to remove id from heading.

PR-URL: #6396
Reviewed-By: Roman Reiss <me@silverwind.io>
Reviewed-By: Anna Henningsen <anna@addaleax.net>
There are over 70 files in the project using template strings and all of
them have followed the convention of no spaces in curly braces.

Good: `${foo}`

Not used: `${ foo }`

Since the project has adopted a convention, and ESLint has a rule to
enforce exactly this convention, enable the rule.

PR-URL: #6591
Reviewed-By: cjihrig - Colin Ihrig <cjihrig@gmail.com>
Reviewed-By: Jeremiah Senkpiel <fishrock123@rocketmail.com>
Reviewed-By: Minwoo Jung <jmwsoft@gmail.com>
Reviewed-By: Michaël Zasso <mic.besace@gmail.com>
There has been occasional nits for spacing in object literals in PRs but
the project does not lint for it and it is not always handled
consistently in the existing code, even on adjacent lines of a file.

This change enables a linting rule requiring no space between the key
and the colon, and requiring at least one space (but allowing for more
so property values can be lined up if desired) between the colon and the
value. This appears to be the most common style used in the current code
base.

Example code the complies with lint rule:

    myObj = { foo: 'bar' };

Examples that do not comply with the lint rule:

    myObj = { foo : 'bar' };
    myObj = { foo:'bar' };

PR-URL: #6592
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Brian White <mscdex@mscdex.net>
* Make the 'extract embedded addons in the documentations' step a normal
  prerequisite.  As an order-only prerequisite, it's sometimes skipped
  when it shouldn't be.

* Make `tools/doc/addon-verify.js` a dependency of that step.  Changes
  to that file should result in a rebuild.

PR-URL: #6652
Reviewed-By: Anna Henningsen <anna@addaleax.net>
Allows building just docs using existing Node instead of building Node
first.

PR-URL: #3888
Reviewed-By: Chris Dickinson <christopher.s.dickinson@gmail.com>
Reviewed-By: Rod Vagg <rod@vagg.org>
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Myles Borins <myles.borins@gmail.com>
101dd1e introduced a regression in the doctool. This commit reverts
the changes that were made to the function signature of the various
doctool functions while maintaining support for passing in specific
node versions.

Refs: 101dd1e

PR-URL: #6680
Reviewed-By: Jeremiah Senkpiel <fishrock123@rocketmail.com>
Reviewed-By: Robert Lindstaedt <robert.lindstaedt@gmail.com>
As the minifier logic is not used at all, this patch removes the code
necessary for it.

PR-URL: #6636
Reviewed-By: Jackson Tian <shvyo1987@gmail.com>
Reviewed-By: Ben Noordhuis <info@bnoordhuis.nl>
PR-URL: #6685
Reviewed-By: Michaël Zasso <mic.besace@gmail.com>
Reviewed-By: Johan Bergström <bugs@bergstroem.nu>
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Roman Reiss <me@silverwind.io>
Reviewed-By: Ben Noorhduis <info@bnoordhuis.nl>
The only tests for `setBroadcast()` (from the `dgram` module) were in
`test/internet` which means they almost never get run. This adds a
minimal test that can check JS-land functionality in `test/parallel`.

I also expanded a comment and did some minor formatting on the existing
`test/internet` test. If there were an easy and reliable way to check
for the BROADCAST flag on an interface, it's possible that a version of
the test could be moved to `test/sequential` or `test/parallel` once it
was modified to only use internal networks.

PR-URL: #6750
Reviewed-By: Ben Noordhuis <info@bnoordhuis.nl>
Reviewed-By: James M Snell <jasnell@gmail.com>
Since I was doing the necessary git spelunking anyway, I took the time
to add the YAML information into the docs about when `setBroadcast()`
first appeared in its current form.

PR-URL: #6750
Reviewed-By: Ben Noordhuis <info@bnoordhuis.nl>
Reviewed-By: James M Snell <jasnell@gmail.com>
Removed reliance on worker exit before arbitrary timeout. Instead of failing
the test after 200 or 1000 ms wait indefinitely for child process exit. If
the test hangs the test harness global timeout will kick in and fail the test.

Note that if the orphaned children are not reaped correctly (in the absence
of init, e.g. Docker) the test will hang and the harness will fail it.

PR-URL: #6531
Reviewed-By: Michael Dawson <michael_dawson@ca.ibm.com>
Reviewed-By: Andreas Madsen <amwebdk@gmail.com>
Reviewed-By: Santiago Gimeno <santiago.gimeno@gmail.com>
The flakiness issue for test-http-regr-gh-2928 on SmartOS was resolved
in late February in #5454. This
change removes its flaky designation in sequential.status.

PR-URL: #6540
Reviewed-By: James M Snell <jasnell@gmail.com>
`test-stdout-buffer-flush-on-exit` was not failing reliably on POSIX
machines and not failing at all on Windows. Revised test fails reliably
on POSIX and is skipped (in CI) on Windows where the issue does not
exist.

Fixes: #6527
PR-URL: #6555
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Anna Henningsen <anna@addaleax.net>
Reviewed-By: Joao Reis <reis@janeasystems.com>
Reviewed-By: Santiago Gimeno <santiago.gimeno@gmail.com>
Just copied the basic tests for log, as they're all the same thing
as log in either stdout or stderr. Cleaned that up a bit.

Also const-ified.

PR-URL: #6538
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
Reviewed-By: Evan Lucas <evanlucas@me.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
Add `known_issues` tests to `make test` and `make test-ci` targets and
their equivalents on Windows.

PR-URL: #6559
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
Reviewed-By: Anna Henningsen <anna@addaleax.net>
Allow out of order replies in the flaky
`test-dgram{-upd6,}-send-default-host.js` files by sorting
the incoming replies after receiving them.

PR-URL: #6607
Fixes: #6577
Reviewed-By: Ben Noordhuis <info@bnoordhuis.nl>
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Santiago Gimeno <santiago.gimeno@gmail.com>
Reviewed-By: Matteo Collina <matteo.collina@gmail.com>
addaleax and others added 10 commits June 2, 2016 22:42
Treat negative length arguments to `Buffer()`/`allocUnsafe()`
as if they were zero so the allocation does not affect the
pool’s offset.

Fixes: #7047
PR-URL: #7051
Reviewed-By: Sakthipriyan Vairamani <thechargingvolcano@gmail.com>
Reviewed-By: Ben Noordhuis <info@bnoordhuis.nl>
Reviewed-By: Сковорода Никита Андреевич <chalkerx@gmail.com>
Reviewed-By: Trevor Norris <trev.norris@gmail.com>
Reviewed-By: Rod Vagg <rod@vagg.org>
Allow the operating system to provide an arbitrary available port rather
than using `common.PORT`, as `common.PORT` makes it likely that a test
will fail with `EADDRINUSE` as a side effect of an earlier test.

PR-URL: #7013
Reviewed-By: Brian White <mscdex@mscdex.net>
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Ben Noordhuis <info@bnoordhuis.nl>
Previous version of weak used for gc tests emitted a warning on OS X.
Updating to current version eliminates warning.

PR-URL: #7014
Reviewed-By: Ben Noordhuis <info@bnoordhuis.nl>
Reviewed-By: James M Snell <jasnell@gmail.com>
test-https-strict sometimes fails with EADDRINUSE in CI. Remove use of
common.PORT to make the test resistant from side effects from other
tests that may have not freed up the port.

PR-URL: #7024
Reviewed-By: Brian White <mscdex@mscdex.net>
Reviewed-By: Anna Henningsen <anna@addaleax.net>
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
test-child-process-fork-net will sometimes fail in CI with EADDRINUSE
because an earlier test failed to free common.PORT. Have the operating
system provide an available port instead.

PR-URL: #7033
Reviewed-By: Brian White <mscdex@mscdex.net>
Reviewed-By: Ben Noordhuis <info@bnoordhuis.nl>
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
Reviewed-By: Anna Henningsen <anna@addaleax.net>
Passing the uid via v8::Integer::New() converts it to a uint32_t. Which
will trim the value early. Instead use v8::Number::New() to convert the
int64_t to a double so that JS can see the full 2^53 range of uid's.

PR-URL: #7096
Reviewed-By: Anna Henningsen <anna@addaleax.net>
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
Reviewed-By: Andreas Madsen <amwebdk@gmail.com>
OS X has a tiny 1kb hard-coded buffer size for stdout / stderr to
TTYs (terminals). Output larger than that causes chunking, which ends
up having some (very small but existent) delay past the first chunk.
That causes two problems:

1. When output is written to stdout and stderr at similar times, the
two can become mixed together (interleaved). This is especially
problematic when using control characters, such as \r. With
interleaving, chunked output will often have lines or characters erased
unintentionally, or in the wrong spots, leading to broken output.
CLI apps often extensively use such characters for things such as
progress bars.

2. Output can be lost if the process is exited before chunked writes
are finished flushing. This usually happens in applications that use
`process.exit()`, which isn't infrequent.

See #6980 for more info.

This became an issue as result of the Libuv 1.9.0 upgrade. A fix to
an unrelated issue broke a hack previously required for the OS X
implementation. This resulted in an unexpected behavior change in node.
The 1.9.0 upgrade was done in c3cec1e,
which was included in v6.0.0.
Full details of the Libuv issue that induced this are at
#6456 (comment)

Refs: #1771
Refs: #6456
Refs: #6773
Refs: #6816
PR-URL: #6895
Reviewed-By: Rod Vagg <rod@vagg.org>
Reviewed-By: Anna Henningsen <anna@addaleax.net>
Many thanks to thefourtheye and addaleax who helped make the python
bits of this possible.

See #6980 for more info regarding
the related TTY issues.

Refs: #6456
Refs: #6773
Refs: #6816
PR-URL: #6895
Reviewed-By: Rod Vagg <rod@vagg.org>
Reviewed-By: Anna Henningsen <anna@addaleax.net>
Remove uses of `common.PORT + 1337` where `common.PORT` suffices.

PR-URL: #7055
Reviewed-By: Ben Noordhuis <info@bnoordhuis.nl>
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
Refs: #6990
Move checklist instructions closer to the checklist. Trim unnecessary
words.

PR-URL: #7058
Reviewed-By: Johan Bergström <bugs@bergstroem.nu>
Reviewed-By: Michaël Zasso <mic.besace@gmail.com>
@nodejs-github-bot nodejs-github-bot added doc Issues and PRs related to the documentations. meta Issues and PRs related to the general management of the project. labels Jun 2, 2016
@rvagg
Copy link
Member Author

rvagg commented Jun 2, 2016

darn it, this PR is against master, I'm not going to both reopening it but it'll land on v6.x

@rvagg rvagg force-pushed the v6.2.1-proposal branch 2 times, most recently from 4091a88 to c853fe0 Compare June 2, 2016 13:58
@rvagg
Copy link
Member Author

rvagg commented Jun 2, 2016

@jasnell
Copy link
Member

jasnell commented Jun 2, 2016

Is the tty blocking on osx actually opt-in or set by default in this? I was under the impression that it was the latter.

@rvagg
Copy link
Member Author

rvagg commented Jun 2, 2016

that's what the wording is supposed to mean, "explicitly opt-in" rather than do it implicitly through a broken libuv, is it not clear enough?

@rvagg
Copy link
Member Author

rvagg commented Jun 2, 2016

Could do with some help interpreting citgm failures, are these expected? https://ci.nodejs.org/view/Node.js-citgm/job/citgm-smoker/288/ history looks patchy.

@rvagg
Copy link
Member Author

rvagg commented Jun 2, 2016

citgm hasn't finished but I'm sus about one of the failures, torrent-stream, I think it's suggesting that our version of npm has some problems.

I cloned torrent-stream and did an npm i && npm t on master and get test failures, something about its torrent-discovery dependency not having some method, so I cd node_modules/torrent-discovery && npm i && npm t and it's fine, go back up and npm t again and it passes! This is repeatable and passing entirely depends on doing an npm i in torrent-discovery. rm -rf node_modules && npm i && npm t fails, npm i from in the torrent-discovery dependency and the parent passes. Installing npm@2 and trying a fresh install and test works just fine.

@zkat @iarna is this to do with version matching compromises being made at the top level and entirely within the realms of normal behaviour for npm@3 flattening, or could this indicate a problem that we need to be concerned about?

I'm going to let this sit and finish the citgm run and hopefully get @thealphanerd to make a judgement about how it looks. The armv6 build is also going to take longer than usual because of a fresh workspace and a bunch of c++ changes included in this release. I'll get back on it in my morning.

@jasnell
Copy link
Member

jasnell commented Jun 2, 2016

No, I don't think it's clear. The way it's worded it sound as if the user is able to opt in as opposed to it being blocking by default on startup.

@MylesBorins
Copy link
Contributor

@rvagg perhaps s/Explicitly opt-in to blocking mode for stdio/Explicitly set blocking mode for stdio

@rvagg
Copy link
Member Author

rvagg commented Jun 2, 2016

doing some more testing with torrent-stream but I'm told that all of the known flaky tests in citgm have passed, moving forward with release after I confirm a few things

* buffer: Ignore negative lengths in calls to Buffer() and
  Buffer.allocUnsafe(). This fixes a possible security concern
  (reported by Feross Aboukhadijeh) where user input is passed
  unchecked to the Buffer constructor or allocUnsafe() as it can
  expose parts of the memory slab used by other Buffers in the
  application. Note that negative lengths are not supported by the
  Buffer API and user input to the constructor should always be
  sanitised and type-checked.
  (Anna Henningsen) #7030
* npm: Upgrade npm to 3.9.3
  (Kat Marchán) #7030
* tty: Default to blocking mode for stdio on OS X. A bug fix
  in libuv 1.9.0, introduced in Node.js v6.0.0, exposed problems with
  Node's use of non-blocking stdio, particularly on OS X which has a
  small output buffer. This change should fix CLI applications that
  have been having problems with output since Node.js v6.0.0 on OS X.
  The core team is continuing to address stdio concerns that exist
  across supported platforms and progress can be tracked at
  #6980.
  (Jeremiah Senkpiel) #6895
* V8: Upgrade to V8 5.0.71.52. This includes a fix that addresses
  problems experienced by users of node-inspector since Node.js
  v6.0.0, see #6980 for details.
  (Michaël Zasso) #6928
@rvagg
Copy link
Member Author

rvagg commented Jun 2, 2016

"Default to blocking mode for stdio on OS X"

@rvagg
Copy link
Member Author

rvagg commented Jun 2, 2016

the torrent-stream failure is the same for the previous version of npm we shipped so it's not a new thing

@mafintosh you might want to look into this (see above), an npm@3-specific bug with torrent-stream, looks like an npm bug to me but perhaps I'm not seeing something obvious.

@MylesBorins
Copy link
Contributor

@rvagg opened a issue when this first came up --> mafintosh/torrent-stream#155

issue on npm tracker too --> npm/npm#11062

@rvagg rvagg closed this Jun 2, 2016
@rvagg rvagg deleted the v6.2.1-proposal branch June 2, 2016 22:36
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
doc Issues and PRs related to the documentations. meta Issues and PRs related to the general management of the project.
Projects
None yet
Development

Successfully merging this pull request may close these issues.