pulsar-edit / pulsar

A Community-led Hyper-Hackable Text Editor
https://pulsar-edit.dev
Other
3.33k stars 140 forks source link

Test CirrusCI with updated `superstring` [DO NOT MERGE] #1048

Closed savetheclocktower closed 4 months ago

savetheclocktower commented 4 months ago

This is designed to kick off a CirrusCI job. If our superstring approach is working, that job should produce a functional Pulsar binary for Apple Silicon.

savetheclocktower commented 4 months ago

Miraculously, @meadowsys reports that the CirrusCI job triggered by this PR produced binaries that appear to work just fine on an Apple Silicon machine. We will proceed with merging the necessary changes into superstring; I'll then produce a cleaner version of this PR.

When that PR lands, we should be back to our standard binary-building schedule on all platforms.

savetheclocktower commented 4 months ago

OK, the number of “renderer process crashed“ messages I'm seeing in CI is concerning. Trying to get to the bottom of it now.

savetheclocktower commented 4 months ago

Take this one, for example. I can't reproduce it locally when I grab a binary from last night's CI run, switch to the autocomplete-css package directory, and run pulsar --dev --test spec. All the specs pass and nothing crashes.

savetheclocktower commented 4 months ago

This turns out to be because we weren't on the specific version of libiconv that was necessary; now grabbing it directly from Apple's OSS GitHub. Early results are promising.

savetheclocktower commented 4 months ago

This turns out to be because we weren't on the specific version of libiconv that was necessary; now grabbing it directly from Apple's OSS GitHub. Early results are promising.

To clarify: this fixed the messages about encoding errors, but not the renderer crashes, which are all happening on Linux CI jobs. @DeeDeeG is going to push a PR that points to the commit that matches what's published in NPM as a sanity check.

savetheclocktower commented 4 months ago

Heartened by @DeeDeeG's experiment, I'm trying all my changes branched off of the version of the repo that's published to NPM.

savetheclocktower commented 4 months ago

OK, one last build with

Re-enabled the CirrusCI silicon build because this is really our best hope. If it works, we'll celebrate in our respective locations. Some of us will grill meats. (That was probably going to happen anyway, but somehow the meat will taste better if the build passes.)

savetheclocktower commented 4 months ago

Triumph. The only failure was the Windows binary, and investigation suggests I missed something in my cherry-picking. I'll pick this back up in a couple days.

savetheclocktower commented 4 months ago

Nope, it was just that I hadn't added the package.json change to the last commit. Now the Windows binary builds fine.

Next steps:

savetheclocktower commented 4 months ago

I'm troubleshooting why the silicon macOS build failed its Gatekeeper check in a local test on my partner's M1 laptop. As a sanity check, I've temporarily changed the CI config on this PR so that it will generate a signed Intel macOS binary; that will allow me to determine whether this issue has anything to do with Apple's stricter code-signing rules for ARM binaries than for Intel binaries. (I've once again disabled the CirrusCI job for Apple Silicon on this PR; I can enable it again later if I need to.)

My preferred outcome is that I discover the same code-signing issue exists with the Intel macOS binary; that would at least suggest that a fix exists, and that I could apply it universally and solve this problem all at once.

savetheclocktower commented 4 months ago

My preferred outcome is that I discover the same code-signing issue exists with the Intel macOS binary; that would at least suggest that a fix exists, and that I could apply it universally and solve this problem all at once.

My wish came true.

The most obscure way for your software to fail a Gatekeeper check (as far as I can tell) is if it wants to reference a .dylib with an install name that points to a non-existent path. I had solved that problem on superstring.node (which wants to reference libiconv.2.dylib and needed to be taught a relative-to-itself way of locating it), but building libiconv also produced a binary called iconv. That binary had an absolute reference to libiconv.2.dylib at a path that only meant something on the CI machine that compiled it.

If we needed that iconv binary for anything, we could fix this with install_name_tool. Since we don't, we can just delete the bin folder after compilation.

No code changes in this last commit; just reverting to the ordinary behavior on GitHub Actions (so that signed builds will once again only be produced when we land to master) and temporarily triggering another build on Apple Silicon — hopefully for the very last time. (Considering that the actual binary worked just fine on an Apple Silicon machine once you bypassed Gatekeeper, I'm quite confident that we'll have solved this thing once and for all.)

savetheclocktower commented 4 months ago

Now that I've gotten confirmation from @meadowsys that we've produced a working Pulsar binary that does not show a scary Gatekeeper dialog… I'm going to close this PR.

Next steps: