Skip to content

build: improve consistency between workflows #40986

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

Closed
wants to merge 161 commits into from

Conversation

Mesteery
Copy link
Contributor

No description provided.

@nodejs-github-bot nodejs-github-bot added the meta Issues and PRs related to the general management of the project. label Nov 26, 2021
@Mesteery Mesteery force-pushed the actions-consistency branch from 0db2a54 to a82c5db Compare November 26, 2021 21:30
@Mesteery Mesteery force-pushed the actions-consistency branch from f4a307c to bf2b40a Compare November 27, 2021 10:33
@targos
Copy link
Member

targos commented Nov 30, 2021

@Mesteery can you please fix the conflicts?

@Mesteery Mesteery force-pushed the actions-consistency branch from bf2b40a to e8df133 Compare December 12, 2021 20:18
tniessen and others added 22 commits January 14, 2022 15:08
PR-URL: nodejs#41490
Co-authored-by: Anna Henningsen <[email protected]>
Reviewed-By: Anna Henningsen <[email protected]>
Reviewed-By: Colin Ihrig <[email protected]>
Original commit message:

    [liftoff] Allow bailout for missing ARMv7

    The bailout is there explicitly in the code, so we should allow it in
    {CheckBailoutAllowed}.

    [email protected]

    Bug: v8:12527
    Change-Id: Ifd906afb5f034f05c2bf7d9a28e3ab458549e7ef
    Reviewed-on: https://chromium-review.googlesource.com/c/v8/v8/+/3372915
    Reviewed-by: Andreas Haas <[email protected]>
    Commit-Queue: Clemens Backes <[email protected]>
    Cr-Commit-Position: refs/heads/main@{#78515}

Refs: v8/v8@3b6b21f

Fixes: nodejs#41402

PR-URL: nodejs#41457
Reviewed-By: Jiawen Geng <[email protected]>
Reviewed-By: Richard Lau <[email protected]>
Reviewed-By: Anna Henningsen <[email protected]>
Reviewed-By: Colin Ihrig <[email protected]>
Reviewed-By: Tobias Nießen <[email protected]>
Reviewed-By: Darshan Sen <[email protected]>
access() does not support OR'ing F_OK with other constants.
This commit updates test-fs-access.js to not test that
scenario.

PR-URL: nodejs#41484
Refs: libuv/libuv#3410
Reviewed-By: Richard Lau
Reviewed-By: Luigi Pinca
Reviewed-By: Tobias Nießen
This commit expands the documentation for the mode parameter
passed to the fs.access() family of functions.

PR-URL: nodejs#41484
Refs: libuv/libuv#3410
Reviewed-By: Richard Lau
Reviewed-By: Luigi Pinca
Reviewed-By: Tobias Nießen
PR-URL: nodejs#41426
Reviewed-By: Richard Lau <[email protected]>
Reviewed-By: Anatoli Papirovski <[email protected]>
Reviewed-By: Luigi Pinca <[email protected]>
Reviewed-By: Rich Trott <[email protected]>
Refs: nodejs#41421

PR-URL: nodejs#41453
Reviewed-By: Tobias Nießen <[email protected]>
Reviewed-By: Rich Trott <[email protected]>
Reviewed-By: Colin Ihrig <[email protected]>
Reviewed-By: Michael Dawson <[email protected]>
Reviewed-By: Darshan Sen <[email protected]>
Refs: nodejs#41434

PR-URL: nodejs#41516
Reviewed-By: Richard Lau <[email protected]>
Reviewed-By: Darshan Sen <[email protected]>
Reviewed-By: Colin Ihrig <[email protected]>
This adds a missing _is_ in the readable.read() text and makes
small style adjustments.

PR-URL: nodejs#41524
Reviewed-By: Richard Lau <[email protected]>
Reviewed-By: Tobias Nießen <[email protected]>
Reviewed-By: Antoine du Hamel <[email protected]>
Reviewed-By: Darshan Sen <[email protected]>
Reviewed-By: Benjamin Gruenbaum <[email protected]>
PR-URL: nodejs#41503
Reviewed-By: Colin Ihrig <[email protected]>
Reviewed-By: Ruy Adorno <[email protected]>
Reviewed-By: Rich Trott <[email protected]>
Reviewed-By: Mohammed Keyvanzadeh <[email protected]>
Reviewed-By: Darshan Sen <[email protected]>
On WSL systems, `./configure` causes appending of carriage return
(`\r\r`) as leftover and will be appended to the `gyp_args`.
Therefore, it will lead to unhandled exceptions from the `./configure`
execution.
Excluded the empty or whitespace item from the `args` array to
fix the issue.

Fixes: nodejs#41459

PR-URL: nodejs#41476
Reviewed-By: Anna Henningsen <[email protected]>
Reviewed-By: Tobias Nießen <[email protected]>
Reviewed-By: Benjamin Gruenbaum <[email protected]>
Reviewed-By: Mohammed Keyvanzadeh <[email protected]>
Reviewed-By: Darshan Sen <[email protected]>
Reviewed-By: Michael Dawson <[email protected]>
Remove comparison to null of variable guaranteed to be a boolean.

PR-URL: nodejs#41488
Reviewed-By: Matteo Collina <[email protected]>
Reviewed-By: Benjamin Gruenbaum <[email protected]>
Reviewed-By: Robert Nagy <[email protected]>
Reviewed-By: Tobias Nießen <[email protected]>
Reviewed-By: Darshan Sen <[email protected]>
The internal use of tls.parseCertString was removed in
a336444. The function does not handle
multi-value RDNs correctly, leading to incorrect representations and
security concerns.

This change is breaking in two ways: tls.parseCertString is removed
(but has been runtime-deprecated since Node.js 9) and
_tls_common.translatePeerCertificate does not translate the `subject`
and `issuer` properties anymore.

This change also removes the recommendation to use querystring.parse
instead, which is similarly dangerous.

PR-URL: nodejs#41479
Reviewed-By: James M Snell <[email protected]>
Reviewed-By: Benjamin Gruenbaum <[email protected]>
Reviewed-By: Matteo Collina <[email protected]>
Reviewed-By: Richard Lau <[email protected]>
Reviewed-By: Colin Ihrig <[email protected]>
Fixes: nodejs#41216

PR-URL: nodejs#41543
Reviewed-By: Antoine du Hamel <[email protected]>
Reviewed-By: Qingyu Deng <[email protected]>
Reviewed-By: Ben Coe <[email protected]>
PR-URL: nodejs#41544
Reviewed-By: Zeyu Yang <[email protected]>
Reviewed-By: Mestery <[email protected]>
Reviewed-By: Luigi Pinca <[email protected]>
Reviewed-By: Colin Ihrig <[email protected]>
This statement was objectively false. Clients usually only need to
generate and/or own a private key if the server sends a
CertificateRequest during the TLS handshake, which is not a common case.

PR-URL: nodejs#41505
Reviewed-By: Filip Skokan <[email protected]>
Reviewed-By: Benjamin Gruenbaum <[email protected]>
Reviewed-By: Colin Ihrig <[email protected]>
Reviewed-By: Michael Dawson <[email protected]>
PR-URL: nodejs#41548
Reviewed-By: Rich Trott <[email protected]>
Reviewed-By: Mestery <[email protected]>
This statement is misleading in that it says "key generation is
expensive". ECDHE key generation (over the elliptic curves that are
commonly used for TLS) is insanely fast compared to most other types
of key generation.

This statement is irrelevant for TLS 1.3, which requires (EC)DHE.

Even if this statement is somewhat true for TLS 1.2, it does not
justify discouraging the use of (EC)DHE.

PR-URL: nodejs#41528
Reviewed-By: Rich Trott <[email protected]>
Reviewed-By: Anna Henningsen <[email protected]>
Reviewed-By: Benjamin Gruenbaum <[email protected]>
`hash.digest('buffer')` has returned a Buffer and not a string since at
least Node.js 0.10.6. The benchmark, as it is written, will not work on
any version of Node.js prior to 16.x (due to `Object.hasOwn()`) and
certainly won't run on versions earlier than 0.10.6 due to const/let and
probably other things. Remove impossible-to-reach code intended to
accommodate Node.js earlier than 0.10.6.

PR-URL: nodejs#41535
Reviewed-By: Tobias Nießen <[email protected]>
Reviewed-By: Luigi Pinca <[email protected]>
Instead of assigning a boolean, move the function call that assigns a
value to the boolean to the only place that boolean is checked. This
avoids the function call in cases where it is not needed.

Refs: nodejs#41488 (review)

PR-URL: nodejs#41534
Reviewed-By: Tobias Nießen <[email protected]>
Reviewed-By: Luigi Pinca <[email protected]>
Reviewed-By: Antoine du Hamel <[email protected]>
Reviewed-By: Gerhard Stöbich <[email protected]>
Reviewed-By: Matteo Collina <[email protected]>
Coverage stats indicate that there is no coverage for util.inspect()
with a negative number and a numeric separator. Add a test case.

Refs: https://coverage.nodejs.org/coverage-df507758e6c35534/lib/internal/util/inspect.js.html#L1463

PR-URL: nodejs#41527
Reviewed-By: Benjamin Gruenbaum <[email protected]>
Reviewed-By: Antoine du Hamel <[email protected]>
Reviewed-By: Luigi Pinca <[email protected]>
Reviewed-By: Mohammed Keyvanzadeh <[email protected]>
Reviewed-By: Tobias Nießen <[email protected]>
Refs: nodejs#41345 (comment)

PR-URL: nodejs#41383
Reviewed-By: Guy Bedford <[email protected]>
Reviewed-By: Geoffrey Booth <[email protected]>
Trott and others added 27 commits January 25, 2022 02:04
Closes: nodejs#41653

PR-URL: nodejs#41658
Fixes: nodejs#41653
Reviewed-By: Richard Lau <[email protected]>
Reviewed-By: Mohammed Keyvanzadeh <[email protected]>
Reviewed-By: Luigi Pinca <[email protected]>
Reviewed-By: Darshan Sen <[email protected]>
Reviewed-By: Mestery <[email protected]>
Reviewed-By: Benjamin Gruenbaum <[email protected]>
PR-URL: nodejs#41661
Reviewed-By: Mohammed Keyvanzadeh <[email protected]>
Reviewed-By: Benjamin Gruenbaum <[email protected]>
Reviewed-By: Mestery <[email protected]>
Reviewed-By: Darshan Sen <[email protected]>
Reviewed-By: Luigi Pinca <[email protected]>
The statement removed was added in 6657b15 but it was not added
as an assertion. It currently returns false but t0he test does not fail
because of the missing assertion. Since the property is no longer one
that exists, there is no need to test its existence. Remove the line.

PR-URL: nodejs#41663
Reviewed-By: Benjamin Gruenbaum <[email protected]>
Reviewed-By: Mestery <[email protected]>
Reviewed-By: Darshan Sen <[email protected]>
Reviewed-By: Luigi Pinca <[email protected]>
Reviewed-By: Tierney Cyren <[email protected]>
Replace Object.prototpye.hasOwnProperty() with Object.hasOwn() where
applicable.

PR-URL: nodejs#41664
Reviewed-By: Benjamin Gruenbaum <[email protected]>
Reviewed-By: Antoine du Hamel <[email protected]>
Reviewed-By: Darshan Sen <[email protected]>
Reviewed-By: Mestery <[email protected]>
Reviewed-By: Anna Henningsen <[email protected]>
Reviewed-By: Tierney Cyren <[email protected]>
Reviewed-By: Luigi Pinca <[email protected]>
Refs: nodejs/help#3698

Signed-off-by: Michael Dawson <[email protected]>

PR-URL: nodejs#41635
Reviewed-By: Tobias Nießen <[email protected]>
Reviewed-By: Benjamin Gruenbaum <[email protected]>
Refs: nodejs/next-10#104

Add initial doc to capture how the project supports
the flow for types generation. Based on discussions
from the Next-10 mini-summit that included project
members and members of the Definitely Typed team.

Will need to be updated as we improve the flow and
resulting json but captures the current flow and
approach and will give us the place where we can
update as things change.

Signed-off-by: Michael Dawson <[email protected]>

PR-URL: nodejs#41464
Reviewed-By: James M Snell <[email protected]>
Reviewed-By: Chengzhong Wu <[email protected]>
Reviewed-By: Michaël Zasso <[email protected]>
Applications should usually keep track of workers themselves to prevent
cross-referencing workers from different "groups" that are assigned to
separate tasks.

Additionally, it is unreasonable to assume that the 'data' event emitted
by a socket object will be an integer. While the example works when the
argument is a string (or Buffer), it can result in various issues (e.g.,
when id === '__proto__' since cluster.workers has a non-null prototype).

Refs: nodejs@5f08c3c

PR-URL: nodejs#41668
Reviewed-By: Benjamin Gruenbaum <[email protected]>
Reviewed-By: Rich Trott <[email protected]>
Reviewed-By: Mestery <[email protected]>
Reviewed-By: Colin Ihrig <[email protected]>
Reviewed-By: Tierney Cyren <[email protected]>
`heap dumps` spelled as `headumps`

PR-URL: nodejs#41694
Reviewed-By: Mohammed Keyvanzadeh <[email protected]>
Reviewed-By: Mestery <[email protected]>
Reviewed-By: Luigi Pinca <[email protected]>
Reviewed-By: Tobias Nießen <[email protected]>
Reviewed-By: Rich Trott <[email protected]>
Reviewed-By: Darshan Sen <[email protected]>
PR-URL: nodejs#41685
Reviewed-By: Mohammed Keyvanzadeh <[email protected]>
Reviewed-By: Darshan Sen <[email protected]>
Add the asIndexedPairs method for readable streams.

PR-URL: nodejs#41681
Refs: https://github.com/tc39/proposal-iterator-helpers#asindexedpairs
Reviewed-By: Robert Nagy <[email protected]>
Reviewed-By: Matteo Collina <[email protected]>
The OOM test uses a value that caused an OOM crash from V8 on certain
machines when V8 did not notify the host of OOM soon enough.

PR-URL: nodejs#41681
Refs: https://github.com/tc39/proposal-iterator-helpers#asindexedpairs
Reviewed-By: Robert Nagy <[email protected]>
Reviewed-By: Matteo Collina <[email protected]>
 is no longer a generator function,
instead it returns a called generator so that validation can be
synchronous and not wait for the first iteration

Fixes: nodejs#41648

PR-URL: nodejs#41652
Reviewed-By: Benjamin Gruenbaum <[email protected]>
Reviewed-By: Matteo Collina <[email protected]>
Reviewed-By: Robert Nagy <[email protected]>
Avoid Object.prototype.hasOwnProperty. Use primordial instead.

PR-URL: nodejs#41692
Reviewed-By: Benjamin Gruenbaum <[email protected]>
Reviewed-By: Tierney Cyren <[email protected]>
Reviewed-By: Luigi Pinca <[email protected]>
The previous link led to an old page that does not document all the
`package.json` fields used by npm.

PR-URL: nodejs#41712
Reviewed-By: Mestery <[email protected]>
Reviewed-By: Tobias Nießen <[email protected]>
Reviewed-By: Benjamin Gruenbaum <[email protected]>
Reviewed-By: Tierney Cyren <[email protected]>
Reviewed-By: Geoffrey Booth <[email protected]>
Reviewed-By: Luigi Pinca <[email protected]>
PR-URL: nodejs#41650
Reviewed-By: Antoine du Hamel <[email protected]>
Reviewed-By: Darshan Sen <[email protected]>
Reviewed-By: Mestery <[email protected]>
Reviewed-By: Tierney Cyren <[email protected]>
PR-URL: nodejs#41723
Reviewed-By: Rich Trott <[email protected]>
Reviewed-By: Antoine du Hamel <[email protected]>
PR-URL: nodejs#41502
Reviewed-By: Matteo Collina <[email protected]>
Reviewed-By: Stephen Belanger <[email protected]>
Refs: nodejs#41675

PR-URL: nodejs#41695
Reviewed-By: Colin Ihrig <[email protected]>
Reviewed-By: Mestery <[email protected]>
Reviewed-By: Mohammed Keyvanzadeh <[email protected]>
Reviewed-By: Luigi Pinca <[email protected]>
PR-URL: nodejs#41701
Reviewed-By: Rich Trott <[email protected]>
Reviewed-By: Mestery <[email protected]>
Reviewed-By: Mohammed Keyvanzadeh <[email protected]>
Reviewed-By: Luigi Pinca <[email protected]>
Reviewed-By: Benjamin Gruenbaum <[email protected]>
Reviewed-By: Harshitha K P <[email protected]>
Support cross realm errors where `instanceof` errors in our
unhandledRejection warning to print better error with stack traces.

PR-URL: nodejs#41682
Refs: nodejs#41676
Reviewed-By: Nitzan Uziely <[email protected]>
Reviewed-By: Tierney Cyren <[email protected]>
Coverity reported some calls that had no effect,
remove them

Signed-off-by: Michael Dawson <[email protected]>

PR-URL: nodejs#41579
Reviewed-By: Yash Ladha <[email protected]>
Reviewed-By: Darshan Sen <[email protected]>
Reviewed-By: Richard Lau <[email protected]>
After Windows 10 version 1903, placeholder files named "python.exe"
and "python3.exe" appear, not executable programs, and open the
MS Store when running without parameters.

Fixes: nodejs#36694
Refs: https://devblogs.microsoft.com/python/python-in-the-windows-10-may-2019-update

PR-URL: nodejs#36696
Reviewed-By: Jiawen Geng <[email protected]>
Reviewed-By: James M Snell <[email protected]>
PR-URL: nodejs#41683
Reviewed-By: Antoine du Hamel <[email protected]>
Reviewed-By: Benjamin Gruenbaum <[email protected]>
Reviewed-By: Luigi Pinca <[email protected]>
Reviewed-By: James M Snell <[email protected]>
Signed-off-by: Darshan Sen <[email protected]>

PR-URL: nodejs#41667
Reviewed-By: Mestery <[email protected]>
Reviewed-By: Rich Trott <[email protected]>
Reviewed-By: Luigi Pinca <[email protected]>
Reviewed-By: Tobias Nießen <[email protected]>
Reviewed-By: James M Snell <[email protected]>
PR-URL: nodejs#41746
Reviewed-By: Michaël Zasso <[email protected]>
Reviewed-By: Mestery <[email protected]>
Reviewed-By: Antoine du Hamel <[email protected]>
Reviewed-By: Benjamin Gruenbaum <[email protected]>
Reviewed-By: Darshan Sen <[email protected]>
Refs: nodejs#41683 (review)

PR-URL: nodejs#41751
Reviewed-By: Mestery <[email protected]>
Reviewed-By: Antoine du Hamel <[email protected]>
Reviewed-By: Benjamin Gruenbaum <[email protected]>
@Mesteery Mesteery closed this Jan 31, 2022
@Mesteery Mesteery deleted the actions-consistency branch February 7, 2022 17:55
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
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.