Closed DeeDeeG closed 4 years ago
I was tempted to update the Node/NPM versions to supported versions (Node 10 or newer, NPM 6 or newer). I guess the older versions might work (it suggests Node 6 and NPM 3, which are very old by now and no-longer supported) but I haven't tested that yet.
(The norm in the Node ecosystem is to point people toward supported versions of Node, and to a lesser extent to point folks toward supported versions of modules such as npm
. I think it would be resonable to update those to v10+ and v6+ respectively, and I'd be happy to do so in this PR if a reviewer here was alright with that.)
I found that Atom requires Node v10.12+ to build now. This comes with npm 6 bundled with it. So that should probably be updated as well.
Research:
Atom has an indirect dependency extract-zip@^2
, which requires Node v10.12+ due to using recursive mkdir
.
fs.mkdir
extract-zip@2
uses recursive fs.mkdir
, thus requiring Node v10.12+Atom indirectly requires extract-zip@2
as of this PR: https://github.com/atom/atom/pull/20879
electron-chromedriver@^9.0.0
and electron-mksnapshot@^9.0.2
, which both depend on extract-zip@2
.One last thing to note, Python 3 only works with npm
v6.12+ https://github.com/npm/cli/blob/latest/CHANGELOG.md#6120-2019-10-08, which was the first version of npm
with node-gyp
v5.0.5+ https://github.com/nodejs/node-gyp/blob/master/CHANGELOG.md#v505-2019-10-04 which is in turn the earliest version of node-gyp
that works with Python 3 "out of the box" (no experimental flags required).
So maybe that npm
version requirement for using Python 3 should be explained here in the docs, or to keep it simple, just say Atom requires npm
6.12+.
Travis test failure seems unrelated. It apparently timed out trying to visit https://bugs.chromium.org/p/gyp/issues/detail?id=393, which I can visit just fine. Just a temporary network issue I guess.
Edit: Somebody re-ran the CI and it passes now. Thanks!
@darangi, @lkashef:
we (@aminya and I) are trying to work on Atom and polish up all the rough edges for those who want to fork it. This important documentation explaining how to get started with Atom is one of the first things users read when trying to contribute. We would really appreciate if these updates could be reviewed and approved.
These suggested updates are based on our active contributions to Atom, including my work on updating some of Atom's build script Node dependencies, and both of our reading through/running the CI configs. I know them to be accurate. It would be great to get updated docs.
Thanks for considering.
- DeeDeeG
@DeeDeeG and @aminya Thanks for the contribution 🙇♂️
We can't provide an accurate timeline but we will definitely put this on our radar and act on it as soon as possible.
More research about which NPM versions are supported:
Briefly: Users with NPM 3.10.7 or newer can bootstrap and build Atom successfully.
(I personally still lean toward recommending folks should use the latest major version of NPM, that is currently NPM 6 (or newer), which is what I did in this PR, but I am open to feedback from the maintainers.)
Research about Visual Studio:
node-gyp 5.x supports Visual Studio 2015, 2017 and 2019, at least officially. (See the CHANGELOG.md. Visual Studio 2019 support was added in node-gyp 5.0.0.)
With the versions of Node we are using, Visual Studio 2013 is no longer supported. If you try to do node-gyp configure --verbose
on Node >= 8, you see this message in the log:
not looking for VS2013 as it is only supported up to Node.js 8
I cannot build Atom using 2019 preview. I believe that's the same as 2019.
I cannot build Atom using 2019 preview. I believe that's the same as 2019.
I just built Atom with Visual Studio Build Tools 2019 to make sure it works.
@DeeDeeG Thank you for your submission. :tada:
Have you considered all the possible combinations and ensured that all the them work? It would be really helpful for us to have a list of the combinations you have tried and the results.
@sadick254 yeah, I can do some testing on this.
There's definitely a lot of "moving parts" so to speak, considering these:
According to researching the documentation, these versions can be used: Node 10.12+, npm 3.10.7+, Python 2.6/2.7 (Python 3.5+ can be used with recent npm), Visual Studio 2015 (Visual Studio 2017 and 2019 can be used with recent npm).
(Note: Node 12.17 and newer are not usable due to this issue: https://github.com/atom/atom/issues/21091)
I can test some combinations within those ranges and let you know how it goes.
@sadick254 I've got a big testing "info dump" here. I did some testing with old Node, npm, Visual Studio, and Python.
Conclusion: Node less than 10.12 can't build Atom.
Conclusion: npm less than 3.10.7 can't build Atom.
Conclusions:
npm config set msvs_version=2015
, OR in your environment variables set GYP_MSVS_VERSION=2015
.)Conclusion: Python 2 works everywhere. Python 3 requires npm 6.12 or newer.
Some notes about Python on Windows:
If users have used the windows-build-tools
package, that will have installed Python 2.7 for them.
Python 3 from Python.org doesn't install to a nice predictable location by default, nor does it put itself on the PATH by default. You have to configure it to a specific install location OR put it on the PATH. That's why the Microsoft Store version of Python 3 is recommended; It always puts itself on the PATH.) (Not an issue for Python 2.7; the default install location is very predictable for Python 2.7).
In my opinion we should say Python 2.7 is preferred over Python 2.6. (2.6 is not supported, and neither is 2.7, but 2.7 has the more recent bugfixes. And 2.7 will be supported in newer node-gyp going forward, whereas Python 2.6 support will be dropped.)
If we required npm 6.10.1 or newer, all currently supported (by Microsoft) versions of Visual Studio would be usable with no extra configuration.
If we required npm 6.12 or newer, Python 3 would be usable always, with the caveat that it has to be installed the right way on Windows, or it won't be detected.
I'm a bit inclined to do that so that developers are using the newest tech, and so the instructions in the flight manual can be simpler.
We could update the hard requirements here:
On another note, we should update the required Node in script/lib/verify-machine-requirements.js
to be at least Node 10.12, if not something newer:
We could maybe set the max Node version at 12.16.x as well, until this issue can be solved: https://github.com/atom/atom/issues/21091
Visual Studio 2019 does not fully work as mentioned in: atom-ide-community/atom#117
I suppose we can hold back on recommending Visual Studio 2019 for now.
The Node version should be fixed to 12.4.0 to prevent the issues mentioned in atom-ide-community/atom#111
My research suggests pinning the system Node version is not relevant to the errors reported there. I have passing CI runs (100% tests passing) with system Node 10.12.0 and with 12.16.3.
There is no point in widening the Node version that is supported.
Neither is there any point in artifically limiting our documented supported versions, when they are known working and well-tested and confirmed to work.
Electron and Node version bumping should not be considered in this document.
Users will use what they have on their system. This document is to tell them what will work and what won't.
This is just a document about the "supported build configuration".
Indeed, Node 10 (10.12+) and Node 12.0.0 through 12.16.x are working. We should support all current LTS versions of Node, minus the ones where we know we are incompatible (meaning, we do not support Node <= 10.11, or Node >= 12.17).
https://nodejs.org/en/about/releases/
I am sorry you and Jeff are running into errors in whatever scenarios that cause them, but you have no "steps to reproduce" and I can't reproduce those build failures. Please either narrow down what causes the errors happen, or do clean installs every time (run script/clean
before attempting to build Atom). I think if you do script/clean
you will find locked down Node versions aren't needed. If that's not the case, then unfortunately more troubleshooting is needed for your specific systems. In general, and following the cleanest install approach possible, with default system configuration, the build "just works" on Node 10 (10.12+) and Node 12 (<= 12.16).
We don't know what exactly causes the erros for you and Jeff, but I believe they are exceptions to the usual, rather than representative of the norm.
@DeeDeeG I see a lot has been done since I last visited this issue. I will go through the research you have done and read through everything. Thank you so much for all the hard work you have done so far. We really appreciate.
I'm pretty happy with the edits I've proposed to the documentation.
The main things I am considering changing are:
windows-build-tools@4
, because the latest v5 sometimes fails to install.
Requirements for Contributing Documentation
Description of the Change
Information on required Visual Studio versions and Python versions were out of date, considering that we use newer
node-gyp
now.See: https://github.com/nodejs/node-gyp/tree/v5.x/#installation for a more up-to-date reference on the required Visual Studio and Python versions, and other miscellaneous requirements of
node-gyp
.I updated "Hacking on Atom Core" to reflect that Python 3 now works (See: https://github.com/atom/atom/pull/20711). Visual Studio 2017 has also been supported by recent
node-gyp
for quite some time. Relatedly, on Windows 10 the Windows 10 SDK is available, not the Windows 8 SDK. Node v10.12+ is required, and NPM < 6 is no longer supported by the NPM authors, so I updated the documentation to reflect that as well.Release Notes
N/A (Atom Flight Manual doesn't appear to compile or publish any release notes)