apply suggested changes

expo · Oct 17, 2024 · b14f074 · b14f074
1 parent f361373
commit b14f074
Show file tree

Hide file tree

Showing 7 changed files with 51 additions and 27 deletions.
diff --git a/packages/build-tools/src/builders/common.ts b/packages/build-tools/src/builders/common.ts
@@ -31,12 +31,10 @@ export async function runBuilderWithHooksAsync<T extends BuildJob>(
         });
       });
 
-      if (
-        ctx.job.platform === Platform.IOS &&
-        (!buildSuccess || (buildSuccess && ctx.shouldUploadXcodeBuildLogsOnSuccess))
-      ) {
+      if (ctx.job.platform === Platform.IOS) {
         await findAndUploadXcodeBuildLogsAsync(ctx as BuildContext<Ios.Job>, {
           logger: ctx.logger,
+          runStatus: buildSuccess ? 'success' : 'errored',
         });
       }
 

diff --git a/packages/build-tools/src/builders/custom.ts b/packages/build-tools/src/builders/custom.ts
@@ -56,20 +56,29 @@ export async function runCustomBuildAsync(ctx: BuildContext<BuildJob>): Promise<
     }
   });
   try {
-    try {
-      await workflow.executeAsync();
-    } finally {
-      if (!ctx.artifacts.XCODE_BUILD_LOGS && ctx.job.platform === Platform.IOS) {
-        try {
-          await findAndUploadXcodeBuildLogsAsync(ctx as BuildContext<Ios.Job>, {
-            logger: ctx.logger,
-          });
-        } catch {
-          // do nothing, it's a non-breaking error.
-        }
+    await workflow.executeAsync();
+
+    if (!ctx.artifacts.XCODE_BUILD_LOGS && ctx.job.platform === Platform.IOS) {
+      try {
+        await findAndUploadXcodeBuildLogsAsync(ctx as BuildContext<Ios.Job>, {
+          logger: ctx.logger,
+          runStatus: 'success',
+        });
+      } catch {
+        // do nothing, it's a non-breaking error.
       }
     }
   } catch (err: any) {
+    if (!ctx.artifacts.XCODE_BUILD_LOGS && ctx.job.platform === Platform.IOS) {
+      try {
+        await findAndUploadXcodeBuildLogsAsync(ctx as BuildContext<Ios.Job>, {
+          logger: ctx.logger,
+          runStatus: 'errored',
+        });
+      } catch {
+        // do nothing, it's a non-breaking error.
+      }
+    }
     err.artifacts = ctx.artifacts;
     throw err;
   }

diff --git a/packages/build-tools/src/context.ts b/packages/build-tools/src/context.ts
@@ -38,9 +38,14 @@ export interface LogBuffer {
 
 export type ArtifactToUpload =
   | {
-      type: ManagedArtifactType;
+      type: ManagedArtifactType.APPLICATION_ARCHIVE | ManagedArtifactType.BUILD_ARTIFACTS;
       paths: string[];
     }
+  | {
+      type: ManagedArtifactType.XCODE_BUILD_LOGS;
+      paths: string[];
+      runStatus: 'success' | 'errored';
+    }
   | {
       type: GenericArtifactType;
       key: string;
@@ -62,7 +67,6 @@ export interface BuildContextOptions {
   reportBuildPhaseStats?: (stats: BuildPhaseStats) => void;
   skipNativeBuild?: boolean;
   metadata?: Metadata;
-  shouldUploadXcodeBuildLogsOnSuccess?: boolean;
 }
 
 export class SkipNativeBuildError extends Error {}
@@ -78,7 +82,6 @@ export class BuildContext<TJob extends Job = Job> {
     options?: { tags?: Record<string, string>; extras?: Record<string, string> }
   ) => void;
   public readonly skipNativeBuild?: boolean;
-  public readonly shouldUploadXcodeBuildLogsOnSuccess: boolean;
   public artifacts: Artifacts = {};
 
   private _env: Env;
@@ -100,7 +103,6 @@ export class BuildContext<TJob extends Job = Job> {
     this.cacheManager = options.cacheManager;
     this._uploadArtifact = options.uploadArtifact;
     this.reportError = options.reportError;
-    this.shouldUploadXcodeBuildLogsOnSuccess = options.shouldUploadXcodeBuildLogsOnSuccess ?? true; // default to true if not provided
 
     const shouldApplyRepackOverrides = job.platform && job.mode === BuildMode.REPACK;
     this._job = {

diff --git a/packages/build-tools/src/ios/xcodeBuildLogs.ts b/packages/build-tools/src/ios/xcodeBuildLogs.ts
@@ -9,7 +9,7 @@ import { BuildContext } from '../context';
 
 export async function findAndUploadXcodeBuildLogsAsync(
   ctx: BuildContext<Ios.Job>,
-  { logger }: { logger: bunyan }
+  { logger, runStatus }: { logger: bunyan; runStatus: 'success' | 'errored' }
 ): Promise<void> {
   try {
     const xcodeBuildLogsPath = await findXcodeBuildLogsPathAsync(ctx.buildLogsDirectory);
@@ -18,6 +18,7 @@ export async function findAndUploadXcodeBuildLogsAsync(
         artifact: {
           type: ManagedArtifactType.XCODE_BUILD_LOGS,
           paths: [xcodeBuildLogsPath],
+          runStatus,
         },
         logger,
       });

diff --git a/packages/build-tools/src/steps/functions/findAndUploadBuildArtifacts.ts b/packages/build-tools/src/steps/functions/findAndUploadBuildArtifacts.ts
@@ -148,6 +148,7 @@ async function uploadXcodeBuildLogs({
     artifact: {
       type: ManagedArtifactType.XCODE_BUILD_LOGS,
       paths: [xcodeBuildLogsPath],
+      runStatus: global.hasAnyPreviousStepFailed ? 'errored' : 'success',
     },
     logger,
   });

diff --git a/packages/build-tools/src/steps/functions/uploadArtifact.ts b/packages/build-tools/src/steps/functions/uploadArtifact.ts
@@ -92,11 +92,19 @@ export function createUploadArtifactBuildFunction(ctx: CustomBuildContext): Buil
         throw result.reason;
       });
 
-      const artifact = {
-        type: parseArtifactTypeInput(`${inputs.type.value}`),
-        paths: artifactPaths,
-        key: inputs.key.value as string,
-      };
+      const artifactType = parseArtifactTypeInput(`${inputs.type.value}`);
+      const artifact =
+        artifactType === ManagedArtifactType.XCODE_BUILD_LOGS
+          ? ({
+              type: artifactType,
+              paths: artifactPaths,
+              runStatus: global.hasAnyPreviousStepFailed ? 'errored' : 'success',
+            } as const)
+          : {
+              type: artifactType,
+              paths: artifactPaths,
+              key: inputs.key.value as string,
+            };
 
       try {
         await ctx.runtimeApi.uploadArtifact({

diff --git a/packages/local-build-plugin/src/ios.ts b/packages/local-build-plugin/src/ios.ts
@@ -1,4 +1,4 @@
-import { Ios, BuildPhase, Env, isManagedArtifact } from '@expo/eas-build-job';
+import { Ios, BuildPhase, Env, isManagedArtifact, ManagedArtifactType } from '@expo/eas-build-job';
 import { Builders, BuildContext, Artifacts } from '@expo/build-tools';
 import omit from 'lodash/omit';
 
@@ -24,6 +24,12 @@ export async function buildIosAsync(
     logBuffer,
     uploadArtifact: async ({ artifact, logger }) => {
       if (isManagedArtifact(artifact)) {
+        if (
+          artifact.type === ManagedArtifactType.XCODE_BUILD_LOGS &&
+          artifact.runStatus !== 'errored'
+        ) {
+          return null;
+        }
         return await prepareArtifacts({
           artifactPaths: artifact.paths,
           logger,
@@ -36,7 +42,6 @@ export async function buildIosAsync(
     env,
     metadata,
     skipNativeBuild: config.skipNativeBuild,
-    shouldUploadXcodeBuildLogsOnSuccess: false,
   });
 
   await ctx.runBuildPhase(BuildPhase.START_BUILD, async () => {