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

v13.10.1 proposal #32099

Merged
merged 17 commits into from
Mar 5, 2020
Merged

v13.10.1 proposal #32099

merged 17 commits into from
Mar 5, 2020

Conversation

MylesBorins
Copy link
Contributor

@MylesBorins MylesBorins commented Mar 4, 2020

2020-03-04, Version 13.10.1 (Current), @MylesBorins

Notable Changes

In Node.js 13.9.0 deps/zlib was switched to the chromium maintained implementation. This change
had the unforseen consequence of breaking building from the tarballs we release as we were too
aggressively removing unneccessary files from the deps/zlib folder. This release includes
a patch that ensures that individuals will once again be able to build Node.js from source.

Commits

  • [723aa41d96] - build: fix zlib tarball generation (Shelley Vohr) #32094
  • [9c1ac50fc5] - build: fix building with ninja (Richard Lau) #32071
  • [478450d6b3] - build: add asan check in Github action (gengjiawen) #31902
  • [0fc45f80b5] - crypto: simplify exportKeyingMaterial (Tobias Nießen) #31922
  • [4dc59b91a7] - dgram: make UDPWrap more reusable (Anna Henningsen) #31871
  • [4ed720e940] - doc: visibility of Worker threads cli options (Harshitha KP) #31380
  • [2518213a1b] - doc: improve doc/markdown file organization coherence (ConorDavenport) #31792
  • [ba3f7ff94d] - doc: update stream.pipeline() signature (vsemozhetbyt) #31789
  • [3c8daa3aa0] - events: convert errorMonitor to a normal property (Gerhard Stoebich) #31848
  • [6b44df2415] - perf,src: add HistogramBase and internal/histogram.js (James M Snell) #31988
  • [6a9cea9ed2] - src: pass resource object along with InternalMakeCallback (Anna Henningsen) #32063
  • [70f046010c] - src: start the .text section with an asm symbol (Gabriel Schulhof) #31981
  • [755da035ce] - src: add node_crypto_common and refactor (James M Snell) #32016
  • [4d5318c164] - src: improve handling of internal field counting (James M Snell) #31960
  • [1539928ed9] - test: add GC test for disabled AsyncLocalStorage (Andrey Pechkurov) #31995
  • [be90817558] - test: remove common.port from test-tls-securepair-client (Rich Trott) #32024

vsemozhetbyt and others added 15 commits March 4, 2020 16:28
PR-URL: #31922
Reviewed-By: Anna Henningsen <[email protected]>
Reviewed-By: Colin Ihrig <[email protected]>
Reviewed-By: James M Snell <[email protected]>
Reviewed-By: David Carlier <[email protected]>
Change suggested by bnoordhuis.

Improve handing of internal field counting by using enums.
Helps protect against future possible breakage if field
indexes are ever changed or added to.

Signed-off-by: James M Snell <[email protected]>

PR-URL: #31960
Reviewed-By: Anna Henningsen <[email protected]>
Reviewed-By: Franziska Hinkelmann <[email protected]>
Reviewed-By: Ben Noordhuis <[email protected]>
Reviewed-By: Matteo Collina <[email protected]>
Separating this out from the QUIC PR to allow it to be separately
reviewed. The QUIC implementation makes use of the hdr_histogram
for dynamic performance monitoring. This introduces a BaseObject
class that allows the internal histograms to be accessed on the
JavaScript side and adds a generic Histogram class that will be
used by both QUIC and perf_hooks (for the event loop delay
monitoring).

Signed-off-by: James M Snell <[email protected]>

PR-URL: #31988
Reviewed-By: Anna Henningsen <[email protected]>
Reviewed-By: Matteo Collina <[email protected]>
OpenSSL s_server accepts port 0 as an indicator to use an open port
provided by the operating system. Use that instead of common.PORT in the
test.

Remove 500ms delay added in 8e46167.
Hopefully the race condition in OpenSSL s_server has been fixed and/or
the change to port 0 means that the server is listening by the time
the ACCEPT text is printed and the setTimeout() is no longer necessary.

PR-URL: #32024
Reviewed-By: Ben Coe <[email protected]>
Reviewed-By: Sam Roberts <[email protected]>
Convert property errorMonitor to a normal property as non-writable
caused unwanted side effects.

Refs: #30932 (comment)

PR-URL: #31848
Reviewed-By: Michaël Zasso <[email protected]>
Reviewed-By: Luigi Pinca <[email protected]>
Reviewed-By: James M Snell <[email protected]>
Reviewed-By: Matheus Marchini <[email protected]>
Reviewed-By: Anna Henningsen <[email protected]>
Reviewed-By: Colin Ihrig <[email protected]>
PR-URL: #31995
Reviewed-By: Anna Henningsen <[email protected]>
Reviewed-By: Gireesh Punathil <[email protected]>
Reviewed-By: Stephen Belanger <[email protected]>
Reviewed-By: Vladimir de Turckheim <[email protected]>
Reviewed-By: James M Snell <[email protected]>
Reviewed-By: Michael Dawson <[email protected]>
Two things in one on this commit:

(a) For the QUIC implementation, we need to separate out various bits
    from node_crypto.cc to allow them to be reused. That's where this
    commit starts.

(b) Quite a bit of the node_crypto.cc code was just messy in terms of
    it's organization and lack of error handling and use of Local vs.
    MaybeLocal. This cleans that up a bit and hopefully makes certain
    parts a bit more manageable also.

Signed-off-by: James M Snell <[email protected]>

PR-URL: #32016
Reviewed-By: Anna Henningsen <[email protected]>
Reviewed-By: Sam Roberts <[email protected]>
We create an object file in assembly which introduces the symbol
`__node_text_start` into the .text section and place the resulting
object file as the first file the linker encounters. We do this to
ensure that we can recognize the boundaries of the .text section when
attempting to establish the address range to map to large pages.

Additionally, we rename the section containing the remapping code from
`.lpstub` to `lpstub` so as to take advantage of the linker's feature
whereby it inserts the symbol `__start_lpstub` when the section's name
can be rendered as a valid C variable. We need this symbol in order to
avoid self-mapping the remapping code to large pages, because doing so
would cause the process to crash.

PR-URL: #31981
Reviewed-By: Franziska Hinkelmann <[email protected]>
Reviewed-By: Ben Noordhuis <[email protected]>
Reviewed-By: James M Snell <[email protected]>
Reviewed-By: David Carlier <[email protected]>
Signed-off-by: Gabriel Schulhof <[email protected]>
Allow using the handle more directly for I/O in other parts of
the codebase.

Originally landed in the QUIC repo

Original review metadata:

```
  PR-URL: nodejs/quic#165
  Reviewed-By: James M Snell <[email protected]>
  Reviewed-By: Daniel Bevenius <[email protected]>
```

Signed-off-by: James M Snell <[email protected]>

PR-URL: #31871
Reviewed-By: Anna Henningsen <[email protected]>
Reviewed-By: Ben Noordhuis <[email protected]>
The ninja build places objects in a different directory.

Co-authored-by: Gabriel Schulhof <[email protected]>
Signed-off-by: Richard Lau <[email protected]>

PR-URL: #32071
Reviewed-By: Matheus Marchini <[email protected]>
Reviewed-By: Gabriel Schulhof <[email protected]>
Reviewed-By: Sam Roberts <[email protected]>
Reviewed-By: Shelley Vohr <[email protected]>
Reviewed-By: Myles Borins <[email protected]>
Reviewed-By: Jiawen Geng <[email protected]>
* Updated cpp style guide file name and location and fixed links to
  this file.

* Updated collaborator guide file name and location and fixed links
  to this file.

* Updated documentation style guide file name and location and updated
  links referencing the file.

* Moved files to appropriate location and updated naming style for
  some of them.

Fixes: #31741

PR-URL: #31792
Reviewed-By: James M Snell <[email protected]>
Reviewed-By: Sam Roberts <[email protected]>
Fixes: #28518
PR-URL: #31380
Fixes: #28518
Reviewed-By: Anna Henningsen <[email protected]>
Reviewed-By: James M Snell <[email protected]>
PR-URL: #32094
Reviewed-By: James M Snell <[email protected]>
Reviewed-By: Richard Lau <[email protected]>
Reviewed-By: Myles Borins <[email protected]>
@nodejs-github-bot nodejs-github-bot added lib / src Issues and PRs related to general changes in the lib or src directory. v13.x labels Mar 4, 2020
@nodejs-github-bot

This comment has been minimized.

@nodejs-github-bot

This comment has been minimized.

@addaleax

This comment has been minimized.

This was an oversight in 9fdb6e6.
Fixing this is necessary to make `executionAsyncResource()` work
as expected.

Refs: #30959
Fixes: #32060

PR-URL: #32063
Reviewed-By: Vladimir de Turckheim <[email protected]>
Reviewed-By: Stephen Belanger <[email protected]>
Reviewed-By: Gireesh Punathil <[email protected]>
Reviewed-By: James M Snell <[email protected]>
MylesBorins added a commit that referenced this pull request Mar 4, 2020
Notable changes:

In Node.js 13.9.0 deps/zlib was switched to the chromium maintained
implementation. This change had the unforseen consequence of breaking
building from the tarballs we release as we were too aggressively
removing `unneccessary files` from the `deps/zlib` folder. This release
includes a patch that ensures that individuals will once again be able
to build Node.js from source.

PR-URL: #32099
@nodejs-github-bot
Copy link
Collaborator

nodejs-github-bot commented Mar 4, 2020

CHANGELOG.md Outdated Show resolved Hide resolved
Notable changes:

In Node.js 13.9.0 deps/zlib was switched to the chromium maintained
implementation. This change had the unforseen consequence of breaking
building from the tarballs we release as we were too aggressively
removing `unneccessary files` from the `deps/zlib` folder. This release
includes a patch that ensures that individuals will once again be able
to build Node.js from source.

PR-URL: #32099
@nodejs-github-bot
Copy link
Collaborator

nodejs-github-bot commented Mar 4, 2020

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

edit from @MylesBorins:

The only failure on this CI run was arm. After a handful of runs we were able to get a green run on both centos arm64 and ubuntu arm64, the two platforms regularly flaky while testing this release.

I believe the issue we are running into is network related, but will make myself available for a quick follow up release if we actually broke anything. Also worth mentioning I was able to reproduce the failures on 13.10.0, so we can't be that much worse off pushing 13.10.1 than we are doing nothing 😅

@richardlau
Copy link
Member

https://nodejs.org/download/rc/v13.10.1-rc.0/ appears to be missing the source tarball.

@MylesBorins
Copy link
Contributor Author

@richardlau it is still building 😅

@MylesBorins
Copy link
Contributor Author

So... I've been banging my head against CI for a little bit and I am going to make the executive decision to move forward even though we are seeing odd issues with arm 💀

After doing all the digging I can do with the tools available to me I strongly believe the issue to be network based. Further we've managed to get a green run on each platform at least once. I've put a more in depth explanation above

@MylesBorins MylesBorins merged commit 59ab6bf into v13.x Mar 5, 2020
MylesBorins added a commit that referenced this pull request Mar 5, 2020
MylesBorins added a commit that referenced this pull request Mar 5, 2020
Notable changes:

In Node.js 13.9.0 deps/zlib was switched to the chromium maintained
implementation. This change had the unforseen consequence of breaking
building from the tarballs we release as we were too aggressively
removing `unneccessary files` from the `deps/zlib` folder. This release
includes a patch that ensures that individuals will once again be able
to build Node.js from source.

PR-URL: #32099
MylesBorins added a commit to nodejs/nodejs.org that referenced this pull request Mar 5, 2020
@richardlau

This comment has been minimized.

@richardlau

This comment has been minimized.

@addaleax addaleax deleted the v13.10.1-proposal branch March 5, 2020 07:10
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
lib / src Issues and PRs related to general changes in the lib or src directory.
Projects
None yet
Development

Successfully merging this pull request may close these issues.