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

doc: add fix for msbuild errors on windows #1657

Closed
wants to merge 63 commits into from

Conversation

bertyhell
Copy link
Contributor

Checklist
Description of change

Add an extra step to fix an issue with node-gyp rebuild on windows. After trying 2 days to fix it any other way.

  • tried installing build tools
  • tried reinstalling node-gyp
  • tried env variables for python and msbuild
  • tried installing visual studio 2017
  • tried reinstalling build tools
  • tried removing node and all node_modules and reinstalling everything

The step that I added was a last resort effort before I was going to format my pc to see if that fixes it.

I kept getting error messages like:

  • error MSB4019: The imported project "C:\Program Files (x86)\MSBuild\Microsoft.Cpp\v4.0\V140\Microsoft.Cpp.Default.props" was not found. Confirm
    that the path in the declaration is correct, and that the file exists on disk
  • C:\Program Files (x86)\MSBuild\Microsoft.Cpp\v4.0\V140\Microsoft.Cpp.Platform.targets(57,5): error MSB8020: The build tools for v141 (Platform Toolset = 'v141') cannot be found
  • error MSB4062: The "SetEnv" task could not be loaded from the assembly C:\Program Files (x86)\Microsoft Visual Studio\2017\Community\Common7\IDE\VC\VCTargets\Microsoft.Build.CppTasks.Common.dll
  • [SetupUpdaterImpl]: Stream was closed
  • node-gyp rebuild failed with: Error: Command failed: node-gyp rebuild

isaacs and others added 30 commits June 6, 2017 06:56
Tar version 3 performs better and is more well tested than its
predecessor.  npm will be using this in the near future, so there is no
benefit in shipping a node-gyp that uses the slower and less reliable
fstream-based tar.

This drops support for node 0.x, and thus should be considered a
breaking semver-major change.

PR-URL: nodejs#1212
Reviewed-By: Refael Ackermann <[email protected]>
Reviewed-By: Ben Noordhuis <[email protected]>
Reviewed-By: Gibson Fahnestock <[email protected]>
* dropping support for node < 4
* signal the CI not to test node < 4
If you're providing a path to a header tarball to install, you probably
want it to always be re-installed.

PR-URL: nodejs#1220
Fixes: nodejs#1216
Reviewed-By: Ben Noordhuis <[email protected]>
Reviewed-By: Refael Ackermann <[email protected]>
* test: build simple addon in path with non-ascii characters
* test: add test-charmap.py

PR-URL: nodejs#1203
Reviewed-By: Refael Ackermann <[email protected]>
Enable linking to the platform specific installation instructions

PR-URL: nodejs#1225
Reviewed-By: Refael Ackermann <[email protected]>
Lifted verbatim from
https://github.com/nodejs/node/blob/master/CONTRIBUTING.md
then `s/Node.js/node-gyp/`.

PR-URL: nodejs#1229
Reviewed-By: Anna Henningsen <[email protected]>
Reviewed-By: Gibson Fahnestock <[email protected]>
Give users reporting bugs a clearer idea of the info that will be
helpful when reporting issues.

PR-URL: nodejs#1228
Refs: https://github.com/nodejs/node/tree/master/.github
Reviewed-By: Refael Ackermann <[email protected]>
Reviewed-By: Ben Noordhuis <[email protected]>
Initial work to add z/OS support to node-gyp.


PR-URL: nodejs#1276
Reviewed-By: Refael Ackermann <[email protected]>
Reviewed-By: Ben Noordhuis <[email protected]>
GYP automatically turns variables ending in _dir, _file or _path into
absolute paths but didn't check for empty strings.

It interacted badly with variables inherited through the environment
from npm, the `scripts-prepend-node-path=false` setting in particular
because it is turned into `npm_config_script_prepend_node_path=`.

Fixes: nodejs#1217
PR-URL: nodejs#1267
Reviewed-By: Gibson Fahnestock <[email protected]>
Reviewed-By: Refael Ackermann <[email protected]>
The description erroneously stated that it should point the node binary.
It needs to point to the node source code.

PR-URL: nodejs#1372
Reviewed-By: Joyee Cheung <[email protected]>
Reviewed-By: Richard Lau <[email protected]>
Retry the download+install dance only once after encountering an EACCES.

That only happens when both the devdir (usually: `$HOME/.node-gyp`) and
the current working directory aren't writable.  Users won't often hit
that except through `sudo npm install` because npm drops privileges
before executing node-gyp.

Fixes: nodejs#1383
PR-URL: nodejs#1384
Reviewed-By: Gibson Fahnestock <[email protected]>
Reviewed-By: Richard Lau <[email protected]>
As node-gyp rebuild doesn't seem to need xcodebuild, we don't need to be
printing the error every time GYP is run.

PR-URL: nodejs#1370
Fixes: nodejs#569
Refs: nodejs#1057
Refs: https://chromium-review.googlesource.com/c/492046/
PR-URL: nodejs#1323
Fixes: nodejs#1295
Reviewed-By: Ben Noordhuis <[email protected]>
Reviewed-By: Gibson Fahnestock <[email protected]>
This fixes the regular expression matching in `xcode_emulation`
to also handle version numbers with multiple-digit major versions
which would otherwise break under use of XCode 10

Fixes: nodejs#1454
PR-URL: nodejs#1455
Reviewed-By: Ben Noordhuis <[email protected]>
Node.js on z/OS uses shared dll (libnode.so). When linking native
addons, node-gyp needs to find the corresponding libnode.x during
the link step.

PR-URL: nodejs#1451
Reviewed-By: Ben Noordhuis <[email protected]>
Reviewed-By: Refael Ackermann <[email protected]>
PR-URL: nodejs#1451
Reviewed-By: Ben Noordhuis <[email protected]>
Reviewed-By: Refael Ackermann <[email protected]>
Currently, on non-Windows platforms, it is possible to have
a multi-target native module and specify building just some
of the targets using `node-gyp build my_target`.

On Windows, however, specifying the targets to build requires the `/t:`
or `/target:` flag. Without this change you will get an `MSB1008` error
because MSBuild thinks you are trying to specify multiple solutions.

PR-URL: nodejs#1164
Reviewed-By: Ben Noordhuis <[email protected]>
Header files for deps are in a different location in the Node.js
source tree compared to the release tarballs.

PR-URL: nodejs#1055
Reviewed-By: Ben Noordhuis <[email protected]>
PR-URL: nodejs#1158
Reviewed-By: Ben Noordhuis <[email protected]>
Reviewed-By: Refael Ackermann <[email protected]>
Shrinks node-gyp's size by about 100 kB.

PR-URL: nodejs#1458
Reviewed-By: Richard Lau <[email protected]>
Fixes grammar, removes extra lines and spaces, etc. Also removes a few
references to `node-waf`, which was removed ~6 years ago now. Happy to
add back if people still need that information.

PR-URL: nodejs#1498
Reviewed-By: Vse Mozhet Byt <[email protected]>
Reviewed-By: Richard Lau <[email protected]>
- Removes "module dependencies" comments and things that, IMHO, don't add
too much value. Happy to add back if helps some people when reading
through `node-gyp`.
- DRY up `lib/process-release.js`.
- Removes a bunch of extra blank lines, as well as random spaces.

PR-URL: nodejs#1508
Reviewed-By: Richard Lau <[email protected]>
- Uses `.eslintrc.yaml` for configuration
- `npm run lint` is part of `npm test`

PR-URL: nodejs#1497
Reviewed-By: Gibson Fahnestock <[email protected]>
Reviewed-By: João Reis <[email protected]>
This makes the parsing more robust and fixes the additional issue
related to USB Device Connectivity component.

Fixes: nodejs#1466
PR-URL: nodejs#1516
Reviewed-By: João Reis <[email protected]>
Reviewed-By: Refael Ackermann <[email protected]>
Reviewed-By: Richard Lau <[email protected]>
This is cleaner than filtering additional strings from
platform.python_version().

PR-URL: nodejs#1504
Reviewed-By: Anna Henningsen <[email protected]>
Reviewed-By: Richard Lau <[email protected]>
Reviewed-By: Jon Moss <[email protected]>
Reviewed-By: Rod Vagg <[email protected]>
danbev and others added 24 commits August 9, 2018 10:51
this is a re-base of the gyp part of
6a09a69ec9d36b705e9bde2ac1a193566a702d96
after bumping GYP version to
https://chromium.googlesource.com/external/gyp/+/eb296f67da078ec01f5e3a9ea9cdc6d26d680161

Original-PR-URL: nodejs/node#11956
Original-Ref: nodejs/node#9163
Original-Reviewed-By: James M Snell <[email protected]>

PR-URL: nodejs/node#12450
Reviewed-By: João Reis <[email protected]>
The ability to set the link rule is used for FIPS, and needs to set
both the `ld =` and `ldxx =` variables in the ninja build file to link
c++ (node) and c (openssl-cli, etc.) executables.

URL: nodejs/node#14227
Reviewed-By: Refael Ackermann <[email protected]>
Reviewed-By: Ben Noordhuis <[email protected]>
Reviewed-By: James M Snell <[email protected]>
PR-URL: nodejs/node#20716
Fixes: nodejs/node#20682
Reviewed-By: Trivikram Kamat <[email protected]>
Reviewed-By: Ruben Bridgewater <[email protected]>
Reviewed-By: Michael Dawson <[email protected]>
Reviewed-By: Colin Ihrig <[email protected]>
Reviewed-By: Luigi Pinca <[email protected]>
Reviewed-By: James M Snell <[email protected]>
Previously running ./configure with only the Xcode Command Line Tools
installed would give:

xcrun: error: unable to lookup item 'PlatformPath' from command line tools installation
xcrun: error: unable to lookup item 'PlatformPath' in SDK '/'

Co-authored-by: Ben Noordhuis <[email protected]>
Fixes: nodejs/node#12531

PR-URL: nodejs/node#21520
Reviewed-By: Ben Noordhuis <[email protected]>
Reviewed-By: Joyee Cheung <[email protected]>
Reviewed-By: Gibson Fahnestock <[email protected]>
Reviewed-By: Refael Ackermann <[email protected]>
Reviewed-By: James M Snell <[email protected]>
Currently the files specified in libraries in node.gyp `cctest` target are
getting a '.lib' extension on windows when generated with ninja.
This commit adds a check to see if a file has a '.obj' extension and in
that case no '.lib' extension will be added.

Also, the LIBS specified in the 'libraries' section are not
being included in the --start-group --end-group section which
means that these libraries will not be searched causing issue
with linkers where the order matters.

PR-URL: nodejs/node#12484
Fixes: nodejs/node#12448
Reviewed-By: Benjamin Gruenbaum <[email protected]>
Reviewed-By: James M Snell <[email protected]>
Reviewed-By: Refael Ackermann <[email protected]>
Reviewed-By: Gibson Fahnestock <[email protected]>
This fixes linting.

nodejs#1561
Reviewed-By: Refael Ackermann <[email protected]>
* assertEquals() with assertEqual()
* iteritems() with items() for Python 3.
* xrange() with range() for Python 3.

nodejs#1150
Reviewed-By: Refael Ackermann <[email protected]>
On Python 2, basestring is (unicode, str).
On Python 3, basestring and unicode are gone, and there is only str.
PR-URL: nodejs#1333
Reviewed-By: Refael Ackermann <[email protected]>
target is an undefined name in this context.

nodejs#1334
Reviewed-By: Refael Ackermann <[email protected]>
Breaking change: needs Node.js version 6 or higher

nodejs#1570
Reviewed-By: Refael Ackermann <[email protected]>
PR-URL: nodejs#1269
Refs: nodejs#1582
Reviewed-By: Rod Vagg <[email protected]>
Reviewed-By: João Reis <[email protected]>
Try everything until Python is found.

PR-URL: nodejs#1582
Reviewed-By: Rod Vagg <[email protected]>
PR-URL: nodejs#1245
Reviewed-By: Gibson Fahnestock <[email protected]>
Reviewed-By: Richard Lau <[email protected]>
Reviewed-By: Refael Ackermann <[email protected]>
Suggest using --verbose npm switch when providing logs. Hopefully,
better direct users to use backticks correctly.

PR-URL: nodejs#1618
Reviewed-By: Richard Lau <[email protected]>
Reviewed-By: João Reis <[email protected]>
Reviewed-By: Ben Noordhuis <[email protected]>
@joaocgreis
Copy link
Member

This should not be fixed by adding instructions to install an outdated version of Visual Studio, but rather by understanding what went wrong and adding a better error message for it. Visual Studio 2017 is fully supported (provided there is no incompatibility in the modules' code) and should be used.

@bertyhell can you paste the full output of node-gyp (or npm) when run with --verbose? It should indicate clearly why it is not using Visual Studio 2017, or provide clues to what the error is. Also, thanks for trying to make node-gyp better and actually submitting this PR!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.