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

Switch apm to @atom-ide-community/atom-package-manager #88

Merged
merged 2 commits into from
Aug 18, 2020

Conversation

DeeDeeG
Copy link
Member

@DeeDeeG DeeDeeG commented Aug 17, 2020

Issue or RFC Endorsed by Atom's Maintainers

Helps with #44

Description of the Change

Use a fork of apm that's updated more frequently. By the same maintainers working on this fork of Atom. This fork is somewhat conservatively updated with needed fixes/dependency updates, and quality of life improvements.

Right now, the changes are limited to:

  • Newer bundled Node (that exactly matches the version of Node that Electron 6 is based on, which we ship at this fork of Atom).
  • Better support for Visual Studio 2017 and newer
  • Normalized line endings (CR/LF) so that releases can be properly published from a Windows machine.
See the Changelogs:

Alternate Designs

None

Possible Drawbacks

None.

Verification Process

CI should pass, manual testing shows that this works as a drop-in replacement with all the commands I've tried.

Release Notes

Switch apm to @atom-ide-community/atom-package-manager (comes with bundled Node v12.4.0)

@aminya aminya changed the title Switch apm to @atom-ide-community/atom-package-manager (includes bundled Node 12.4.0) Switch apm to @atom-ide-community/atom-package-manager Aug 17, 2020
apm/package.json Outdated Show resolved Hide resolved
@DeeDeeG

This comment has been minimized.

@DeeDeeG

This comment has been minimized.

@DeeDeeG

This comment has been minimized.

@aminya

This comment has been minimized.

@DeeDeeG DeeDeeG force-pushed the atom-ide-community_apm branch 2 times, most recently from 622ebda to 645cd8a Compare August 17, 2020 16:50
@DeeDeeG
Copy link
Member Author

DeeDeeG commented Aug 17, 2020

@aminya rebased, and using the new versions of @atom-ide-community/atom-package-manager.

  • 2.5.0-atomic.1.1
  • 2.5.0-atomic.2.1

This comes with better Visual Studio 2017 and 2019+ support,
and bundles Node v12.0.0, which matches the Node version Electron 5
is based on.
This bundles Node v12.4.0, which matches the version of Node
that Electron 6 is based on.
@DeeDeeG
Copy link
Member Author

DeeDeeG commented Aug 17, 2020

I'm seeing something weird on this branch. The Linux build is segfaulting/erroring out after successfully building Atom and packaging the .deb and .rpm packages. CI failure on Linux.

I also had the same thing happen when building on my local machine.

This happens occasionally on other branches, so maybe this PR is just getting a series of unlucky flaky runs. But maybe we actually have to change something on this branch.

@aminya
Copy link
Member

aminya commented Aug 17, 2020

There is not a useful log to troubleshoot this. If got better logs let me know.

@aminya aminya marked this pull request as ready for review August 18, 2020 05:05
@aminya aminya added this to In progress in Package Management via automation Aug 18, 2020
@aminya aminya added this to In progress in Build, bootstrap, CI via automation Aug 18, 2020
@aminya aminya merged commit 3ab703b into master Aug 18, 2020
Package Management automation moved this from In progress to Done Aug 18, 2020
Build, bootstrap, CI automation moved this from In progress to Done Aug 18, 2020
@DeeDeeG
Copy link
Member Author

DeeDeeG commented Aug 18, 2020

We finally got it printing an error message:

FATAL ERROR: v8::HandleScope::CreateHandle() Cannot create a handle without a HandleScope
 1: 0x98edf0 node::Abort() [node]
 2: 0x98fe56 node::OnFatalError(char const*, char const*) [node]
 3: 0xb1506a v8::Utils::ReportApiFailure(char const*, char const*) [node]
 4: 0xeeab4a v8::internal::HandleScope::Extend(v8::internal::Isolate*) [node]
 5: 0xb16ad1 v8::HandleScope::CreateHandle(v8::internal::Isolate*, unsigned long) [node]
 6: 0x94ac93  [node]
 7: 0xee6f3b v8::internal::GlobalHandles::InvokeSecondPassPhantomCallbacksFromTask() [node]
 8: 0xc07d03 v8::internal::CancelableTask::Run() [node]
 9: 0x9f94a5 node::PerIsolatePlatformData::RunForegroundTask(std::unique_ptr<v8::Task, std::default_delete<v8::Task> >) [node]
10: 0x9fa582 node::PerIsolatePlatformData::FlushForegroundTasksInternal() [node]
11: 0x9fb8ad node::NodePlatform::DrainTasks(v8::Isolate*) [node]
12: 0x9d09be node::NodeMainInstance::Run() [node]
13: 0x967051 node::Start(int, char**) [node]
14: 0x7fa25947a840 __libc_start_main [/lib/x86_64-linux-gnu/libc.so.6]
15: 0x9055b5  [node]

https://dev.azure.com/atomcommunity/atomcommunity/_build/results?buildId=561&view=logs&j=0da5d1d9-276d-5173-c4c4-9d4d4ed14fdb&s=d654deb9-056d-50a2-1717-90c08683d50a&t=cccdf131-0893-5d7b-ccea-9ccfc8953f95&l=4981

This is on master branch "Release Branch Build" pipeline after merging this PR.

@aminya
Copy link
Member

aminya commented Aug 19, 2020

This seems to be related to one of the Node modules or mksnapshot.

Other people faced the same issue:
https://stackoverflow.com/questions/62431517/fatal-error-in-v8handlescopecreatehandle-cannot-create-a-handle-without

This one recommends using NAPI-Thread-Safe-Promise or node-napi-threadsafe-deferred instead of node-api-addon which is thread-safe. We need to find which package is using that and try to rewrite it using the thread-safe package.

Some important issues about this:
nodejs/nan#440
nwjs/nw.js#2489
LiskArchive/lisk-node#2
TooTallNate/node-weak#65

This is a N-api issue. One of the Native modules (or Electron itself) does not use thread-safe promises

For example: electron/electron#22843 fixed one missing handle scope in Electron 10, but this is not back ported to 6.

All the GitHub issues regarding this: https://github.com/search?q=Cannot+create+a+handle+without+a+HandleScope+is%3Aissue&type=Issues

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

Successfully merging this pull request may close these issues.

None yet

2 participants