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

Bundle Saturn L2 node #34

Merged
merged 2 commits into from
Jun 8, 2022
Merged

Bundle Saturn L2 node #34

merged 2 commits into from
Jun 8, 2022

Conversation

bajtos
Copy link
Member

@bajtos bajtos commented Jun 6, 2022

  • Add a build step to download Saturn L2 Node binaries from project's GitHub Releases
  • Modify Electron Builder configuration to include Saturn L2 binary in packaged Resources
  • Add a dummy main script to print the path of the Saturn L2 Node binary and verify that the path exists

Close #19

@bajtos bajtos force-pushed the feat/bundled-saturn-l2-node branch 4 times, most recently from 73b7ac2 to ae7ef9e Compare June 6, 2022 14:07
@bajtos bajtos self-assigned this Jun 6, 2022
@bajtos bajtos added this to the M0 milestone Jun 6, 2022
@bajtos bajtos force-pushed the feat/bundled-saturn-l2-node branch from ae7ef9e to 71ece08 Compare June 6, 2022 14:11
@bajtos bajtos requested a review from lidel June 6, 2022 14:13
Copy link
Contributor

@lidel lidel left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

  • 🟢 including binary inside of DMG should be fine
    • this is actually an easy hack to ensure your daemon binary is correctly notarized on macOS, even when upstream binary does not include signing/notarization metadata – as part of notarization step on CI everything inside of DMG gets hashed and "blessed" by Apple
  • ⚠️ fetching release artifacts will get rate-limited and your builds will fail (example) – i think you can fix this by using GITHUB_TOKEN provided by Github Action env (raises request limit to 1k / hours)

@bajtos
Copy link
Member Author

bajtos commented Jun 7, 2022

Thank you, @lidel! Your insights are always helpful 👏🏻

I have two more questions if you don't mind.

(1) In the current implementation, I am downloading release archives for all combinations of (platform, architecture). This would be useful if we were cross-compiling for multiple platforms from the same source tree. However, IIUC, our build setup is always building for the current platform & architecture only. That means we can download only one release archive. Is it a reasonable assumption?

(2) Do you happen to know what's the story for building Electron for M1 Macs?

  • I am working on a darwin-arm64 machine, building darwin-arm64 package works well locally.
  • The next step is to build darwin-arm64 as part of our GitHub Actions release workflow. I believe GHA does not offer darwin-arm64 runners yet, one has to setup a self-hosted runner. But then do we need that? Isn't it possible to build darwin-arm64 package on an x64 host?
  • Finally, there are two options how to package macOS applications - either provide two installers, one for x64 another for arm64, or create a universal app bundling both flavours into a single binary. To do that, we need to build both x64 & arm64 versions from the same source tree, which brings me back to the first question whether it's ok to download only one release archive of saturn-l2.

Support for darwin-arm64 is most likely out of scope of MVP, I created a new GH issue for that: #35

@bajtos
Copy link
Member Author

bajtos commented Jun 7, 2022

That means we can download only one release archive. Is it a reasonable assumption?

Based on what I found regarding Apple M1 builds, I am leaning towards downloading all architectures (x64, arm64) for the current platform (e.g. darwin).

@bajtos
Copy link
Member Author

bajtos commented Jun 7, 2022

In 8b9208e, I reworked the implementation to use GITHUB_TOKEN when calling GitHub and fetch release archives for the current platform only.

Using the dist files attached to GHA run, I verified on the following platforms that Station contains saturn-l2 binary that can be executed: MacOS M1, Windows x64.

@lidel
Copy link
Contributor

lidel commented Jun 7, 2022

  • Re (1) **binary per platform: ok **
    • we can download only one release archive. Is it a reasonable assumption?

      • yes, that would be ideal – in Brave, they fetch platform specific go-ipfs binary and run native ARM code on M1 Macs
      • in ipfs-desktop we use npm-go-ipfs which takes care of fetching correct binary from https://dist.ipfs.io/go-ipfs/ (windows/linux/darwin), but it recently got tricky when it comes to darwin (macOS), and afaik we are still using Intel binary for go-ipfs in DMG I think (notes below)
  • Re (2) macOS: apple silicon comes with caveats
    • I am leaning towards downloading all architectures (x64, arm64) for the current platform (e.g. darwin).

      • sounds like sensible approach
      • there is some prior art in feat: kubo 0.29 and native apple silicon ipfs/ipfs-desktop#1856 around producing "universal" DMG that has Electron for both Intel and Apple Silicon, but that PR is missing code that decides which version of go-ipfs binary should be used when Apple Silicon runtime is detected
        • in other words, you can produce universal DMG which will have both Intel and Apple Silicon versions of Electron, but you need to also include both versions of all additional binaries you embedd inside of DMG and have code that decides which on e should be run
      • NOTE: if you don't use Universal DMG, or if you have universal DMG but only with two versions of electron, but only Intel version of daemon binary, then it will still work on Apple Silicon (M1) but it will require Rosetta (which could be missing, and cause issues like this)

@bajtos bajtos force-pushed the feat/bundled-saturn-l2-node branch 2 times, most recently from 89f3e91 to 0f1d220 Compare June 8, 2022 06:39
@bajtos bajtos force-pushed the feat/bundled-saturn-l2-node branch from 0f1d220 to d0d3195 Compare June 8, 2022 06:47
@bajtos bajtos merged commit 7cad143 into main Jun 8, 2022
@bajtos bajtos deleted the feat/bundled-saturn-l2-node branch June 8, 2022 07:02
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.

Bundled Saturn L2 Node
2 participants