Page MenuHomePhabricator

[services] [electron update server] Patch hazel to work with desktop-vX.Y.Z tags
ClosedPublic

Authored by michal on Jan 23 2023, 11:25 AM.
Tags
None
Referenced Files
F1701888: D6354.diff
Sat, May 4, 6:49 PM
Unknown Object (File)
Fri, Apr 5, 12:01 AM
Unknown Object (File)
Fri, Apr 5, 12:01 AM
Unknown Object (File)
Fri, Apr 5, 12:01 AM
Unknown Object (File)
Fri, Apr 5, 12:00 AM
Unknown Object (File)
Thu, Apr 4, 11:54 PM
Unknown Object (File)
Feb 21 2024, 1:23 PM
Unknown Object (File)
Feb 21 2024, 10:45 AM
Subscribers

Details

Summary

Because we have both mobile and desktop releases in our github repo we need to add a small change to the Hazel electron update server so that it only looks for releases that have a git tag that looks like desktop-vX.Y.Z.

Depends on D6353

Test Plan

Check if hazel correctly looks for desktop-vX.Y.Z git tags and ignores other tags.

Diff Detail

Repository
rCOMM Comm
Lint
Lint Not Applicable
Unit
Tests Not Applicable

Event Timeline

services/electron-update-server/patches/hazel-server+5.1.1.patch
14 ↗(On Diff #21224)

Ignore GitHub releases that don't start with desktop-

22 ↗(On Diff #21224)

Strip the desktop- tag from the version when caching the release.

35 ↗(On Diff #21224)

When pointing to GitHub url we need to add the prefix.

michal retitled this revision from [electron update server] Path hazel to work with desktop-vX.Y.Z tags to [services] [electron update server] Patch hazel to work with desktop-vX.Y.Z tags.Jan 23 2023, 11:30 AM
ashoat requested changes to this revision.Jan 23 2023, 12:11 PM
ashoat added inline comments.
services/electron-update-server/package.json
16 ↗(On Diff #21224)

We handle patch-package at the root level

services/electron-update-server/patches/hazel-server+5.1.1.patch
1 ↗(On Diff #21224)

Let's move this to the root patches folder

This revision now requires changes to proceed.Jan 23 2023, 12:11 PM

Move the patch to the root level.

NOTE: patch-package is run only in the postinstall script of the native workspace. So if native built before electron-update-server the hazel patch wouldn't be applied. So we probably have two options: - add patch-package to root and call it in the postinstall - add a postinstall script in the update server that's cd ../.. && yarn patch-package

It's important that yarn patch-package runs before pod install, and the best way I've found to guarantee that is to include it in native/package.json. I don't think it's bad to call patch-package multiple times

NOTE: patch-package is run only in the postinstall script of the native workspace. So if native built before electron-update-server the hazel patch wouldn't be applied.

Can you clarify this scenario a bit more?

  • add patch-package to root and call it in the postinstall
  • add a postinstall script in the update server that's cd ../.. && yarn patch-package

I'm not sure, but I think either will work as long as we leave the patch-package call in native untouched.

This revision is now accepted and ready to land.Jan 24 2023, 2:52 PM

Can you clarify this scenario a bit more?

Something like this:

  1. yarn cleaninstall is run and everything is cleaned
  2. All native dependencies are installed
  3. postinstall in native is called (-> hazel is not patched because it doesn't exist in the node_modules yet)
  4. All electron-update-server dependencies are installed

In the end hazel is installed but isn't patched

I've made diff that adds patch-package to root in D6391

Can you clarify this scenario a bit more?

Something like this:

  1. yarn cleaninstall is run and everything is cleaned
  2. All native dependencies are installed
  3. postinstall in native is called (-> hazel is not patched because it doesn't exist in the node_modules yet)
  4. All electron-update-server dependencies are installed

In the end hazel is installed but isn't patched

Is this a hypothetical scenario, or were you able to observe it? I would've assumed that postinstall hooks won't run until all dependencies (for all workspaces) are installed

Is this a hypothetical scenario, or were you able to observe it?

I wasn't able to observe it because native takes much longer than electron-update-server to install but I've tested it by making a small repo with this structure:

  • workspace
    • workspace member A, that has one dependency depA
    • workspace member B, that has one dependency depB

(Neither depA not depB are members of the workspace).
I was able to get this order of postinstall scripts:

  1. depB
  2. B
  3. depA
  4. A
  5. workspace

So postinstall scripts run as soon as all dependencies were installed.