lib: migrate requests to fetch #2220

imatlopez · 2020-09-13T16:40:09Z

Checklist

npm install && npm test passes
tests are included
commit message follows commit guidelines

Description of change

requests is deprecated, this gets rid of it and modernizes parts of install.js

resolves #2047
closes #2198
closes #2222

imatlopez · 2020-09-13T16:41:21Z

@rvagg the tests are failing when downloading headers because the untar is not working with the streams here. Wonder if you might be able to find where?

after further testing, it's literally just the tar, I don't know what difference is between the streams of request and fetch

ok turns out it was that requests is an event emitter that resembles a stream, streams can only be consumed once

windows is now broken :(

rvagg · 2020-09-14T04:05:13Z

lib/install.js

-        return isValid
+    } catch (err) {
+      if (err.code === 'EACCES') {
+        eaccesFallback(err)


This is going to result in a double call to cb() because you've at once handed off flow control to the returning promise while also attempting to regain control here. eaccesFallback calls cb but you also have then(cb, cb) so 🤷. I'm not really sure how best to resovle this without pulling it apart even further, the middle-place with half Promises and half callbacks is very unpleasant. (See also #2140 which perhaps you'd be interested in contributing to if you're interested in this challenge).

rvagg · 2020-09-14T04:11:48Z

lib/install.js

-
-          fs.mkdir(dir, { recursive: true }, function (err) {
-            if (err) {
-              return done(err)


just noticing all of the potential double-callbacks for errors in this old code, yikes!

rvagg · 2020-09-14T04:21:28Z

I can't see anything obvious that would make all windows and some macos to fail (the macos failure could suggest a flaky?). The windows version has to do a lot more parallel downloads in the downloadNodeLibs() so maybe there's something in there going awry. Maybe print some debug output in there that will show up in the Actions output - print what's going to get downloaded and what you eventually successfully get downloaded. Maybe there's a missing stream drain or missed callback in there somewhere that's not obvious.

Migrate the use of deprecated `requests` to `make-fetch-happen`. Replaces a lot of callbacks with async/await or promises.

Some tests are calling the recently converted promise apis, this converts them to use the new format.

Because listeners were placed on the response body, it caused the data to be consumed prematurely, hence using a stream Transform, no data is consumed.

make-fetch-happen already has proxy support

Due to a posibility that callbacks are called twice on error, the install function is now internally completely promisified.

Windows seems to be hanging, hopefully this reduces the number of promises in the chain.

Tests are were not waiting for their servers to close.

oliverjessner · 2021-01-22T07:58:56Z

lets merge this?

Otherwise any succesful return would make the program think it failed

imatlopez · 2021-02-02T12:49:38Z

@rvagg should this get a semver-major tag?

imatlopez · 2021-02-20T01:35:42Z

@rvagg @cclauss since python2 is being dropped, which is a major change, is now the time to get this in?

rvagg · 2021-02-20T08:42:52Z

Yeah, semver-major to be safe, too much can go wrong with this critical piece, although it's great to see make-fetch-happen being used so we're more likely to have parity with the same environments as npm as long as we're using it roughly the same. I don't have time to review this unfortunately, do we have another collaborator here that can look over and give it a 👍? I'm happy to ship this as a semver-major and maintain two majors for a little while (and probably ask npm not to hop on the new one too quickly).

gengjiawen

LGTM.

gengjiawen · 2021-03-05T03:25:23Z

LGTM. I have tested on linux with some example from https://github.com/nodejs/node-addon-examples, works good too.

indolent-developer · 2021-03-09T21:50:09Z

package.json

@@ -25,9 +25,9 @@
    "env-paths": "^2.2.0",
    "glob": "^7.1.4",
    "graceful-fs": "^4.2.3",
+    "make-fetch-happen": "^8.0.9",


I guess the fetch requirement if pretty basic. Should we just use native fetch function. No need to again upgrade in couple of years :)

There is (at the time of this writing) no native fetch function in Node.js.

how about https https://nodejs.org/api/https.html , https://nodejs.dev/learn/making-http-requests-with-nodejs

make-fetch-happen also deals with proxies which is a big thing for corporate users.

ok proxy support would be critical requirement. No point implementing that.

Trott · 2021-03-10T04:57:11Z

@nodejs/node-gyp Anyone else comfortable reviewing this?

gengjiawen · 2021-03-10T06:25:57Z

@nodejs/node-gyp Anyone else comfortable reviewing this?

@rvagg @cclauss

rvagg · 2021-03-10T08:58:17Z

Yeah, from my brief look a few months ago this seemed fine, I'm mainly concerned about the huge potential for breakage but using the npm http client does alleviate concerns and we can publish this as a semver-major. I really don't have the bandwidth to give this a proper review, but I give it a very general 👍 (maybe that's a conceptual 👍). @gengjiawen if you're happy with the code then that might be enough.

gengjiawen · 2021-03-18T01:30:28Z

Thanks for the great contribution 👍

* gyp: update gyp to 0.2.1 PR-URL: nodejs/node-gyp#2092 Reviewed-By: Rod Vagg <[email protected]> * deps: replace mkdirp with {recursive} mkdir only supported on Node.js 10+ Closes: #2084 PR-URL: nodejs/node-gyp#2123 Reviewed-By: Richard Lau <[email protected]> Reviewed-By: Jiawen Geng <[email protected]> * doc: update acid test and introduce curl|bash test script PR-URL: nodejs/node-gyp#2105 Reviewed-By: Rod Vagg <[email protected]> * doc: update catalina xcode clt download link PR-URL: nodejs/node-gyp#2133 Reviewed-By: Rod Vagg <[email protected]> Reviewed-By: Christian Clauss <[email protected]> * v7.0.0: bump version and update changelog PR-URL: nodejs/node-gyp#2124 * deps: increase "engines" to "node" : ">= 10.12.0" Makes npm warn users if they are using an unsupported Node version. Refs: nodejs/node-gyp#2123 PR-URL: nodejs/node-gyp#2153 Reviewed-By: Jiawen Geng <[email protected]> Reviewed-By: Rod Vagg <[email protected]> * doc: silence curl for macOS Catalina acid test PR-URL: nodejs/node-gyp#2150 Reviewed-By: Rod Vagg <[email protected]> * docs: note that node-gyp@7 should solve Catalina CLT issues PR-URL: nodejs/node-gyp#2156 Reviewed-By: Christian Clauss <[email protected]> * build: support apple silicon (arm64 darwin) builds Reviewed-By: Rod Vagg <[email protected]> Reviewed-By: Jiawen Geng <[email protected]> PR-URL: nodejs/node-gyp#2165 * gyp: update gyp to 0.4.0 Reviewed-By: Rod Vagg <[email protected]> Reviewed-By: Jiawen Geng <[email protected]> PR-URL: nodejs/node-gyp#2165 * build: add update-gyp script Co-authored-by: Christian Clauss <[email protected]> Reviewed-By: Jiawen Geng <[email protected]> Reviewed-By: Christian Clauss <[email protected]> PR-URL: nodejs/node-gyp#2167 * v7.1.0: bump version and update changelog * doc: drop the --production flag for installing windows-build-tools This isn't needed, and was probably copy-pasted from windows-build-tools' README.md, which has since been changed to drop the `--production` flag from the install instructions. PR-URL: nodejs/node-gyp#2206 Reviewed-By: Richard Lau <[email protected]> Reviewed-By: Christian Clauss <[email protected]> Reviewed-By: Rod Vagg <[email protected]> * ci: switch to GitHub Actions Co-authored-by: Christian Clauss <[email protected]> Co-authored-by: Matias Lopez <[email protected]> PR-URL: nodejs/node-gyp#2210 Closes: #2127 Closes: #2209 * doc: replace status badges with new Actions badge PR-URL: nodejs/node-gyp#2218 Reviewed-By: Christian Clauss <[email protected]> Reviewed-By: Matias Lopez <[email protected]> Reviewed-By: Richard Lau <[email protected]> * test: GitHub Actions: Test on Python 3.9 From python: [3.6, 3.7, 3.8] --> python: [3.6, 3.8, 3.9] because if things work on Python 3.6 and 3.8 then they should work on 3.7. https://www.python.org/downloads/release/python-390/ PR-URL: nodejs/node-gyp#2230 Reviewed-By: Shelley Vohr <[email protected]> Reviewed-By: Richard Lau <[email protected]> * lib: better log message when ps fails PR-URL: nodejs/node-gyp#2229 Reviewed-By: Bartosz Sosnowski <[email protected]> Reviewed-By: Rod Vagg <[email protected]> * gyp: update gyp to 0.6.1 Closes: nodejs/node-gyp#2236 PR-URL: nodejs/node-gyp#2238 Reviewed-By: Christian Clauss <[email protected]> Reviewed-By: Richard Lau <[email protected]> Reviewed-By: Myles Borins <[email protected]> * deps: update deps to match npm@7 PR-URL: nodejs/node-gyp#2240 Reviewed-By: Richard Lau <[email protected]> * v7.1.1: bump version and update changelog PR-URL: nodejs/node-gyp#2239 Reviewed-By: Christian Clauss <[email protected]> * doc: add cmd to reset `xcode-select` to initial state PR-URL: nodejs/node-gyp#2235 Reviewed-By: Christian Clauss <[email protected]> * gyp: update gyp to 0.6.2 Refs: https://github.com/nodejs/gyp-next/releases/tag/v0.6.2 PR-URL: nodejs/node-gyp#2241 Reviewed-By: Rod Vagg <[email protected]> * v7.1.1: bump version and update changelog PR-URL: nodejs/node-gyp#2242 * doc: add missing `sudo` to Catalina doc PR-URL: nodejs/node-gyp#2244 Reviewed-By: Rod Vagg <[email protected]> * ci: migrate deprecated grammar (#2285) PR-URL: nodejs/node-gyp#2285 Reviewed-By: Richard Lau <[email protected]> * doc: updated README.md to copy easily (#2281) PR-URL: nodejs/node-gyp#2281 Reviewed-By: Jiawen Geng <[email protected]> * gyp: update gyp to v0.7.0 (#2284) PR-URL: nodejs/node-gyp#2284 Reviewed-By: Richard Lau <[email protected]> Reviewed-By: Christian Clauss <[email protected]> * doc: update macOS_Catalina.md (#2293) PR-URL: nodejs/node-gyp#2293 Reviewed-By: Jiawen Geng <[email protected]> * gyp: update gyp to v0.8.0 (#2318) PR-URL: nodejs/node-gyp#2318 Reviewed-By: Rod Vagg <[email protected]> Reviewed-By: Jiawen Geng <[email protected]> * lib: avoid changing process.config (#2322) PR-URL: nodejs/node-gyp#2322 Refs: nodejs/node#36902 Reviewed-By: Richard Lau <[email protected]> Reviewed-By: Rod Vagg <[email protected]> * gyp: remove support for Python 2 (#2300) PR-URL: nodejs/node-gyp#2300 Reviewed-By: Jiawen Geng <[email protected]> * lib: migrate requests to fetch (#2220) PR-URL: nodejs/node-gyp#2220 Reviewed-By: Jiawen Geng <[email protected]> * lib: drop Python 2 support in find-python.js (#2333) Co-authored-by: Christian Clauss <[email protected]> PR-URL: nodejs/node-gyp#2333 Reviewed-By: Christian Clauss <[email protected]> Reviewed-By: Jiawen Geng <[email protected]> * ci: update actions/setup-node to v2 (#2302) PR-URL: nodejs/node-gyp#2302 Reviewed-By: Christian Clauss <[email protected]> Reviewed-By: Richard Lau <[email protected]> Reviewed-By: Rod Vagg <[email protected]> Reviewed-By: Jiawen Geng <[email protected]> * doc: add downloads badge (#2352) PR-URL: nodejs/node-gyp#2352 Reviewed-By: Christian Clauss <[email protected]> Reviewed-By: Richard Lau <[email protected]> * deps: sync mutual dependencies with npm Sync with npm 7.7.0 PR-URL: nodejs/node-gyp#2348 Reviewed-By: Rod Vagg <[email protected]> Reviewed-By: Jiawen Geng <[email protected]> * gyp: Improve our flake8 linting tests PR-URL: nodejs/node-gyp#2356 Reviewed-By: Jiawen Geng <[email protected]> * gyp: update gyp to v0.8.1 (#2355) PR-URL: nodejs/node-gyp#2355 Reviewed-By: Jiawen Geng <[email protected]> Reviewed-By: Christian Clauss <[email protected]> * v8.0.0: bump version and update changelog * doc: fix v8.0.0 release date PR-URL: nodejs/node-gyp#2346 * meta: add `release-please-action` for automated releases (#2395) Co-authored-by: gengjiawen <[email protected]> * lib: fail gracefully if we can't find the username (#2375) * lib: log as yes/no whether build dir was created (#2370) This bit of logging apparently expected to be given a boolean, but was receiving either a path or undefined based on the result of fs.mkdir. Now it prints either "Yes" or "No", rather than printing either a path or "undefined", respectively. * doc: Update README.md Visual Studio Community page polski to auto (#2371) changed URL of Visual Studio Community from a default polski URL to the one without the lenguage code * doc: remove redundant version info (#2403) * feat(gyp): update gyp to v0.9.1 (#2402) * chore: release 8.1.0 (#2418) Co-authored-by: github-actions[bot] <41898282+github-actions[bot]@users.noreply.github.com> * lib: deep-copy process.config during configure (#2368) * fix: change default gyp update message (#2420) * fix: add error arg back into catch block for older Node.js users * chore: fix typos discovered by codespell (#2442) * Initial Commit * doc(wiki): Created "binding.gyp" files out in the wild (markdown) * doc(wiki): Updated "binding.gyp" files out in the wild (markdown) * doc(wiki): Updated "binding.gyp" files out in the wild (markdown) * doc(wiki): fixed node-serialport link * doc(wiki): Updated "binding.gyp" files out in the wild (markdown) * doc(wiki): Updated "binding.gyp" files out in the wild (markdown) * doc(wiki): add bcrypt * doc(wiki): change bcrypt url to binding.gyp file * doc(wiki): Updated "binding.gyp" files out in the wild (markdown) * doc(wiki): add one more example * doc(wiki): Add a link to the node-midi binding.gyp file. * doc(wiki): Updated "binding.gyp" files out in the wild (markdown) * doc(wiki): Adds node-inotify and v8-profiler * doc(wiki): Updated "binding.gyp" files out in the wild (markdown) * doc(wiki): Adds node-fann * doc(wiki): Add node-canvas * doc(wiki): Updated "binding.gyp" files out in the wild (markdown) * doc(wiki): Created Linking to OpenSSL (markdown) * doc(wiki): Updated Home (markdown) * doc(wiki): Updated "binding.gyp" files out in the wild (markdown) * doc(wiki): Updated Linking to OpenSSL (markdown) * doc(wiki): added levelup * doc(wiki): Updated "binding.gyp" files out in the wild (markdown) * doc(wiki): Updated "binding.gyp" files out in the wild (markdown) * doc(wiki): Updated "binding.gyp" files out in the wild (markdown) * doc(wiki): Created Updating npm's bundled node gyp (markdown) * doc(wiki): Updated Updating npm's bundled node gyp (markdown) * doc(wiki): Updated Updating npm's bundled node gyp (markdown) * doc(wiki): Updated Updating npm's bundled node gyp (markdown) * doc(wiki): Updated Updating npm's bundled node gyp (markdown) * doc(wiki): Created Visual Studio 2010 Setup (markdown) * doc(wiki): Updated Home (markdown) * doc(wiki): Created Common issues (markdown) * doc(wiki): Updated Home (markdown) * doc(wiki): Add helpful information * doc(wiki): Created Error: "pre" versions of node cannot be installed (markdown) * doc(wiki): Updated Error: "pre" versions of node cannot be installed (markdown) * doc(wiki): Updated Home (markdown) * doc(wiki): fix link to gyp file used to build libsqlite3 * doc(wiki): Updated "binding.gyp" files out in the wild (markdown) * doc(wiki): Bumping Python version from 2.3 to 2.7 as per the node-gyp readme * doc(wiki): Add node-openvg-canvas and node-openvg. * doc(wiki): Updated Home (markdown) * doc(wiki): Adding link to node-cryptopp's gyp file * doc(wiki): Updated Linking to OpenSSL (markdown) * doc(wiki): add topcube, node-osmium, and node-osrm * doc(wiki): Created use of undeclared identifier 'TypedArray' (markdown) * doc(wiki): Created Visual studio 2012 setup (markdown) * doc(wiki): Destroyed Visual studio 2012 setup (markdown) * doc(wiki): Correcting the link to node-osmium * doc(wiki): Updated "binding.gyp" files out in the wild (markdown) * doc(wiki): Fix link to node-zipfile * doc(wiki): Explicit link to Visual C++ 2010 Express * doc(wiki): Added tip about resolving frustrating LNK1181 error * doc(wiki): Updated node-levelup to node-leveldown (broken links) * doc(wiki): Added details for properly fixing * doc(wiki): Updated "binding.gyp" files out in the wild (markdown) * doc(wiki): Added nk-mysql (nodamysql) * doc(wiki): Added nk-xrm-installer .gyp references, including .py scripts for providing complete reference to examples of fetching source via http, extracting, and moving files (as opposed to copying) * doc(wiki): Note: VS2010 seems to be no longer available! VS2013 or nothing! * doc(wiki): node-sass in the wild * doc(wiki): Clarification + direct link to VS2010 * doc(wiki): Updated Updating npm's bundled node gyp (markdown) * doc(wiki): Updated "binding.gyp" files out in the wild (markdown) * doc(wiki): if ouns that the -h did not help. I founs on github that there was support for visual studio 2015, while i couldn't install node-red beacuse it kept telling me the key 2015 was missing. looking in he gyp python code i found the local file was bot up t dat with the github repo. updating took several efforts before i tried to drop the -g option. * doc(wiki): sorry, forgot to mention a specific windows version. * doc(wiki): Updated "binding.gyp" files out in the wild (markdown) * doc(wiki): Added Ghostscript4JS * doc(wiki): I highly missing it in common issue as every windows biggner face that issue * doc(wiki): ADDED: Node.js binding to OpenCV * doc(wiki): Updated "binding.gyp" files out in the wild (markdown) * doc(wiki): Adding the sharp library to the list * doc(wiki): node-srs was a 404 * doc(wiki): C++ build tools version upgraded * doc(wiki): Destroyed Visual Studio 2010 Setup (markdown) * doc(wiki): Updated Home (markdown) * doc(wiki): Lower case L * doc(wiki): Updated "binding.gyp" files out in the wild (markdown) * doc(wiki): Make changes discussed in nodejs/node-gyp#2416 * doc(wiki): Drop in favor of * doc(wiki): Different commands for Windows npm v6 vs. v7 * doc(wiki): Improve Unix instructions * doc(wiki): Updated Updating npm's bundled node gyp (markdown) * doc(wiki): If permissions error, please try and then the command. * doc(wiki): move wiki docs into doc/ * doc(wiki): link to docs/ from README * doc(wiki): safer doc names, remove unnecessary TypedArray doc * ci: GitHub Actions Test on node: [12.x, 14.x, 16.x] (#2439) * Add title to node-gyp version document (#2452) * Add title to node-gyp version document * Update Updating-npm-bundled-node-gyp.md * fix: doc how to update node-gyp independently from npm * fix: missing spaces * ISSUE_TEMPLATE.md: Instructions for old versions (#2470) * ISSUE_TEMPLATE.md: Instructions for old versions Also, add a caution about `node sass` being deprecated. * Update .github/ISSUE_TEMPLATE.md Co-authored-by: Rod Vagg <[email protected]> Co-authored-by: Rod Vagg <[email protected]> * chore(deps): bump tar from 6.1.0 to 6.1.2 (#2474) Addresses GHSA-3jfq-g458-7qm9 and GHSA-r628-mhmh-qjhw * doc: correct link to "binding.gyp files out in the wild" (#2483) correct link to "binding.gyp files out in the wild" * feat(gyp): update gyp to v0.9.6 (#2481) * chore: release 8.2.0 * chore: refactor the creation of config.gypi file * test: Python 3.10 was release on Oct. 4th (#2504) * chore(deps): bump make-fetch-happen from 8.0.14 to 9.1.0 The breaking change in this module was a cache parameter that `node-gyp` is not using, so this module is not affected. * feat(gyp): update gyp to v0.10.0 (#2521) * chore: release 8.3.0 * feat: support vs2022 (#2533) * feat: build with config.gypi from node headers * chore: release 8.4.0 * docs: fix typo in powershell node-gyp update * deps: [email protected] * fix: windows command missing space (#2553) * chore: release 8.4.1 * chore: add minimal SECURITY.md (#2560) * doc: Rename and update Common-issues.md --> docs/README.md (#2567) Update the common problems to track with current issues on this repo and shorten the URL to just https://github.com/nodejs/node-gyp/tree/master/docs * docs: title match content (#2574) * docs: Add notes/disclaimers for upgrading the copy of node-gyp that npm uses (#2585) * docs: rephrase explanation of which node-gyp is used by npm (#2587) * doc: Update Python versions (#2571) * Add Python 3.10 * Drop Python 3.6 which [EOLs on 23 Dec. 2021](https://devguide.python.org/#status-of-python-branches) * macOS: clarify `Xcode Command Line Tools` standalone vs. from full Xcode * Window: Use the same URL as https://github.com/nodejs/node/blob/master/BUILDING.md#windows * deps!: increase "engines" to "node" : "^12.22 || ^14.13 || >=16" (#2601) Makes npm warn users if they are using an unsupported Node version. * deps: [email protected] The breaking change was dropping node10 support, which node-gyp has already done. * fix: update make-fetch-happen to a minimum of 10.0.3 * added node-heapdump binding.gyp * fix: _ in npm_config_ env variables * lib: add lib.target as path for searching libnode on z/OS * chore: release 9.0.0 * test: Upgrade GitHub Actions (#2623) * doc: update docs/README.md with latest version number * fix: typo on readme * fix: new ca & server certs, bundle in .js file and unpack for testing bundling in certs.js rather than including the raw files should avoid some false positives that low-quality security scanners keep on complaining about. * fix: extend tap timeout length to allow for slow CI * Add Python symlink to path (for non-Windows OSes only) (#2362) * lib: create a Python symlink and add it to PATH Helps to ensure a version of Python validated by lib/find-python.js is used to run various Python scripts generated by gyp. Known to affect gyp-mac-tool, probably affects gyp-flock-tool as well. These Python scripts (such as `gyp-mac-tool`) are invoked directly, via the generated Makefile, so their shebang lines determine which Python binary is used to run them. The shebang lines of these scripts are all `#!/usr/bin/env python3`, so the first `python3` on the user's PATH will be used. By adding a symlink to the Python binary validated by find-python.js, and putting this symlink first on the PATH, we can ensure we use a compatible version of Python to run these scripts. (Only on Unix/Unix-like OSes. Symlinks are tricky on Windows, and Python isn't used at build-time anyhow on Windows, so this intervention isn't useful or necessary on Windows. A similar technique for Windows, no symlinks required, would be to make batch scripts which execute the target binary, much like what Node does for its bundled copy of npm on Windows.) * test: update mocked graceful-fs for configure test Add missing functions "unlink()" and "symlink()" to mocked module. * lib: log any errors when creating Python symlink Warn users about errors, but continue on in case the user does happen to have new enough Python on their PATH. (The symlinks are only meant to fix an issue in a corner case, where the user told `node-gyp` where new enough Python is, but it's not the first `python3` on their PATH. We should not introduce a new potential failure mode to all users when fixing this bug. So no hard errors during the symlink process.) * lib: improve error formatting for Python symlink Logging the entire error object shows the stack twice, and all the other information is contained in the stack. It also messes with the order of what is logged. Rather than logging a bunch of redundant information in a messy way, we can log only the stack. Logging it in a separate log.warn() also gets rid of an extra space character at the beginning of the line. * lib: restore err.errno to logs for symlink errors This info (err.errno) is the only piece of information in the error object that is not redundant to err.stack. * lib: use log.verbose, not log.warn These messages aren't important enough to be `log.warn`s. Log as verbose only; they will also appear in full error output. * Clarify wording to redirect to macOS_Catalina.md (#2588) * build: update due to rename of primary branch * Migrate macOS acid test from master to main (#2686) Follow-on to #2495 * feat: Update function getSDK() to support Windows 11 SDK (#2565) * test: Upgrade GitHub Actions (#2701) * test: Upgrade GitHub Actions * node: 18x --> 18.x * test: Try msvs-version: [2016, 2019, 2022] (#2700) * test: Try msvs-version: [2016, 2019, 2022] * main, not master * Don't npm audit fix --force * fix: re-label (#2689) * chore: release 9.1.0 * lib: enable support for zoslib on z/OS (#2600) Check if zos-base.h is in the directory identified by environment variable ZOSLIB_INCLUDES if set; otherwise search for it from a set of candidates under nodeRootDir. Then pass it as -Dzoslib_include_dir=<path-found> to gyp_main.py for use in common.gypi to set 'includes_dir' when compiling addons. Co-authored-by: Gaby Baghdadi <[email protected]> Co-authored-by: Gaby Baghdadi <[email protected]> * chore: update dependency - [email protected] (#2707) No functional changes, just dropping old node versions from engines, linting, and fixing CI. * fix: node.js debugger adds stderr (but exit code is 0) -> shouldn't throw (#2719) * fix: node.js debugger adds stderr (but exit code is 0) -> shouldn't throw * input.py: subprocess.Popen() -> subprocess.run() * feat(gyp): update gyp to v0.13.0 * feat: Add proper support for IBM i Python 3.9 on IBM i now properly returns "os400" for sys.platform instead of claiming to be AIX as it did previously. While the IBM i PASE environment is compatible with AIX, it is a subset and has numerous differences which makes it beneficial to distinguish, however this means that it now needs explicit support here. * Adding tarfile member sanitization to extractall() (#2741) Co-authored-by: TrellixVulnTeam <[email protected]> * chore: release 9.2.0 (#2735) Co-authored-by: github-actions[bot] <41898282+github-actions[bot]@users.noreply.github.com> * feat: support IBM Open XL C/C++ on z/OS (#2743) * feat: remove support for VS2015 in Node.js >=19 (#2746) * feat(gyp): update gyp to v0.14.0 (#2749) * chore: release 9.3.0 * Add Python 3.11 to the testing https://docs.python.org/3/whatsnew/3.11.html * ci: update python test matrix (#2774) * ci: drop python 3.6 from test matrix * Update .github/workflows/tests.yml Co-authored-by: Christian Clauss <[email protected]> Co-authored-by: Christian Clauss <[email protected]> * fix: increase node 12 support to ^12.13 (#2771) * chore: release 9.3.1 * feat: Upgrade Python linting from flake8 to ruff (#2815) [Ruff](https://beta.ruff.rs/) supports [over 500 lint rules](https://beta.ruff.rs/docs/rules) including bandit, isort, pylint, pyupgrade, and flake8 plus its plugins and is written in Rust for speed. This GitHub Action will provide contributors with intuitive GitHub Annotations. ![image](https://user-images.githubusercontent.com/3709715/223758136-afc386d2-70aa-4eff-953a-2c2d82ceea23.png) The `Required` in the checks below should be: 1. Removed from `flake8-annotation` and added to `ruff-annotation` which replaces it. 2. Removed from `isort` and added to `ruff` which replaces it. * Improved advise on repacing node-sass with sass (#2758) * Improved advise on repacing node-sass with sass * Update README.md * chore: get update-gyp.py to work with Python >= v3.5 (#2826) * chore: get update-gyp.py to work with Python v3.9 * Ruff ignore rule PLC1901 --------- Co-authored-by: Christian Clauss <[email protected]> * doc: Update README.md (#2822) Co-authored-by: Christian Clauss <[email protected]> * update make-fetch-happen to 11.0.3 (#2796) http-cache-semantics 4.1.0 is vulnerable https://www.cve.org/CVERecord?id=CVE-2022-25881 * docs: docs/README.md add advise about deprecated node-sass (#2828) * feat: add support for native windows arm64 build tools Visual Studio 2022 17.4 adds a native C++ compiler for Windows on ARM. This allows arm64 devices to leverage native build tools, leading to a 35% (or more) speed increase. https://devblogs.microsoft.com/visualstudio/arm64-visual-studio-is-officially-here/ Signed-off-by: Dennis Ameling <[email protected]> * fix: extract tarball to temp directory on Windows (#2846) * fix: check for errors while extracting downloaded tarball Signed-off-by: David Sanders <[email protected]> * test: parallel installs Signed-off-by: David Sanders <[email protected]> * fix: extract tarball to temp directory on Windows Signed-off-by: David Sanders <[email protected]> --------- Signed-off-by: David Sanders <[email protected]> * Migration from tap to mocha (#2851) * migrate from tap to mocha After make-fetch-happen update GitHub Actions started failing. Migrating from tap to mocha testing framework for GitHub Action stability. * write custom test reporter for more verbose output Implemented a simple custom mocha test reporter to replace the default one. Made test report more developer friendly. * fix: log statement is for devDir not nodedir (#2840) Signed-off-by: David Sanders <[email protected]> * win,install: only download target_arch node.lib (#2857) Instead of downloading node.lib for all architectures, just download the one that will be needed. Install.js changed to enable downloading just node.lib for node versions that already have tarball downloaded and extracted. Not fetching lib now fails the installation. Increased installVersion because of the changes. Refs: nodejs/node-gyp#2847 * test: remove deprecated Node.js and Python (#2868) * test: remove deprecated node.js and python Removed Node.js v14.x and Python v3.7. Also added Node.js v20.x. * Update .github/workflows/tests.yml Co-authored-by: Christian Clauss <[email protected]> --------- Co-authored-by: Christian Clauss <[email protected]> * chore: release 9.4.0 * Sync deps and engines with npm (#2770) * feat!: update `engines.node` to `^14.17.0 || ^16.13.0 || >=18.0.0` * deps: nopt@^7.0.0 * feat: replace npmlog with proc-log * deps: [email protected] and fix linting errors * deps: [email protected] - this also promiisifies the build command * deps: [email protected] * feat: drop rimraf dependency * fix: use fs/promises in favor of fs.promises * lib: find python checks order changed on windows (#2872) These changes favor py launcher over other checks excluding command line or npm configuration and environment variable checks. Also, updated supported python versions list. Fixes: nodejs/node-gyp#2871 * Fix reading msvs version on Windows (#2644) * fix: fix reading msvs version on windows * docs: Update windows installation instructions in README.md (#2882) * Update windows installation instructions in README.md * Fix Python lint error by using an f-string (#2886) * test: increase mocha timeout (#2887) * fix: create Python symlink only during builds, and clean it up after (#2721) * fix: create Python symlink only during builds, and clean it up after Previously in b9ddcd5 this was created during configuration, and the symlink persisted indefinitely. This causes problems with many tools that do not expect a codebase to include symlinks to external absolute paths. This PR largely reverts that commit, and instead writes the path to link to into the config, and then creates the symlink only temporarily during the build process, always deleting it afterwards. * assert install_path == self.output, f"{install_path} != {self.output}" --------- Co-authored-by: Christian Clauss <[email protected]> * docs: README.md Do not hardcode the supported versions of Python (#2880) * Fix incorrect Xcode casing in README (#2896) * test: update expired certs (#2908) * doc: Add note about Python symlinks (PR 2362) to CHANGELOG.md for 9.1.0 (#2783) The PR for this change was merged without a prefixed name, such as "lib:" or "fix:". That means release-please didn't include it in the changelog for v9.1.0. This change did end up affecting users, though. (See issue 2713 and PR 2721). Therefore, I believe it should be noted in the CHANGELOG.md, so users can better understand the behavior they are seeing. * Python lint: ruff --format is now --output-format Fixes the failing `ruff` linting in GitHub Actions. * chore: GitHub Workflows security hardening (#2740) * build: harden tests.yml permissions Signed-off-by: Alex <[email protected]> * build: harden release-please.yml permissions Signed-off-by: Alex <[email protected]> * build: harden visual-studio.yml permissions Signed-off-by: Alex <[email protected]> * Update release-please.yml --------- Signed-off-by: Alex <[email protected]> * chore: empty commit to add changelog entries from #2770 feat!: update engines.node to ^14.17.0 || ^16.13.0 || >=18.0.0 deps: nopt@^7.0.0 feat: replace npmlog with proc-log deps: [email protected] and fix linting errors deps: [email protected] fix: promisify build command deps: [email protected] feat: drop rimraf dependency fix: use fs/promises in favor of fs.promises * docs: update applicable GitHub links from master to main (#2843) Signed-off-by: David Sanders <[email protected]> * feat(gyp): update gyp to v0.16.1 (#2923) * feat(gyp): update gyp to v0.15.1 * Add Python 3.12 to tests * Try to fix CI * Try specifying msvs-version * Modify the visual-studio matrix * Fix pythonLocation var * Fix Python tests * Get path * polish * feat(gyp): update gyp to v0.16.0 * feat(gyp): update gyp to v0.16.1 * CI: Don't install Python 'packaging' module (vendored in 'gyp-next' now) * Apply suggestions from code review * Upgrade to actions/checkout@v4 --------- Co-authored-by: Raymond Zhao <[email protected]> Co-authored-by: Christian Clauss <[email protected]> * chore: add check engines script to CI (#2922) * deps: [email protected] (#2926) * deps: [email protected] (#2927) * deps: [email protected] (#2928) * feat!: drop node 14 support (#2929) BREAKING CHANGE: `node-gyp` now supports node `^16.14.0 || >=18.0.0` * feat: convert all internal functions to async/await BREAKING CHANGE: All internal functions have been coverted to return promises and no longer accept callbacks. This is not a breaking change for users but may be breaking to consumers of `node-gyp` if you are requiring internal functions directly. * feat: convert internal classes from util.inherits to classes BREAKING CHANGE: the `Gyp` class exported is now created using ECMAScript classes and therefore might have small differences to classes that were previously created with `util.inherits`. * chore: misc testing fixes (#2930) * chore: misc test fixes * Sort test runs by os first * Use cross-env for test env var * Try sorting matrix params * Make FAST_TEST the default and rename to FULL_TEST * Separate helper functions to not need to export test obj in files * feat!: use .npmignore file to limit which files are published (#2921) * feat!: use package.json files to limit which files are published Fixes: #2372 * Use npmignore instead of package.json#files * Add update-gyp.py to npmignore * Add install to pack test * Use output var for pack dir * Move existing .gitignore entries to .npmignore * Sort git and npm ignores * Update and cleanup workflows * chore: run tests after release please PR * chore: release 10.0.0 (#2920) Co-authored-by: github-actions[bot] <41898282+github-actions[bot]@users.noreply.github.com> * chore: add parallel test logging * chore: lint fixes * chore: use platform specific timeouts in tests * fix: use local `util` for `findAccessibleSync()` The `findAccessibleSync()` function is in the local `util` module instead of Node.js' builtin `util` module. * chore: release 10.0.1 * Fix wrong changes that were on repo before merge * fix: output path generation for iOS --------- Signed-off-by: Dennis Ameling <[email protected]> Signed-off-by: David Sanders <[email protected]> Signed-off-by: Alex <[email protected]> Co-authored-by: Ujjwal Sharma <[email protected]> Co-authored-by: Rod Vagg <[email protected]> Co-authored-by: Dario Vladovic <[email protected]> Co-authored-by: DeeDeeG <[email protected]> Co-authored-by: Chia Wei Ong <[email protected]> Co-authored-by: Samuel Attard <[email protected]> Co-authored-by: Christian Clauss <[email protected]> Co-authored-by: Shelley Vohr <[email protected]> Co-authored-by: Matias Lopez <[email protected]> Co-authored-by: Martin Midtgaard <[email protected]> Co-authored-by: Valera Rozuvan <[email protected]> Co-authored-by: Myles Borins <[email protected]> Co-authored-by: Karl Horky <[email protected]> Co-authored-by: Jiawen Geng <[email protected]> Co-authored-by: மனோஜ்குமார் பழனிச்சாமி <[email protected]> Co-authored-by: iMrLopez <[email protected]> Co-authored-by: Michaël Zasso <[email protected]> Co-authored-by: Matias Lopez <[email protected]> Co-authored-by: Sora Morimoto <[email protected]> Co-authored-by: gengjiawen <[email protected]> Co-authored-by: Gustavo de León <[email protected]> Co-authored-by: github-actions[bot] <41898282+github-actions[bot]@users.noreply.github.com> Co-authored-by: Livia Rett <[email protected]> Co-authored-by: TooTallNate <[email protected]> Co-authored-by: milani <[email protected]> Co-authored-by: Psychless <[email protected]> Co-authored-by: mixu <[email protected]> Co-authored-by: shtylman <[email protected]> Co-authored-by: lloyd <[email protected]> Co-authored-by: bolgovr <[email protected]> Co-authored-by: justinlatimer <[email protected]> Co-authored-by: kkaefer <[email protected]> Co-authored-by: c4milo <[email protected]> Co-authored-by: lperrin <[email protected]> Co-authored-by: oransel <[email protected]> Co-authored-by: voodootikigod <[email protected]> Co-authored-by: ehansin <[email protected]> Co-authored-by: xverges <[email protected]> Co-authored-by: Niggler <[email protected]> Co-authored-by: felquis <[email protected]> Co-authored-by: Dane Springmeyer <[email protected]> Co-authored-by: Alex Treppass <[email protected]> Co-authored-by: Luis Reis <[email protected]> Co-authored-by: lilo003 <[email protected]> Co-authored-by: Syrian watermelon <[email protected]> Co-authored-by: Evan Su <[email protected]> Co-authored-by: fov42550564 <[email protected]> Co-authored-by: tcbeutler <[email protected]> Co-authored-by: raztus <[email protected]> Co-authored-by: vweevers <[email protected]> Co-authored-by: Andreas Brekken <[email protected]> Co-authored-by: ralphtheninja <[email protected]> Co-authored-by: Zeke Sonxx <[email protected]> Co-authored-by: Дмитрий Цветцих <[email protected]> Co-authored-by: Richard Winters <[email protected]> Co-authored-by: Mark Jeghers <[email protected]> Co-authored-by: Marcin Cieślak <[email protected]> Co-authored-by: Dieter De Paepe <[email protected]> Co-authored-by: Operations Research Engineering Software+ <[email protected]> Co-authored-by: Flandre Scarlet <[email protected]> Co-authored-by: peter--bolier--zero <[email protected]> Co-authored-by: Nick Desaulniers <[email protected]> Co-authored-by: Nicola Del Gobbo <[email protected]> Co-authored-by: Abdul Hameed <[email protected]> Co-authored-by: xdf <[email protected]> Co-authored-by: Matt Hirsch <[email protected]> Co-authored-by: João Reis <[email protected]> Co-authored-by: Bert Verhelst <[email protected]> Co-authored-by: Mayank <[email protected]> Co-authored-by: nineninesevenfour <[email protected]> Co-authored-by: Cheng Zhao <[email protected]> Co-authored-by: Gar <[email protected]> Co-authored-by: csett86 <[email protected]> Co-authored-by: HeatonZ <[email protected]> Co-authored-by: Rich Trott <[email protected]> Co-authored-by: owl from hogvarts <[email protected]> Co-authored-by: nlf <[email protected]> Co-authored-by: Mohamed-Elzohary <[email protected]> Co-authored-by: alexcfyung <[email protected]> Co-authored-by: Doni Rubiagatra <[email protected]> Co-authored-by: Nick Wang <[email protected]> Co-authored-by: Michael Dawson <[email protected]> Co-authored-by: hubbergit <[email protected]> Co-authored-by: Gaby Baghdadi <[email protected]> Co-authored-by: Mr. Doge <[email protected]> Co-authored-by: Kevin Adler <[email protected]> Co-authored-by: TrellixVulnTeam <[email protected]> Co-authored-by: Luke Karrys <[email protected]> Co-authored-by: Raymond Zhao <[email protected]> Co-authored-by: Maksim Beliaev <[email protected]> Co-authored-by: ravindraP20 <[email protected]> Co-authored-by: Dennis Ameling <[email protected]> Co-authored-by: David Sanders <[email protected]> Co-authored-by: Stefan Stojanovic <[email protected]> Co-authored-by: James Cook <[email protected]> Co-authored-by: Rareș <[email protected]> Co-authored-by: Tim Perry <[email protected]> Co-authored-by: Iulian Onofrei <[email protected]> Co-authored-by: Alex <[email protected]> Co-authored-by: Richard Lau <[email protected]> Co-authored-by: Denis Bogomolov <[email protected]>

imatlopez force-pushed the fetch-happen branch from 6d4db65 to 257a56e Compare September 13, 2020 18:22

rvagg reviewed Sep 14, 2020

View reviewed changes

imatlopez changed the title ~~feat: migrate requests to make-fetch-happen~~ feat: migrate requests to fetch Sep 14, 2020

imatlopez force-pushed the fetch-happen branch from 65093f1 to 4bd289a Compare September 14, 2020 15:13

rvagg force-pushed the master branch from 1e2e94d to 3e7f8cc Compare October 14, 2020 23:25

rvagg mentioned this pull request Oct 15, 2020

v7.1.1 proposal #2239

Merged

imatlopez added 7 commits November 3, 2020 07:48

feat: replace install.js internals with make-fetch

6f90faf

Migrate the use of deprecated `requests` to `make-fetch-happen`. Replaces a lot of callbacks with async/await or promises.

test: fix tests in download.js that need promises

bd0334c

Some tests are calling the recently converted promise apis, this converts them to use the new format.

fix: replace pipe listeners with transform stream

c73eb8c

Because listeners were placed on the response body, it caused the data to be consumed prematurely, hence using a stream Transform, no data is consumed.

feat: remove proxy support code

36467d2

make-fetch-happen already has proxy support

feat: install is completely promisified

4357d61

Due to a posibility that callbacks are called twice on error, the install function is now internally completely promisified.

fix: reduce one level promise

b2bfa22

Windows seems to be hanging, hopefully this reduces the number of promises in the chain.

test: use proper teardown

95a7305

Tests are were not waiting for their servers to close.

imatlopez force-pushed the fetch-happen branch from c15a776 to 95a7305 Compare November 3, 2020 12:48

imatlopez marked this pull request as ready for review January 18, 2021 17:28

imatlopez requested a review from rvagg January 18, 2021 17:29

imatlopez added 2 commits January 24, 2021 09:18

fix: skip error return on then callback

cd6d856

Otherwise any succesful return would make the program think it failed

Merge branch 'master' into fetch-happen

70fb316

imatlopez force-pushed the fetch-happen branch from c3f2cae to 70fb316 Compare January 24, 2021 14:19

daveta mentioned this pull request Jan 27, 2021

[Orchestrator-BugBash] warnings when installing plugin microsoft/botframework-cli#1054

Closed

cclauss mentioned this pull request Feb 15, 2021

warning electron-rebuild > node-gyp > [email protected]: request has been deprecated, see https://github.com/request/request/issues/3142 #2328

Closed

cclauss requested review from AshCripps and BridgeAR February 20, 2021 10:04

cclauss requested review from cjihrig, gengjiawen, richardlau, targos and Trott February 20, 2021 10:04

gengjiawen approved these changes Mar 5, 2021

View reviewed changes

indolent-developer reviewed Mar 9, 2021

View reviewed changes

gengjiawen added the semver-major label Mar 16, 2021

gengjiawen changed the title ~~feat: migrate requests to fetch~~ lib: migrate requests to fetch Mar 18, 2021

gengjiawen merged commit e81602e into nodejs:master Mar 18, 2021

rvagg mentioned this pull request Mar 18, 2021

Release proposal: v8.0.0 #2346

Merged

donacarr mentioned this pull request Mar 25, 2021

SHA-1 Weak Authentication Algorithm vulnerability in dependency "request" RuntimeTools/appmetrics#647

Open

This was referenced Mar 31, 2021

feat: promisify clean #2357

Closed

feat: promisify build #2358

Closed

deps: [email protected] npm/cli#3030

Closed

imatlopez mentioned this pull request Apr 8, 2021

deps: [email protected] npm/run-script#26

Closed

luckman212 mentioned this pull request May 3, 2021

[BUG] node-gyp@v7 still has dependency on deprecated requests module npm/cli#3183

Closed

DeeDeeG mentioned this pull request Jul 2, 2021

Update wrong catch block #2441

Merged

4 tasks

altro3 mentioned this pull request Jul 8, 2021

[BUG] Upgrade node-gyp to ^8.1.2 version npm/cli#3525

Closed

1 task

stramel mentioned this pull request Jul 26, 2021

[BUG] Migrate to node-gyp@^8 npm/run-script#35

Closed

asamuzaK mentioned this pull request Nov 20, 2021

Update node-gyp mongodb-js/boxednode#23

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

lib: migrate requests to fetch #2220

lib: migrate requests to fetch #2220

imatlopez commented Sep 13, 2020 •

edited

Loading

imatlopez commented Sep 13, 2020 •

edited

Loading

rvagg Sep 14, 2020

rvagg Sep 14, 2020

rvagg commented Sep 14, 2020

oliverjessner commented Jan 22, 2021

imatlopez commented Feb 2, 2021

imatlopez commented Feb 20, 2021

rvagg commented Feb 20, 2021

gengjiawen left a comment

gengjiawen commented Mar 5, 2021

indolent-developer Mar 9, 2021

Trott Mar 9, 2021

indolent-developer Mar 9, 2021

imatlopez Mar 9, 2021

indolent-developer Mar 10, 2021

Trott commented Mar 10, 2021

gengjiawen commented Mar 10, 2021

rvagg commented Mar 10, 2021

gengjiawen commented Mar 18, 2021

lib: migrate requests to fetch #2220

lib: migrate requests to fetch #2220

Conversation

imatlopez commented Sep 13, 2020 • edited Loading

Checklist

Description of change

imatlopez commented Sep 13, 2020 • edited Loading

rvagg Sep 14, 2020

Choose a reason for hiding this comment

rvagg Sep 14, 2020

Choose a reason for hiding this comment

rvagg commented Sep 14, 2020

oliverjessner commented Jan 22, 2021

imatlopez commented Feb 2, 2021

imatlopez commented Feb 20, 2021

rvagg commented Feb 20, 2021

gengjiawen left a comment

Choose a reason for hiding this comment

gengjiawen commented Mar 5, 2021

indolent-developer Mar 9, 2021

Choose a reason for hiding this comment

Trott Mar 9, 2021

Choose a reason for hiding this comment

indolent-developer Mar 9, 2021

Choose a reason for hiding this comment

imatlopez Mar 9, 2021

Choose a reason for hiding this comment

indolent-developer Mar 10, 2021

Choose a reason for hiding this comment

Trott commented Mar 10, 2021

gengjiawen commented Mar 10, 2021

rvagg commented Mar 10, 2021

gengjiawen commented Mar 18, 2021

imatlopez commented Sep 13, 2020 •

edited

Loading

imatlopez commented Sep 13, 2020 •

edited

Loading