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

[v6.x backport] build,windows: implement PEP514 python detection #14842

Closed
wants to merge 82 commits into from

Conversation

refack
Copy link
Contributor

@refack refack commented Aug 15, 2017

Checklist
  • make -j4 test (UNIX), or vcbuild test (Windows) passes
  • commit message follows commit guidelines
Affected core subsystem(s)

build

jasnell and others added 30 commits August 14, 2017 11:05
Export a new common.noop no-operation function for general use.
Allow using common.mustCall() without a fn argument to simplify
test cases.

Replace various non-op functions throughout tests with common.noop

PR-URL: nodejs#12027
Reviewed-By: Jeremiah Senkpiel <fishrock123@rocketmail.com>
Reviewed-By: Richard Lau <riclau@uk.ibm.com>
Reviewed-By: Gibson Fahnestock <gibfahn@gmail.com>
Reviewed-By: Teddy Katz <teddy.katz@gmail.com>
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
Reviewed-By: Franziska Hinkelmann <franziska.hinkelmann@gmail.com>
Prefer using `common.mustNotCall()` over `common.mustCall(fn, 0)`

PR-URL: nodejs#12027
Reviewed-By: Jeremiah Senkpiel <fishrock123@rocketmail.com>
Reviewed-By: Richard Lau <riclau@uk.ibm.com>
Reviewed-By: Gibson Fahnestock <gibfahn@gmail.com>
Reviewed-By: Teddy Katz <teddy.katz@gmail.com>
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
Reviewed-By: Franziska Hinkelmann <franziska.hinkelmann@gmail.com>
PR-URL: nodejs#12935
Reviewed-By: Anna Henningsen <anna@addaleax.net>
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
* add `mustCall` and `mustNotCall` to all callbacks
* added `known_issue` for port binding

Backport-PR-URL: nodejs#14327
PR-URL: nodejs#13100
Refs: nodejs#13055
Refs: nodejs#12999
Refs: nodejs#13526
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Matteo Collina <matteo.collina@gmail.com>
Fix the bug that you start process without network at first, but it
connected lately, `dns.resolve` will stay failed with ECONNREFUSED
because c-ares servers fallback to 127.0.0.1 at the very beginning.

If c-ares servers "127.0.0.1" is detected and its not set by user self,
and last query is not OK, recreating `ares_channel` operation will be
triggered to reload servers.

Fixes: nodejs#1644
Backport-PR-URL: nodejs#14434
PR-URL: nodejs#13076
Reviewed-By: Anna Henningsen <anna@addaleax.net>
Reviewed-By: Ben Noordhuis <info@bnoordhuis.nl>
A few of the CLI option values exposed as properties on the process
object were missing a comment, fix this.

Backport-PR-URL: nodejs#14483
PR-URL: nodejs#12245
Reviewed-By: Richard Lau <riclau@uk.ibm.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Anna Henningsen <anna@addaleax.net>
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
bootstrap_node.js was directly parsing process.execArgv to see if
internals should be exposed, even though the argv was already parsed by
node. This is unusual and unnecessary, change it to set the option value
from the parser onto the config binding.

Backport-PR-URL: nodejs#14483
PR-URL: nodejs#12245
Reviewed-By: Richard Lau <riclau@uk.ibm.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Anna Henningsen <anna@addaleax.net>
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
PR-URL: nodejs#14495
Fixes: nodejs#14430
Reviewed-By: Refael Ackermann <refack@gmail.com>
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
Backport-PR-URL: nodejs#14787
PR-URL: nodejs#14438
Reviewed-By: Refael Ackermann <refack@gmail.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Anna Henningsen <anna@addaleax.net>
Reviewed-By: Luigi Pinca <luigipinca@gmail.com>
Reviewed-By: Timothy Gu <timothygu99@gmail.com>
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
Reviewed-By: Khaidi Chu <i@2333.moe>
`path.resolve()` and `path.join()` are left alone in this commit due to
the lack of clear semantics.

Backport-PR-URL: nodejs#14787
PR-URL: nodejs#14440
Fixes: nodejs#14405
Reviewed-By: Refael Ackermann <refack@gmail.com>
Reviewed-By: Tobias Nießen <tniessen@tnie.de>
As the length of `path` is known at this point, there is no point in
making an exact copy using `slice`.

Backport-PR-URL: nodejs#14787
PR-URL: nodejs#14438
Reviewed-By: Refael Ackermann <refack@gmail.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Anna Henningsen <anna@addaleax.net>
Reviewed-By: Luigi Pinca <luigipinca@gmail.com>
Reviewed-By: Timothy Gu <timothygu99@gmail.com>
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
Reviewed-By: Khaidi Chu <i@2333.moe>
* we use u_setDataDirectory() in "unicode/putil.h"
* at present, this header is indirectly included,
  but this will change in ICU 59
* no impact on past ICUs.
* this is an exact analog to nodejs#11753

PR-URL: nodejs#12078
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Richard Lau <riclau@uk.ibm.com>
Timers should work even if the user has monkey-patched `.call()` and
`.apply()` to undesirable values.

PR-URL: nodejs#12960
Ref: nodejs#12956
Reviewed-By: Refael Ackermann <refack@gmail.com>
Reviewed-By: Jeremiah Senkpiel <fishrock123@rocketmail.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Joyee Cheung <joyeec9h3@gmail.com>
Reviewed-By: Rich Trott <rtrott@gmail.com>
Original commit message:

    This commit adds a getter for the private is_verbose_ member.
    The use case for this comes from Node.js where the ability to avoid
    calling FatalException if the TryCatch is verbose would be nice
    to have.

    BUG=

    Review-Url: https://codereview.chromium.org/2840803002
    Cr-Commit-Position: refs/heads/master@{nodejs#45018}

PR-URL: nodejs#12826
Reviewed-By: Anna Henningsen <anna@addaleax.net>
Reviewed-By: Ben Noordhuis <info@bnoordhuis.nl>
This commit attempts to address the TODO regarding not calling
FatalException if the try_catch is verbose.

PR-URL: nodejs#12826
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
Reviewed-By: Anna Henningsen <anna@addaleax.net>
Reviewed-By: Ben Noordhuis <info@bnoordhuis.nl>
`configure` will now call `node_gyp` as a module instead of forking
makes it easier to debug

PR-URL: nodejs#12653
Reviewed-By: Ben Noordhuis <info@bnoordhuis.nl>
Reviewed-By: Gibson Fahnestock <gibfahn@gmail.com>
When a no-op message event handler is used in a test, make it clear what
is expected by using `common.mustCall()` and `common.mustNotCall()`.

PR-URL: nodejs#13125
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Anna Henningsen <anna@addaleax.net>
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
Reviewed-By: Michael Dawson <mhdawson@ibm.com>
Reviewed-By: Gibson Fahnestock <gibfahn@gmail.com>
Reviewed-By: Refael Ackermann <refack@gmail.com>
Reviewed-By: Luigi Pinca <luigipinca@gmail.com>
Use common.mustCall() in test-fs-makeStatsCallback to confirm that the
callback is invoked.

PR-URL: nodejs#13132
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
Reviewed-By: Anna Henningsen <anna@addaleax.net>
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Michael Dawson <mhdawson@ibm.com>
Reviewed-By: Alexey Orlenko <eaglexrlnk@gmail.com>
Reviewed-By: Sakthipriyan Vairamani <thechargingvolcano@gmail.com>
Reviewed-By: Luigi Pinca <luigipinca@gmail.com>
In test/pummel/test-stream2-basic.js, check that noop functions are
called the expected number of times.

PR-URL: nodejs#13146
Reviewed-By: Refael Ackermann <refack@gmail.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Luigi Pinca <luigipinca@gmail.com>
Reviewed-By: Matteo Collina <matteo.collina@gmail.com>
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
PR-URL: nodejs#13146
Reviewed-By: Refael Ackermann <refack@gmail.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Luigi Pinca <luigipinca@gmail.com>
Reviewed-By: Matteo Collina <matteo.collina@gmail.com>
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
test-stream2-basic runs in a few seconds. It can be moved to parallel.

PR-URL: nodejs#13146
Reviewed-By: Refael Ackermann <refack@gmail.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Luigi Pinca <luigipinca@gmail.com>
Reviewed-By: Matteo Collina <matteo.collina@gmail.com>
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
Use the same dtrace command on ARM as on i386. Patch obtained from
FreeBSD PR 218081 [1].

1. https://bugs.freebsd.org/bugzilla/show_bug.cgi?id=218081

PR-URL: nodejs#12193
Reviewed-By: Ben Noordhuis <info@bnoordhuis.nl>
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
PR-URL: nodejs#13215
Refs: nodejs#12586
Reviewed-By: Luigi Pinca <luigipinca@gmail.com>
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
Reviewed-By: Refael Ackermann <refack@gmail.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Gibson Fahnestock <gibfahn@gmail.com>
Use `common.mustNotCall()` in test-stream2-objects.js to confirm that
noop function is never invoked.

PR-URL: nodejs#13249
Reviewed-By: Refael Ackermann <refack@gmail.com>
Reviewed-By: Luigi Pinca <luigipinca@gmail.com>
Reviewed-By: Benjamin Gruenbaum <benjamingr@gmail.com>
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
Use `common.mustCall()` to make sure noop function is called as
expected.

PR-URL: nodejs#13259
Reviewed-By: Refael Ackermann <refack@gmail.com>
Reviewed-By: Richard Lau <riclau@uk.ibm.com>
Reviewed-By: Luigi Pinca <luigipinca@gmail.com>
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
* Use common.mustNotCall() and common.mustCall() as appropriate
* Use block scoping
* Move assertions out of `exit` handler and into callbacks
* Order assert.strictEqual() args per docs
* Remove console.log() calls
* Move test from `parallel` to `sequential` so `common.PORT` can be used
  without conflicting with OS-provided ports in other `parallel` tests

PR-URL: nodejs#13273
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Luigi Pinca <luigipinca@gmail.com>
PR-URL: nodejs#13359
Reviewed-By: Daniel Bevenius <daniel.bevenius@gmail.com>
Reviewed-By: Luigi Pinca <luigipinca@gmail.com>
Reviewed-By: Vse Mozhet Byt <vsemozhetbyt@gmail.com>
Reviewed-By: Gibson Fahnestock <gibfahn@gmail.com>
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
Reviewed-By: Michael Dawson <michael_dawson@ca.ibm.com>
Reviewed-By: Richard Lau <riclau@uk.ibm.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: David Cai <davidcai1993@yahoo.com>
* Change common.noop to common.mustNotCall() to verify callback is not
  invoked.
* Add destructuring assignment for clarity. Yeah, clarity. That's why.

PR-URL: nodejs#13443
Reviewed-By: Anna Henningsen <anna@addaleax.net>
Reviewed-By: Gibson Fahnestock <gibfahn@gmail.com>
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Michael Dawson <michael_dawson@ca.ibm.com>
Use `common.mustNotCall()` to confirm that callback is not invoked when
`dns.lookup()` throws.

PR-URL: nodejs#13456
Reviewed-By: Tobias Nießen <tniessen@tnie.de>
Reviewed-By: Anna Henningsen <anna@addaleax.net>
Reviewed-By: Refael Ackermann <refack@gmail.com>
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Luigi Pinca <luigipinca@gmail.com>
Reviewed-By: Gibson Fahnestock <gibfahn@gmail.com>
* Check that noop callback is or isn't invoked as appropriate using
  common.mustCall() and common.mustNotCall()
* Fix typo in array literal

PR-URL: nodejs#13480
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Refael Ackermann <refack@gmail.com>
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
Reviewed-By: Luigi Pinca <luigipinca@gmail.com>
Reviewed-By: Michael Dawson <michael_dawson@ca.ibm.com>
vsemozhetbyt and others added 10 commits August 15, 2017 09:03
* Do not require if test is skipped.
* Do not re-require without need.
* Sort requiring by module names.

PR-URL: nodejs#14008
Reviewed-By: Refael Ackermann <refack@gmail.com>
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
Add clarification to the documentation on util.format()
and console.log() regarding how excessive arguments are treated
when the first argument is a non-format string
compared to when it is not a string at all.

PR-URL: nodejs#14027
Fixes: nodejs#13908
Reviewed-By: Vse Mozhet Byt <vsemozhetbyt@gmail.com>
Reviewed-By: Luigi Pinca <luigipinca@gmail.com>
Reviewed-By: Franziska Hinkelmann <franziska.hinkelmann@gmail.com>
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
Also fix repl and url libs for the rule.

PR-URL: nodejs#14032
Refs: http://eslint.org/docs/rules/no-use-before-define
Reviewed-By: Daijiro Wachi <daijiro.wachi@gmail.com>
Because of a race condition, connection listener may not be invoked if
test is run under load. Remove `common.mustCall()` wrapper from the
listener. Move the test to `parallel` because it now works under load.
Make similar change to http test to keep them in synch even though it is
much harder to trigger the race in http.

PR-URL: nodejs#14134
Fixes: nodejs#14133
Reviewed-By: Refael Ackermann <refack@gmail.com>
Reviewed-By: Benjamin Gruenbaum <benjamingr@gmail.com>
Reviewed-By: Luigi Pinca <luigipinca@gmail.com>
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Santiago Gimeno <santiago.gimeno@gmail.com>
The tests include a callback that might not be invoked but is wrapped in
common.mustCall(). Remove the common.mustCall() wrapper and add a
comment explaining that it should not be added.

Add a new test case that sets the timeout to 1ms and waits for both the
connection handler and the timeout handler to be invoked. This version
keeps the common.mustCall() wrapper intact around the connection handler
(although it's mostly semantic and not necessary for the test as the
test will certainly fail or time out if that handler isn't invoked).

PR-URL: nodejs#14380
Fixes: nodejs#11768
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
Reviewed-By: Refael Ackermann <refack@gmail.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
PR-URL: nodejs#13900
Fixes: nodejs#13882
Reviewed-By: Tobias Nießen <tniessen@tnie.de>
* rename :exit to :distexit

PR-URL: nodejs#13969
Refs: nodejs#13900 (review)
Reviewed-By: Tobias Nießen <tniessen@tnie.de>
Reviewed-By: João Reis <reis@janeasystems.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Gibson Fahnestock <gibfahn@gmail.com>
Reviewed-By: Rod Vagg <rod@vagg.org>
Handle spaces in the path to python.exe, in case it is installed
under some directory like "C:\Program Files".

PR-URL: nodejs#14546
Reviewed-By: Timothy Gu <timothygu99@gmail.com>
Reviewed-By: Refael Ackermann <refack@gmail.com>
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Anna Henningsen <anna@addaleax.net>
@nodejs-github-bot nodejs-github-bot added build Issues and PRs related to build files or the CI. install Issues and PRs related to the installers. tools Issues and PRs related to the tools directory. v6.x windows Issues and PRs related to the Windows platform. labels Aug 15, 2017
@refack refack added python PRs and issues that require attention from people who are familiar with Python. v6.x and removed install Issues and PRs related to the installers. v6.x labels Aug 15, 2017
@MylesBorins
Copy link
Contributor

landed in 44d3047...1b9563a

MylesBorins pushed a commit that referenced this pull request Aug 16, 2017
Backport-PR-URL: #14842
PR-URL: #13900
Fixes: #13882
Reviewed-By: Tobias Nießen <tniessen@tnie.de>
MylesBorins pushed a commit that referenced this pull request Aug 16, 2017
* rename :exit to :distexit

Backport-PR-URL: #14842
PR-URL: #13969
Refs: #13900 (review)
Reviewed-By: Tobias Nießen <tniessen@tnie.de>
Reviewed-By: João Reis <reis@janeasystems.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Gibson Fahnestock <gibfahn@gmail.com>
Reviewed-By: Rod Vagg <rod@vagg.org>
MylesBorins pushed a commit that referenced this pull request Aug 16, 2017
Handle spaces in the path to python.exe, in case it is installed
under some directory like "C:\Program Files".

Backport-PR-URL: #14842
PR-URL: #14546
Reviewed-By: Timothy Gu <timothygu99@gmail.com>
Reviewed-By: Refael Ackermann <refack@gmail.com>
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Anna Henningsen <anna@addaleax.net>
MylesBorins pushed a commit that referenced this pull request Sep 5, 2017
Backport-PR-URL: #14842
PR-URL: #13900
Fixes: #13882
Reviewed-By: Tobias Nießen <tniessen@tnie.de>
MylesBorins pushed a commit that referenced this pull request Sep 5, 2017
* rename :exit to :distexit

Backport-PR-URL: #14842
PR-URL: #13969
Refs: #13900 (review)
Reviewed-By: Tobias Nießen <tniessen@tnie.de>
Reviewed-By: João Reis <reis@janeasystems.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Gibson Fahnestock <gibfahn@gmail.com>
Reviewed-By: Rod Vagg <rod@vagg.org>
MylesBorins pushed a commit that referenced this pull request Sep 5, 2017
Handle spaces in the path to python.exe, in case it is installed
under some directory like "C:\Program Files".

Backport-PR-URL: #14842
PR-URL: #14546
Reviewed-By: Timothy Gu <timothygu99@gmail.com>
Reviewed-By: Refael Ackermann <refack@gmail.com>
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Anna Henningsen <anna@addaleax.net>
@refack refack deleted the backport-13900-to-v6.x branch October 24, 2017 13:53
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
build Issues and PRs related to build files or the CI. python PRs and issues that require attention from people who are familiar with Python. tools Issues and PRs related to the tools directory. windows Issues and PRs related to the Windows platform.
Projects
None yet
Development

Successfully merging this pull request may close these issues.