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

[local-build-plugin][build-tools][eas-build-job] copy Xcode build logs to target destination on local iOS build failure to simplify debuging #447

Draft
wants to merge 2 commits into
base: main
Choose a base branch
from
Draft
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions packages/build-tools/src/builders/common.ts
Original file line number Diff line number Diff line change
Expand Up @@ -34,6 +34,7 @@ export async function runBuilderWithHooksAsync<T extends BuildJob>(
if (ctx.job.platform === Platform.IOS) {
await findAndUploadXcodeBuildLogsAsync(ctx as BuildContext<Ios.Job>, {
logger: ctx.logger,
runStatus: buildSuccess ? 'success' : 'errored',
});
}

Expand Down
31 changes: 20 additions & 11 deletions packages/build-tools/src/builders/custom.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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;
}
Expand Down
7 changes: 6 additions & 1 deletion packages/build-tools/src/context.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down
3 changes: 2 additions & 1 deletion packages/build-tools/src/ios/xcodeBuildLogs.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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);
Expand All @@ -18,6 +18,7 @@ export async function findAndUploadXcodeBuildLogsAsync(
artifact: {
type: ManagedArtifactType.XCODE_BUILD_LOGS,
paths: [xcodeBuildLogsPath],
runStatus,
},
logger,
});
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -148,6 +148,7 @@ async function uploadXcodeBuildLogs({
artifact: {
type: ManagedArtifactType.XCODE_BUILD_LOGS,
paths: [xcodeBuildLogsPath],
runStatus: global.hasAnyPreviousStepFailed ? 'errored' : 'success',
},
logger,
});
Expand Down
18 changes: 13 additions & 5 deletions packages/build-tools/src/steps/functions/uploadArtifact.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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({
Expand Down
11 changes: 11 additions & 0 deletions packages/eas-build-job/src/artifacts.ts
Original file line number Diff line number Diff line change
Expand Up @@ -27,3 +27,14 @@ export const isGenericArtifact = <
}
return false;
};

export const isManagedArtifact = <
TSpec extends { type: GenericArtifactType | ManagedArtifactType },
>(
artifactSpec: TSpec
): artifactSpec is TSpec & { type: ManagedArtifactType } => {
if (Object.values(ManagedArtifactType).includes(artifactSpec.type as ManagedArtifactType)) {
return true;
}
return false;
};
Comment on lines +35 to +40
Copy link
Contributor

Choose a reason for hiding this comment

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

): artifactSpec is TSpec & { type: ManagedArtifactType } => Object.values(ManagedArtifactType).includes(artifactSpec.type as ManagedArtifactType)

?

12 changes: 7 additions & 5 deletions packages/local-build-plugin/src/android.ts
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
import { Android, ManagedArtifactType, BuildPhase, Env } from '@expo/eas-build-job';
import { Android, BuildPhase, Env, isManagedArtifact } from '@expo/eas-build-job';
import { Builders, BuildContext, Artifacts } from '@expo/build-tools';
import omit from 'lodash/omit';

Expand All @@ -23,10 +23,12 @@ export async function buildAndroidAsync(
logger,
logBuffer,
uploadArtifact: async ({ artifact, logger }) => {
if (artifact.type === ManagedArtifactType.APPLICATION_ARCHIVE) {
return await prepareArtifacts(artifact.paths, logger);
} else if (artifact.type === ManagedArtifactType.BUILD_ARTIFACTS) {
return await prepareArtifacts(artifact.paths, logger);
if (isManagedArtifact(artifact)) {
return await prepareArtifacts({
artifactPaths: artifact.paths,
logger,
artifactType: artifact.type,
});
} else {
return null;
}
Expand Down
26 changes: 23 additions & 3 deletions packages/local-build-plugin/src/artifacts.ts
Original file line number Diff line number Diff line change
Expand Up @@ -3,10 +3,19 @@ import path from 'path';
import { bunyan } from '@expo/logger';
import fs from 'fs-extra';
import * as tar from 'tar';
import { ManagedArtifactType } from '@expo/eas-build-job';

import config from './config';

export async function prepareArtifacts(artifactPaths: string[], logger?: bunyan): Promise<string> {
export async function prepareArtifacts({
artifactPaths,
artifactType,
logger,
}: {
artifactPaths: string[];
artifactType: ManagedArtifactType;
logger?: bunyan;
}): Promise<string> {
const l = logger?.child({ phase: 'PREPARE_ARTIFACTS' });
l?.info('Preparing artifacts');
let suffix;
Expand Down Expand Up @@ -35,10 +44,21 @@ export async function prepareArtifacts(artifactPaths: string[], logger?: bunyan)
suffix = '.tar.gz';
localPath = archivePath;
}
const artifactName = `build-${Date.now()}${suffix}`;
const artifactName =
artifactType === ManagedArtifactType.APPLICATION_ARCHIVE
? `build-${Date.now()}${suffix}`
: artifactType === ManagedArtifactType.BUILD_ARTIFACTS
? `artifacts-${Date.now()}${suffix}`
: `xcode-logs-${Date.now()}${suffix}`;
Copy link
Contributor

Choose a reason for hiding this comment

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

Same as below

const destPath = config.artifactPath ?? path.join(config.artifactsDir, artifactName);
await fs.copy(localPath, destPath);
l?.info({ phase: 'PREPARE_ARTIFACTS' }, `Writing artifacts to ${destPath}`);
if (artifactType === ManagedArtifactType.APPLICATION_ARCHIVE) {
l?.info({ phase: 'PREPARE_ARTIFACTS' }, `Writing application archive to ${destPath}`);
} else if (artifactType === ManagedArtifactType.BUILD_ARTIFACTS) {
l?.info({ phase: 'PREPARE_ARTIFACTS' }, `Writing build artifacts to ${destPath}`);
} else {
Copy link
Contributor

Choose a reason for hiding this comment

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

Prefer switch over enums. This way if we add more managed types we won't forget to add handling.

l?.info({ phase: 'PREPARE_ARTIFACTS' }, `Writing Xcode logs to ${destPath}`);
}
return destPath;
}

Expand Down
18 changes: 13 additions & 5 deletions packages/local-build-plugin/src/ios.ts
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
import { Ios, BuildPhase, Env, ManagedArtifactType } 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';

Expand All @@ -23,10 +23,18 @@ export async function buildIosAsync(
logger,
logBuffer,
uploadArtifact: async ({ artifact, logger }) => {
if (artifact.type === ManagedArtifactType.APPLICATION_ARCHIVE) {
return await prepareArtifacts(artifact.paths, logger);
} else if (artifact.type === ManagedArtifactType.BUILD_ARTIFACTS) {
return await prepareArtifacts(artifact.paths, logger);
if (isManagedArtifact(artifact)) {
if (
artifact.type === ManagedArtifactType.XCODE_BUILD_LOGS &&
artifact.runStatus !== 'errored'
) {
Copy link
Contributor

Choose a reason for hiding this comment

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

Would it make sense to just always upload Xcode build logs? It would simplify code greatly (no tracking of runStatus, uploading artifact in finally)

Copy link
Member Author

Choose a reason for hiding this comment

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

Maybe it would. Would it be ok for you as a user if they were always uploaded for you local builds (copied to project dir) 🤔

return null;
}
return await prepareArtifacts({
artifactPaths: artifact.paths,
logger,
artifactType: artifact.type,
});
} else {
return null;
}
Expand Down
Loading