Executable Versions + Script Runner #111
Comments
|
You mean to specify some versions in the project somewhere, so it will warn or prevent you proceeding if you've got a build setup that won't work? An interesting thought, although I think we should focus on testing a set of supported build setups and documenting which are supported. I just did a very large amount of research and testing as to what works for building Atom today that you may be interested in: atom/flight-manual.atom.io#630 (comment)
|
|
We already have a good way of detecting Node, npm and Python and stopping you from bootstrapping if you've got the wrong versions. (Edit: This is a bit out of date for Node version, though, as of atom#20879. ( It's surprisingly difficult to properly detect a working Visual Studio install. And node-gyp will tell you if you've got no working Visual Studio installs. I'd like to leave detecting Visual Studio to node-gyp. (Long aside: We have a fix for I would be shocked and surprised if powershell version or cmd.exe version had any impact on a successful build. Okay, I've seen a lot of things, so it wouldn't be that surprising. It's my sense that that isn't the case though, absent specific testing that shows otherwise. |
#10 will solve this issue partially. You will not need anything to start hacking the core using the pre-bootstrapped repository. To build the Atom, I just install One other way is to use a docker file. We have a docker file but it is out of date. I need to update this script. This is missing a couple of things. |
|
Docker would be nice. I'm on Mac but I triple boot and I'd like to generate a redistributable app for all three OS's.
Idk if that can be expected to be reliable. I've tried node 14, 12, 10, 11, python 2 and 3.6, 3.7, 3.8, disabling the GitHub extension, upgrading downgrading npm packages, deleting node_modules and the other usual suspects, resulting in different build failures. I've got the full install of xcode and the other prerequisites on the flight manual. Just now (after changing nothing inside the folder for a week), the build script completed for the first time. But as luck would have it, the Dev Atom immediately fails on opening with a A package-lock style of guaranteed-to-work versions of node, python, etc would just be nice so that I can rule those out when trying to debug why the build is failing.
@aminya |
|
(Thanks for the help btw) |
Yes, you can edit your path so the Node provided by apm is the first. Yes, I said, I need to change the scripts so that Node version is automatically found! The script that we have which checks the versions is out of date and does not reflect the actual requirements! For now, you can download and use the following 😄. Use the absolute path to this to run the scripts. Like: To make this Node version yourself, run |
|
How old is your mac? Are we talking recent like Mojave or Catalina? Older than that? (Should still work, but it would be nice to know.) Also, having Edit to add: It's a good habit to do Lastly: How much RAM and free disk space do you have? ~4GB is enough RAM, but could cause issues if you also have a lot of other things open. Especially web browsers, even moreso with lots of tabs open. If you're almost 100% out of disk space, things could get dicey during install, files not ending up where they're supposed to, etc. I am on Linux and Windows, and builds there are fairly reliable on my real machines on command-line. (As opposed to CI, which is its own separate thing)
Technically you could copy that somewhere and add it to your PATH... But, uh... on Windows I just download the x64 .zip from here: https://nodejs.org/dist/ extract to somewhere and add it to my PATH. And on Linux I use
For macOS, you should just need these on your PATH:
Nothing else should be required whatsoever in terms of system requirements (on macOS). But willing to troubleshoot with you. |
Haha, yeah thats why I think they need to be locked to a specific version. I'm on node 12.0.0 which is probably why the build worked this time, I was previously on 12.16.x At the moment I'm testing on a Hackintosh, 48Gb ram, running Sierra, but I've got mac books with Mojave and High Sierra I can build on as well. I run a lot of projects with a lot of different versions so switching isn't too bad, I use asdf, nvm, and nix for managing versions. Note:
Note the first Node is 12.4.0, then the second one is Node 10.20.1 x64 which is weird
I don't think this is a good policy. It takes 1 minute to add a powershell version to a package.json or somewhere else. The "Node.js 6.x or later" thats still on the Flight manual was probably also a "It's my sense that that isn't the case" recommendation, which has cost me and others days of debugging. I'd bet most people trying just give up and don't contribute. I don't mean to sound attacking; you guys are helping me out and I see you contributing everywhere. But the CI tests are already spotty right? pinning down the versions seems like the best action to start making builds more reliable. It takes 5 min to record all the working versions you have, it can even be automated; but, on my end, checking all the possible combinations of versions is nearly impossible. Why would you not just preemptively record the versions? |
|
Hey @aminya your trick worked, Atom Dev actually opened without crashing 🎊 🎉 Thanks 👍 |
|
Thank you Jeff. I will update the scripts such that the versions are pinned. Currently, the only version of Node that I support is 12.4.0. It should build everything from Atom to apm itself.
That is totally true. I have had this issue myself. That's why I created #10. Using that PR you can edit the JavaScript code right away! It is like a prebuilt docker image! Additionally, I am also trying to pin and unify the versions everywhere. A single pinned version makes sure that everything works. For example, different node versions in Atom and APM created a lot of issues for us. Currently, this file includes all the versions I recommend: I am thinking of making a Using a container is another solution, but not all people have experience with using those. A |
That sounds fantastic, I'll have to look into that further.
When I propose changes, I'll try to help with that. I noticed just now that oginumura and the tree sitter are using native modules even though there's WASM builds for both so I'll be trying to swap those out to lessen the dependency hell. |
That's awesome! I am looking forward to your contributions! Currently, Atom does not tree-sitter by default, and you should enable it in the settings. I was thinking about making them the default. I'll wait until you bring those changes, and we can switch to tree-sitter as the default language mode. |
|
Hi again, folks. I do not doubt your personal experiences, but if there are specific failures on various Node versions, we should document them, as they may be specific bugs we can solve. The reason most projects do NOT pint to an exact version is that you simply don't have to. The code builds on many Node versions. It is bordering on superstitious to pin to an exact Node version. Most Node code is Node version agnostic to a certain degree forward and back. It is certainly less informative (and less proactively healthy for the code) to pin a Node version, rather an tracking down and solving these bugs. Though admittedly, Atom runs on fixed versions of Node/Electron, so the (direct) impact to end-users is none. It's more of a "looking out for developers' sanity, long-term" thing. My two cents. Glad to see you are having more success now building Atom, Jeff. |
|
Also please be careful doing It should be (This is due to a weird workaround for using apm with npm@5, as |
if the code is only JavaScript that is not a problem because Node is not breaking most of the time, but native modules are required to be built against the same Node versions that run them! There is no way you can build a native module on a different Node version and run it. See this document so you get familiar with this: That is why we shall use the same Node version in both apm and Atom. The specific projects that use prebuild or node-pre-gyp may run in Electron without rebuilding from the source. |
|
First of all I have no problem with this:
If you want to run a system Node version synced with What you say is true. But consider this additional information, which is true as well:
All code is executed by the Node that installs it, (or built for/executed by the Electron that runs Atom) so there is theoretically no problem if multiple different versions are used. There is no particular reason they need to be in sync. (The build scripts in (Real-world example: Atom was on Electron 5 (~Node 12.0.0) while Again, it would be really ideal to track down any situation where this is not the case, document it, make it reproducible, and squash the bug. I continue to recommend using the most stable/reliable system Node possible, and nothing more than a year old. (Newer means more bugfixes from Node's developers, I guess.) Doesn't matter what version exactly. What works for you to build Atom is all that really matters. And for I know it's easier to think about using one version of Node everywhere, but it's even better to understand our technology stack and not be afraid of the moving parts. And to proactively fix bugs when we find them rather than pinning Node as a workaround. Hacking on Atom doesn't (shouldn't) require an exact version of Node on the system PATH, for convenience's sake. Developers should confidently build Atom from whatever recent-ish Node they have installed. If something breaks on other versions of system Node, it's a bug. Sorry to keep saying this, but it's true every time I say it. |
|
I wonder how you explain the issue in atom/apm#889 (comment). |
|
Those ABI versions in the error message (ABI 70 and 73) are reserved strictly for Electron. (Node and Electron no-longer share ABI versions starting roughly January 2019. nodejs/TSC#651) According to the error message, the code was built for electron 5, but trying to run in Electron 6. The only logical explanation (barring very severe bugs) is that you accidentally had code (an Atom package) installed for one major version of Electron and tried to run it on a newer Electron around the time of the Electron 6 bump. We do switch branches a lot working on various things, so this is a totally understandable situation, in my opinion. I don't know the exact details of how they happened, but the error message is pretty clear about what's going on, once you know what place to look up those ABI version numbers. There is no reasonable way that Node version bumps could cause those errors. |
|
I did not build that Atom. I just downloaded the artifacts from the Electron 2 pull request, and rebuilt the package using Atom's included The whole point is that Atom's runtime should use the same Node version of Electron for building the native modules. It is OK to rely on being lucky sometimes, but I don't want to advertise that in the documentation. There is no point in using different Node versions recklessly just because we can. If we want to remove this issue completely, we should use electron-rebuild. The |
|
I think the If We can pin Node in CI if you want, but I happily build Atom on my personal machine with Node 10.x, and anyone else who wants to use a version of Node other than 12.4.0 is welcome to post bugs here if they run into problems, and I'll help troubleshoot them in my spare time. 10.12 through to the latest 10.x should work, as should Node 12.0.0 through 12.16.3. If one of those doesn't work and somebody wants help to build Atom, I'll help them do so. That's my personal take. What Node versions should work isn't a matter of opinion, it's the facts of our build stack. I really don't want to argue just for argument's sake, so at some point I'll stop, but my opinion hasn't changed on this. (If you're running into errors, you may be running a different build technology stack than you thought you were running. Maybe |
This was true at the time it was written. The flight manual's build requirements just haven't been updated for a long, long time. We are working on a PR for that. Just need to decide what phrasing to use or use what's already in the PR, get an approving review and a merge by upstream. Or... I'd really rather not fork the docs. Upstream is still alive. They need current docs, too. |
I'd hope for something like: "
"
Without evidence, saying its true doesn't add to the conversation. Builds are not reliable, even if you want them to be, even if they should be. "If something breaks on other versions of system Node, it's a bug." Fine, lets label it a bug, that doesn't help me or other people build Atom. If you want to label that Atom "should" work with ___, thats fine with me. But withholding a "We know for sure you can build Atom with these exact versions" makes it harder for people to contribute. What downside is there to adding that? Docker was invented BECAUSE of this problem of reliability, its a serious industry issue inherent to all software. But, let me provide evidence; as part of the coding club at my university we take student projects of all kinds, help them get a team, and make the project reliable for all of them. Students come with all kinds of operating systems, preinstalled tools and environments because they're developers, just like us. These projects are fairly simple; static websites, mobile apps, python data plots, command line tools, and even some electron apps. Me and the rest of the senior team created and tested build scripts for weeks on VM's of multiple versions of Windows, Ubuntu, Manjaro, and laptops with the last 4 versions of MacOS. With the build script working in all of those environments, we gave a tutorial to a room of 60 people. Almost 20% had setup failures and issues of some kind. But that wasn't an isolated event, this happened bi-weekly for years. It happened so often that I put together a team to build a toolkit called ATK (its on Github) for creating a reliable build scripts. I spent 2 years on the project, and after testing it in hundreds of different environments, it is still unreliable. There are so many hidden dependencies and interactions, reliability can't even be ensured even when exact versions are pinned. I've spent way to much time working on the dependency problem, pinned versions help development. Its like booting atom in safe mode. Just because should work without safe mode doesn't mean safe mode isn't needed. |
|
Long story short: I'm sorry for being difficult and for my tone. And I still want us to spend some effort understanding why the build failures you are seeing are happening, and why Node v12.4.0 and other pinned versions would help (please do file issues about it if you can spare the time/energy! I'm just sincerely trying to troubleshoot). But I'm willing to work on tracking down these build failure scenarios myself if no-one else wants to worry about it. And I don't personally understand very well which versions need pinning (I can't reproduce the conditions where pinned versions help), so I don't want to undertake writing that documentation myself. @aminya or @jeff-hykin please one of you handle pinning the versions. I'm not against it in the short-term, but in the long-term, I think understanding the specific build failure scenarios would also be a boon to this project. (More thoughts: We can't stay on Node 12.4.0 forever if we want Electron 7 (Electron 8 is the oldest supported Electron!) and we will probably want to keep bumping Full ginormous reply (click to expand if you want):Sorry for the tone of my responses.
If folks feel strongly about this, I won't try to stop them, and I don't mean to be annoying or disrespectful about it. My apologies. I should really cool down and be more measured with how I respond.
I hear your frustration and I acknowledge you and @aminya reporting that this doesn't work reliably, if at all, for either of you. So it's two against one now, meaning my assertions that it "should work" aren't helping much, as you say. I'm squarely outnumbered as it pertains to anecdotal levels of evidence. What I should have emphasized is that this "works for me," which is a lower standard of proof than "always works." I don't mean "It always works," just "it should be working and let's find out why it's not." When I say "It's a bug" I'm very sincerely asking that you or @aminya file an issue here. Let's change it from "something isn't working but we're not sure exactly what it is" to "Okay we sort of know what's not working" or even "We know exactly what's not working"... best-case scenario we find the fix or the least-impactful workaround, and it helps us refine the documentation to be more helpful and accurate. "It's a bug" isn't meant to be dismissive, it's a call to action to post issues with whatever details you can find and let's track down the problem. I apologize again if it came off that way, because I was admittedly getting upset, and I could've been calmer.
Thanks for sharing that. I can say I hadn't known where you were coming as clearly before. I admire that project and I respect the experience and learning you've had. I don't know how to segue that into anything, but I don't want to dismiss anyone else's expertise. I too share a desire to have this project, and all open-source software, be accessible to the maximum number of people. My belief is that, long-term, knowing why our dependency stack is what it is is more important than finding some versions that work in the now. Any old dependency could become unavailable or receive a patch-level bump that breaks the house of cards we've built. I want to understand our margins of safety to know how healthy or close-to-breaking our tech stack is. I don't want it to be a surprise when a build fails, I want us to know the answer of why. Pinning is okay, but still, if you've tried anything other than the pinned versions and it fails, please do send me the error messages you get, in some form or other. I want to track down the problems and fix them. I'm personally dedicated to diving in and understanding what frustrates our potential developers in as granular a level as possible and fixing it.
I am cautious about it, because the evidence for it is anecdotal. What works for three people (Aminya, you and myself) may not work for the next person just because it's working for us. Node 12.4.0 isn't exactly the same as Electron 6. It has a separate API/ABI. Using it only closely approximates the Electron we are building for. Without any complete explanation for why only this version works, other than intuitively "it matches Electron" (when it shouldn't have to) is again a bit superstitious vs. factual. That shouldn't stop us from doing that informally, or saying "If you're having difficulty building Atom, the following versions are the most-tested and should work for you." The issues are just that pinning means you need to keep manually updating the pinned version (including in the docs). People forget to. An if we update the pinned version with any frequency, someone will inevitably land on outdated docs at some point and be confused why the exact version we suggested isn't working anymore. Non-pinned versions are a luxury I'm trying to preserve. They're the optimal situation, if actually workable. But I can admit it's proving controversial to suggest that that works, when two out of three people here can't use non-pinned versions, anecdotally. Bottom line: I am willing to be neutral toward using pinned versions. I won't stop anyone from doing it. If at all possible, I'd like us not to not simply lament the lack of evidence for or against, but rather compile more evidence so we can say definitively what's going on. Pinned versions are an acceptable workaround, but absent any proper explanation of why they might be needed, I continue to seek the missing explanations so we can fill in the gaps and understand the problem. If you folks are just wanting to move on, I'll have to do that research myself when I get the time. Otherwise join the effort, file bugs. I think that's all I should say about that. I don't understand intimately which pinned versions are needed, so @aminya or @jeff-hykin I would leave it to one of you to put together the documentation and so on about it. |
|
I made #118 and #116 to start looking into this kind of dependency/build issues, to work out what's going wrong. I have a baseline "steps to reproduce" to reproducibly build on Node Best Regards, - DeeDeeG |
|
Sorry for late reply @DeeDeeG, I got swamped with work later that day.
Hey, I appreciate it. Don't worry about apologizing 👍 just promise when our roles are reversed (and I'm the one being difficult) that you'll be patient with me, and keep pushing your point. 😁
On a similar note to my last comment, there may have been some poor communication on my side on this original post. I mean pinning in a way similar to a package-lock (auto generated), not at all in the way that Atom pins/freezes the version of Electron for a long period of time. I'm just talking about having a record: when any build is completed, the version of node used (whatever it might be) is saved, like
No problem 👍 I'll happily work on this. I've got a bunch of tools for it. Assuming I can also get the Electron 6 version working, I'll probably have a PR next week. You might also be more comforted by a system that lists multiple pinned versions. Basically have every successful build for a particular commit record the exact OS version along with all the versions. In the CI scripts we could iterate through versions of node/python/etc basically providing more and more proven options with each additional test.
All of them haha; git, node, npm, powershell, python, XCode, Visual Studio and anything else that is depended upon.
Thats a good point, and maybe I wasn't initially clear. I think most everyone (certainly myself) will still use loose/unpinned versions for every day builds and testing: my node version practically changes with the sunrise, BUT whenever we get a build issue, we can compare ours to the pinned versions to start figuring out what's different/wrong. Rest assured though, I agree we should still treat failures on different versions like bugs and fix them. Fragility is bad, and I'd even like to work on making the electron version less locked-in. 👍
True, I should have changed 'you' => 'we' for "We know for sure we can build Atom with these exact versions"
We can just have the docs reference the auto-generated record of the version so they're never out of date. (Or, even better, we could maybe auto-generate a change in the docs). |
I didn't realize this was part of the problem. Is there a way to build it with Electron directly? (Similar to using the apm version does? @aminya) I would want to use the node/electron inside the Atom app, except there needs to be a way to try to port Atom to new system like raspberry pi or the new ARM Macs. If there's a way to create builds for them without being on those systems then using the node/electron inside the Atom app seems viable. |
That would be awesome! I was thinking about how to approach this issue. But since you have done similar things before, it would help us a lot.
That is not a problem. As stated in the docs, each Electron version uses a certain Node version. So running If native modules are built with another version, and the build succeeds, in the end you can use |
Yeah that sounds pretty bad (and difficult to fix). Also thanks for the details on the env and node-gyp stuff. IMO we (as users) should be able to have multiple versions of Atom installed simultaneously even with different node versions. It might be tough to do, but I think (eventually) ~/.atom should only have config info. E.g. which extensions but not the entire extension itself. |
I agree. The version of node that the extension was built could be saved inside the extension's package.json. This will also be helpful when users generally have issues with extensions. The check can be done asynchronously after Atom has already started, so it won't add much of a performance penalty. For normal users it should cover any issues they have from updating or somehow using multiple versions of Atom, since they're unlikely to bound between versions. |
I think this would be nice for testing so different configs could exist. If not too difficult I think we should explore this. However, I don't think this is a good solution for compiled dependencies
|
|
We could make it configurable. Something like |
|
It would also need to be configurable in |
|
Also, just to clarify my position, if the compiled extensions are to be moved outside of |
Maybe automatically use the name of the app (and still allow a --config-folder option). This would solve my first point (forking), and my second point is low priority anyways |
I don't think it would be a breaking change if we made it configurable as long as we keep the default the same. |
|
Instead of using different folders, we should first fix this so Atom detects the issue and recompiles things: #111 (comment) Then we can work on optimizing by using different folders such that no recompilation is needed for different Atom versions. IMO, the folder name should be based on the Electron version which affects the native module recompilation: |
I agree, lets focus on recording and checking the versions and worry about multiple Atom versions in a separate issue. I built a tool and published it on npm for tracking versions of binaries, and I'm working on a PR now. My build still fails with node |
|
PR created: #134 |
|
Note, maybe a new issue should be made for this: (Look at both This was from my successful build with latest master 🤔 🤔 🤔 🤔 🤔 |
|
Hi @jeff-hykin that's a great question to ask. I happen to have the answer. The first set of versions are what you have on your system PATH. These are printed by verify-machine-requirements.js output, explained line by line (click to expand):
The second set of versions are printed by apm --version output, explained line by line (click to expand):
In conclusion: Nothing too janky. This is all running as expected. Maybe we should print "System Dependencies:" for the first set of versions, and then "apm Dependencies:" for the second set, to make this less confusing. I admit it could be made clearer at a glance what's going on. |
|
Finally got Atom today but not reliably. There are some serious issues :/ First I gutted everything from my bash/zsh profile/rc except for adding python, node, and npm to my path. Somehow node-gyp was still finding and defaulting to python 2.7.18 (an ASDF shim) even though asdf was disabled and not on my path. I removed Then atom was defaulting to python 2 instead of python 3, part of that is an issue with the code here the order of the lines should be changed. Then I was getting After upgrading brew I had/have node v15 and npm v7. Now the below process * almost * leads to Atom building The final script build will fail because of this Then, atom continues the build, but not without warnings (see final log: log#5). Log#1 Log#2 Log#2.1 (tried again, slightly different error) Log#3 (success) Log#4 Log#5 |
We should add this fix to the build script. Could you write what change are you applying exactly? We have an issue for this in Atom One of the issues that my big babel PR fixes is this, but that PR is blocked on some CI issues. |
Sorry I forgot that was a long issue. The change is just to And yuppp, this^ should fix that -> atom#21091 |
|
I pushed a patch to GitHub. if this solves the issue, we don't need a hot patch. |
|
Thanks man @aminya. I probably should've asked this sooner, but is (was) the docker build capable of building atom for any OS? If thats the case I'll stop trying to build on mac right now, and work on updating the docker image instead. |
|
Working on another project and found this 😑 |
Edit to add: I think this is the Dockerfile in question? CI currently builds the As far as I know, you have to be running macOS to build for macOS, running Linux to build for Linux, running Windows to build for Windows. I'm not aware of a way to cross-compile native module/package/dependency code in the Node ecosystem for another OS, but that's what would be needed here to compile for macOS or Windows in Docker. Or some way of running macOS and Windows as virtualized/containerized guest systems. |
|
Running Is anybody aware on how this can be fixed? |
The fix is described here: indentjs/objectorarray#7 (comment) . For the two files in question, here is a patch: |
This still happens with the Debian package built from the latest sources. As the instructions here seem to be focused on Windows please let me kindly ask you for clarification how to fix that error on Debian Linux. Thank you very mich in advance. |
AFAIK, there isn't a standard place for mentioning versions of binaries. For example, what version of powershell is being used for the CI scripts, and versions of python are needed for node-gyp for building native modules.
Ideally there would be a standard way to list versions of these binaries (outside of atom) but I'm unaware of any standard. So, in substitution of that, could the versions be added to the package.json as a "bin" or "binaries" or "executables" object?
I think this would be helpful since it's already a struggle trying to build atom.
The text was updated successfully, but these errors were encountered: