[eas-build-job] add channel to Job #24

jkhales · 2021-05-12T20:39:29Z

Why

In order to support the new EAS version of expo-udpates, we need to be able to set the channel of a build in the same way we set the releaseChannel for classic expo-updates.

[edit]
A bit more context for posterity:

Q: "why can't we use releaseChannel for everything?"
A:

In EAS Update, the name is channel and while it is similar to releaseChannel it is different. In modern updates, we link a build with a channel, publish to a branch and then point a channel at a branch. In classic Updates, builds and publishes were both connected to releaseChannel
We write releaseChannel and channel to different Plist elements:
releaseChannel https://github.com/expo/expo/blob/master/packages/expo-updates/ios/EXUpdates/EXUpdatesConfig.m#L29
channel is saved as a value in a requestHeaders object and then saved here: https://github.com/expo/expo/blob/master/packages/expo-updates/ios/EXUpdates/EXUpdatesConfig.m#L28

How

added channel where ever I saw releaseChannel.

Put in an oxor that only allows "neither", "channel", or "releaseChannel" to be allowed.

Test Plan

yarn test

wkozyra95 · 2021-05-13T10:01:35Z

In terms of adding this field, it's everything ok, but you are not doing anything with it, so the question is what it should do?

If this is just a replacement for releaseChannel then you can just remove it and we will handle old reqests in API.

If you are planning to add in the future other stuff than a channel for updates I would suggest wrapping that into sth else
e.g { updates: { channel: string } }

If you are planning to use this as a flag to switch between update services (old OTA vs EAS Update), then it's fine as long as it's a temporary solution and releaseChannel will be deprecated.

wkozyra95

adding this comment just to mark PR as reviewed :D

jkhales · 2021-05-13T12:59:44Z

If you are planning to use this as a flag to switch between update services (old OTA vs EAS Update), then it's fine as long as it's a temporary solution and releaseChannel will be deprecated.

yes this is the situation, but classic updates will be supported indefinitely, even though there is going to be a very strong recommendation to use modern EAS Updates.

If you are planning to add in the future other stuff than a channel for updates I would suggest wrapping that into sth else
e.g { updates: { channel: string } }

Good suggestion, wrapped.

I left releaseChannel outside of the updates object as there will be another PR renaming it (as we discussed in slack) and I want to minimize the number of potentially breaking changes.

jkhales · 2021-05-21T17:16:26Z

sorry, did pushed by accident, please don't re-review! 🙏

jkhales · 2021-05-22T15:06:32Z

OK, so this is a bigger PR

You've already reviewed eas-build-job/*
A lot of it is renaming/refactoring.

I would suggest looking at packages/build-tools/src/utils/expoUpdates.ts first.

It replaces packages/build-tools/src/generic/expoUpdates.ts and packages/build-tools/src/managed/expoUpdates.ts, the only difference between these two files was that the generic version had an additional attempt to see if the releaseChannel was defined natively.

The other change to note it that packages/build-tools/src/*/releaseChannel.ts was change to the more general packages/build-tools/src/*/expoUpdates.ts

testing

I added a bunch of unit tests and did a sequence of builds with different parameters on both iOS and android.
In order to get this to work I had to make the following changes to my local eas-cli https://github.com/expo/eas-cli/tree/%40jkhales/companion-test-branch (will tidy this up and turn it into a PR next)

I confirmed that classic updates still worked by publishing an update.

eas updates needs the update URL to be changed as well, so I was not able to do and e2e test yet. I limited my testing of it to just checking the plist/manifest in the working directory after setting EAS_LOCAL_BUILD_SKIP_CLEANUP. Since this is not being used yet this partial testing seems safe to me.

jkhales · 2021-05-22T15:08:40Z

packages/build-tools/package.json

@@ -35,8 +35,7 @@
    "node-forge": "^0.9.1",
    "nullthrows": "^1.1.1",
    "plist": "^3.0.1",
-    "uuid": "^3.3.3",
-    "xml2js": "^0.4.23"


I replaced the manual xml2js usage in packages/build-tools/src/android/releaseChannel.ts with calls to methods in @expo/config-plugins.AndroidConfig.Manifest.* in packages/build-tools/src/android/expoUpdates.ts

jkhales · 2021-05-22T15:13:57Z

packages/build-tools/package.json

@@ -34,9 +34,7 @@
    "fs-extra": "^9.0.0",
    "node-forge": "^0.9.1",
    "nullthrows": "^1.1.1",
-    "plist": "^3.0.1",
-    "uuid": "^3.3.3",


uuid wasn't being used.

wkozyra95 · 2021-05-24T07:41:46Z

packages/build-tools/src/utils/expoUpdates.ts

+  switch (true) {
+    case !!ctx.job.updates?.channel: {
+      await configureEASExpoUpdatesAsync(ctx, platform);
+      return;
+    }
+    default: {
+      await configureClassicExpoUpdatesAsync(ctx, platform);
+    }
+  }


Suggested change

switch (true) {

case !!ctx.job.updates?.channel: {

await configureEASExpoUpdatesAsync(ctx, platform);

return;

}

default: {

await configureClassicExpoUpdatesAsync(ctx, platform);

}

}

if (ctx.job.updates?.channel) {

await configureEASExpoUpdatesAsync(ctx, platform);

} else {

await configureClassicExpoUpdatesAsync(ctx, platform);

}

or make the first condition based on the existence of updates field instead, this way we can use the default channel without specifying it, it may be useful if we would switch default channel name in the fututre

We don't have the 'default' workflow figured out yet. It will likely be something like 'main' instead, but it will be subtly different than before and would prefer to hold off on making assumptions here till we have settled in.

(changed to if/else)

packages/build-tools/src/android/expoUpdates.ts

packages/build-tools/src/utils/expoUpdates.ts

wkozyra95 · 2021-05-24T07:55:45Z

packages/build-tools/src/android/expoUpdates.ts

+  await AndroidConfig.Manifest.writeAndroidManifestAsync(manifestPath, androidManifest);
+}
+
+export const androidSetReleaseChannelNativelyAsync = async (


androidSetClasicChannelNativelyAsync or androidSetLegacyChannelNativelyAsync or sth simliair that mekes it more clear, distinction between realeasChannel and channel is often hard to spot.

When we switch to eas updates old key in eas.json will also have to be renamed this way to

Suggested change

export const androidSetReleaseChannelNativelyAsync = async (

export async function androidSetReleaseChannelNativelyAsync(

packages/build-tools/src/ios/expoUpdates.ts

packages/build-tools/src/android/expoUpdates.ts

packages/build-tools/src/builders/androidManaged.ts

wkozyra95 · 2021-05-24T08:08:24Z

packages/build-tools/src/utils/expoUpdates.ts

+  ctx: ManagedBuildContext<ManagedJob> | BuildContext<GenericJob>,
+  platform: Platform
+): Promise<void> => {
+  await setChannelNativelyAsync(ctx, platform);


What about managed workflow? By default, eject will configure it for classic updates, what should be responsible for updating update url expo eject or this code?

The update URL is set dynamically in a bunch of different places.

I am planning on reworking the URL setting functions where they live.

It's likely going to be a complex PR and so I am planning on doing it in a follow up.

Once those are both landed it maybe worth considering a 3rd PR to move the code closer together, but I am not yet sure that makes sense so would prefer to wait till the first 2 have landed to decide.

dsokal · 2021-05-24T09:28:57Z

packages/eas-build-job/src/metadata.ts

+   * Channel (for Expo Updates when it is configured for for use with EAS)
+   * It's undefined if the expo-updates package is not configured for use with EAS.
+   */
+  channel?: string;


If you want to add the new channel to build metadata, you should also update www and eas-cli.

See those two PRs if have a better understanding what needs to be done:

https://github.com/expo/universe/pull/7597

add appBuildVersion to build metadata eas-cli#413

Thank you, I will consult these when I work on those PR's

packages/eas-build-job/src/__tests__/ios.test.ts

packages/eas-build-job/src/__tests__/android.test.ts

packages/build-tools/src/__mocks__/fs.ts

packages/build-tools/src/utils/expoUpdates.ts

packages/build-tools/src/ios/expoUpdates.ts

packages/build-tools/src/android/expoUpdates.ts

Co-authored-by: Wojciech Kozyra <wojciech.kozyra@swmansion.com> Co-authored-by: Dominik Sokal <dominik.sokal@swmansion.com>

jkhales requested a review from wkozyra95 May 12, 2021 21:28

jkhales marked this pull request as ready for review May 13, 2021 00:45

wkozyra95 reviewed May 13, 2021

View reviewed changes

jkhales force-pushed the @jkhales/add-channel-fields branch from 44dd2e6 to 7825f01 Compare May 13, 2021 13:04

jkhales requested a review from wkozyra95 May 13, 2021 13:15

Jonathan 小林 Hales added 12 commits May 21, 2021 10:15

[eas-build-job] add channel to Job

1f1fd7e

testse

f619f4a

add oxor

ca1ca13

refactor to umbrella 'updates' object

912ec74

fix lint

b22b560

metadata schema

1795157

metadata test

d95347a

remove nesting and validation from metadata

d41e809

add native mutations

d4f041d

refactore to platform tests

c0e4f9a

remove stray logging

324e14e

fix ios plist names

b038cb2

jkhales force-pushed the @jkhales/add-channel-fields branch from 5dbca87 to b038cb2 Compare May 21, 2021 17:15

Test User added 3 commits May 22, 2021 06:51

add assert

3ff6a41

clean up

5fd42d7

fix lint

1f47d19

jkhales commented May 22, 2021

View reviewed changes

remove uuid

803d789

jkhales commented May 22, 2021

View reviewed changes

wkozyra95 reviewed May 24, 2021

View reviewed changes

wkozyra95 requested a review from dsokal May 24, 2021 08:09

dsokal reviewed May 24, 2021

View reviewed changes

jkh and others added 12 commits May 24, 2021 19:16

Apply suggestions from code review

1a637d1

Co-authored-by: Wojciech Kozyra <wojciech.kozyra@swmansion.com> Co-authored-by: Dominik Sokal <dominik.sokal@swmansion.com>

use getAndroidManifestAsync

300f55e

rename to ...classic...

9b70011

simplify typing

eb3b79e

remove redundant platform parameter

75cf095

use IOSConfig

c1e7bab

remove switch

e4aa8c2

remove undefined

bccfc34

remove assertion

d3327c1

rename more classic

fb60fc2

fix tests

eb8333b

fix tests

c24eee8

dsokal approved these changes May 25, 2021

View reviewed changes

dsokal requested a review from wkozyra95 May 25, 2021 09:36

wkozyra95 approved these changes May 25, 2021

View reviewed changes

jkhales merged commit bf11421 into main May 26, 2021

jkhales deleted the @jkhales/add-channel-fields branch May 26, 2021 16:31

jkhales mentioned this pull request Jun 16, 2021

[eas-cli][eas-json] add updates.channel to build metadata expo/eas-cli#461

Merged

2 tasks

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[eas-build-job] add channel to Job #24

[eas-build-job] add channel to Job #24

jkhales commented May 12, 2021 •

edited

wkozyra95 commented May 13, 2021

wkozyra95 left a comment

jkhales commented May 13, 2021

jkhales commented May 21, 2021

jkhales commented May 22, 2021

jkhales May 22, 2021

jkhales May 22, 2021

wkozyra95 May 24, 2021

jkhales May 25, 2021

jkhales May 25, 2021

wkozyra95 May 24, 2021

wkozyra95 May 24, 2021

wkozyra95 May 24, 2021

jkhales May 25, 2021

dsokal May 24, 2021

jkhales May 25, 2021

	export const androidSetReleaseChannelNativelyAsync = async (
	export async function androidSetReleaseChannelNativelyAsync(

[eas-build-job] add channel to Job #24

[eas-build-job] add channel to Job #24

Conversation

jkhales commented May 12, 2021 • edited

Why

How

Test Plan

wkozyra95 commented May 13, 2021

wkozyra95 left a comment

Choose a reason for hiding this comment

jkhales commented May 13, 2021

jkhales commented May 21, 2021

jkhales commented May 22, 2021

testing

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

jkhales commented May 12, 2021 •

edited