Skip to content

Conversation

@savetheclocktower
Copy link
Contributor

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
Copy link
Contributor Author

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
Copy link
Contributor Author

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
Copy link
Contributor Author

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
Copy link
Contributor Author

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
Copy link
Contributor Author

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
Copy link
Contributor Author

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

…and enable CirrusCI for one more job.
@savetheclocktower
Copy link
Contributor Author

OK, one last build with

  • last known good superstring, plus
  • the changes needed for macOS with libiconv, plus
  • migrating win-iconv from a submodule to a vendor directory, plus
  • changes that only affect superstring’s CI.

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
Copy link
Contributor Author

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
Copy link
Contributor Author

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:

  • Decide whether to make this SHA on superstring the head of a special branch or to change the state of master to match it.
  • Once that's sorted, open a different PR that changes our superstring dependency from NPM to our pulsar-edit/superstring fork, along with the other minor changes that need to happen in CI.
  • If it's green, land it.

@savetheclocktower
Copy link
Contributor Author

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.

…and trigger an Apple Silicon binary (now that we seem to have figured this out)
@savetheclocktower
Copy link
Contributor Author

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
Copy link
Contributor Author

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:

  • Bring pulsar-edit/superstring#master into a state that it can be referenced in Pulsar's package.json; this would involve reverting the few changes that have been made to the actual superstring source code. (Or do the same on a named branch.)
  • At that point, a change to package.json (and yarn.lock) to point to our version of superstring is all that we'd need to get binaries building again. (We'd also want to roll back some of our CI changes; for instance, installing libiconv from Homebrew won't be necessary anymore.)

@savetheclocktower savetheclocktower deleted the test-superstring-in-ci-2 branch July 6, 2024 00:44
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.

2 participants