[steps] fix resolving custom function path (#259)

* [steps] fix resolving custom function path

* fix tests

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

@@ -21,10 +21,11 @@ export async function runCustomBuildAsync<T extends Job>(ctx: BuildContext<T>):
     relativeConfigPath
   );
 
-  const globalContext = new BuildStepGlobalContext(customBuildCtx, false, configPath);
+  const globalContext = new BuildStepGlobalContext(customBuildCtx, false);
   const easFunctions = getEasFunctions(customBuildCtx, ctx);
   const parser = new BuildConfigParser(globalContext, {
     externalFunctions: easFunctions,
+    configPath,
   });
   const workflow = await ctx.runBuildPhase(BuildPhase.PARSE_CUSTOM_WORKFLOW_CONFIG, async () => {
     try {

packages/steps/src/BuildConfig.ts

@@ -270,6 +270,16 @@ export async function readAndValidateBuildConfigAsync(
   const rawConfig = await readRawBuildConfigAsync(configPath);
 
   const config = validateConfig(BuildConfigSchema, rawConfig);
+  for (const functionName in config.functions) {
+    const customFunctionPath = config.functions[functionName].path;
+    if (customFunctionPath) {
+      config.functions[functionName].path = maybeResolveCustomFunctionRelativePath(
+        path.dirname(configPath),
+        customFunctionPath
+      );
+    }
+  }
+
   const importedFunctions = await importFunctionsAsync(configPath, config.configFilesToImport);
   mergeConfigWithImportedFunctions(config, importedFunctions);
   validateAllFunctionsExist(config, params);
@@ -302,13 +312,17 @@ async function importFunctionsAsync(
     visitedFiles.add(childConfigPath);
     try {
       const childConfig = await readAndValidateBuildFunctionsConfigFileAsync(childConfigPath);
+      const childDir = path.dirname(childConfigPath);
       for (const functionName in childConfig.functions) {
         if (!(functionName in importedFunctions)) {
-          importedFunctions[functionName] = childConfig.functions[functionName];
+          const f = childConfig.functions[functionName];
+          if (f.path) {
+            f.path = maybeResolveCustomFunctionRelativePath(childDir, f.path);
+          }
+          importedFunctions[functionName] = f;
         }
       }
       if (childConfig.configFilesToImport) {
-        const childDir = path.dirname(childConfigPath);
         configFilesToVisit.push(
           ...childConfig.configFilesToImport.map((relativePath) =>
             path.resolve(childDir, relativePath)
@@ -426,3 +440,10 @@ export function validateAllFunctionsExist(
     );
   }
 }
+
+function maybeResolveCustomFunctionRelativePath(dir: string, customFunctionPath: string): string {
+  if (!path.isAbsolute(customFunctionPath)) {
+    return path.resolve(dir, customFunctionPath);
+  }
+  return customFunctionPath;
+}

packages/steps/src/BuildConfigParser.ts

@@ -1,5 +1,4 @@
 import assert from 'assert';
-import path from 'path';
 
 import {
   BuildConfig,
@@ -34,19 +33,21 @@ import { duplicates } from './utils/expodash/duplicates.js';
 import { uniq } from './utils/expodash/uniq.js';
 
 export class BuildConfigParser {
+  private readonly configPath: string;
   private readonly externalFunctions?: BuildFunction[];
 
   constructor(
     private readonly ctx: BuildStepGlobalContext,
-    { externalFunctions }: { externalFunctions?: BuildFunction[] }
+    { configPath, externalFunctions }: { configPath: string; externalFunctions?: BuildFunction[] }
   ) {
     this.validateExternalFunctions(externalFunctions);
 
+    this.configPath = configPath;
     this.externalFunctions = externalFunctions;
   }
 
   public async parseAsync(): Promise<BuildWorkflow> {
-    const config = await readAndValidateBuildConfigAsync(this.ctx.configPath, {
+    const config = await readAndValidateBuildConfigAsync(this.configPath, {
       externalFunctionIds: this.getExternalFunctionFullIds(),
     });
     const configBuildFunctions = this.createBuildFunctionsFromConfig(config.functions);
@@ -174,17 +175,12 @@ export class BuildConfigParser {
     shell,
     command,
     supportedRuntimePlatforms,
-    path: relativeCustomFunctionModulePath,
+    path: customFunctionModulePath,
   }: BuildFunctionConfig & { id: string }): BuildFunction {
     const inputProviders =
       inputsConfig && this.createBuildStepInputProvidersFromBuildFunctionInputs(inputsConfig);
     const outputProviders =
       outputsConfig && this.createBuildStepOutputProvidersFromBuildFunctionOutputs(outputsConfig);
-    let customFunctionModulePath = relativeCustomFunctionModulePath;
-    if (relativeCustomFunctionModulePath) {
-      const baseConfigDir = path.dirname(this.ctx.configPath);
-      customFunctionModulePath = path.resolve(baseConfigDir, relativeCustomFunctionModulePath);
-    }
     return new BuildFunction({
       id,
       name,

packages/steps/src/BuildStepContext.ts

@@ -45,7 +45,6 @@ export interface SerializedBuildStepGlobalContext {
   stepById: Record<string, SerializedBuildStepOutputAccessor>;
   provider: SerializedExternalBuildContextProvider;
   skipCleanup: boolean;
-  configPath: string;
 }
 
 export class BuildStepGlobalContext {
@@ -57,8 +56,7 @@ export class BuildStepGlobalContext {
 
   constructor(
     private readonly provider: ExternalBuildContextProvider,
-    public readonly skipCleanup: boolean,
-    public readonly configPath: string
+    public readonly skipCleanup: boolean
   ) {
     this.stepsInternalBuildDirectory = path.join(os.tmpdir(), 'eas-build', uuidv4());
     this.runtimePlatform = provider.runtimePlatform;
@@ -133,7 +131,6 @@ export class BuildStepGlobalContext {
         env: this.provider.env,
       },
       skipCleanup: this.skipCleanup,
-      configPath: this.configPath,
     };
   }
 
@@ -151,11 +148,7 @@ export class BuildStepGlobalContext {
       env: serialized.provider.env,
       updateEnv: () => {},
     };
-    const ctx = new BuildStepGlobalContext(
-      deserializedProvider,
-      serialized.skipCleanup,
-      serialized.configPath
-    );
+    const ctx = new BuildStepGlobalContext(deserializedProvider, serialized.skipCleanup);
     for (const [id, stepOutputAccessor] of Object.entries(serialized.stepById)) {
       ctx.stepById[id] = BuildStepOutputAccessor.deserialize(stepOutputAccessor);
     }

packages/steps/src/__tests__/BuildConfigParser-test.ts

@@ -19,10 +19,11 @@ const __dirname = url.fileURLToPath(new URL('.', import.meta.url));
 describe(BuildConfigParser, () => {
   describe('constructor', () => {
     it('throws if provided external functions with duplicated IDs', () => {
-      const ctx = createGlobalContextMock({ configPath: './fake.yml' });
+      const ctx = createGlobalContextMock();
       const error = getError<BuildStepRuntimeError>(() => {
         // eslint-disable-next-line no-new
         new BuildConfigParser(ctx, {
+          configPath: './fake.yml',
           externalFunctions: [
             new BuildFunction({ id: 'abc', command: 'echo 123' }),
             new BuildFunction({ id: 'abc', command: 'echo 456' }),
@@ -34,12 +35,11 @@ describe(BuildConfigParser, () => {
     });
 
     it(`doesn't throw if provided external functions don't have duplicated IDs`, () => {
-      const ctx = createGlobalContextMock({
-        configPath: path.join('./fake.yml'),
-      });
+      const ctx = createGlobalContextMock();
       expect(() => {
         // eslint-disable-next-line no-new
         new BuildConfigParser(ctx, {
+          configPath: './fake.yml',
           externalFunctions: [
             new BuildFunction({ namespace: 'a', id: 'abc', command: 'echo 123' }),
             new BuildFunction({ namespace: 'b', id: 'abc', command: 'echo 456' }),
@@ -51,19 +51,19 @@ describe(BuildConfigParser, () => {
 
   describe(BuildConfigParser.prototype.parseAsync, () => {
     it('returns a BuildWorkflow object', async () => {
-      const ctx = createGlobalContextMock({
+      const ctx = createGlobalContextMock();
+      const parser = new BuildConfigParser(ctx, {
         configPath: path.join(__dirname, './fixtures/build.yml'),
       });
-      const parser = new BuildConfigParser(ctx, {});
       const result = await parser.parseAsync();
       expect(result).toBeInstanceOf(BuildWorkflow);
     });
 
     it('parses steps from the build workflow', async () => {
-      const ctx = createGlobalContextMock({
+      const ctx = createGlobalContextMock();
+      const parser = new BuildConfigParser(ctx, {
         configPath: path.join(__dirname, './fixtures/build.yml'),
       });
-      const parser = new BuildConfigParser(ctx, {});
       const workflow = await parser.parseAsync();
       const buildSteps = workflow.buildSteps;
       expect(buildSteps.length).toBe(6);
@@ -150,10 +150,10 @@ describe(BuildConfigParser, () => {
     });
 
     it('parses inputs', async () => {
-      const ctx = createGlobalContextMock({
+      const ctx = createGlobalContextMock();
+      const parser = new BuildConfigParser(ctx, {
         configPath: path.join(__dirname, './fixtures/inputs.yml'),
       });
-      const parser = new BuildConfigParser(ctx, {});
       const workflow = await parser.parseAsync();
       const buildSteps = workflow.buildSteps;
       expect(buildSteps.length).toBe(1);
@@ -200,10 +200,10 @@ describe(BuildConfigParser, () => {
     });
 
     it('parses outputs', async () => {
-      const ctx = createGlobalContextMock({
+      const ctx = createGlobalContextMock();
+      const parser = new BuildConfigParser(ctx, {
         configPath: path.join(__dirname, './fixtures/outputs.yml'),
       });
-      const parser = new BuildConfigParser(ctx, {});
       const workflow = await parser.parseAsync();
       const buildSteps = workflow.buildSteps;
       expect(buildSteps.length).toBe(2);
@@ -255,10 +255,10 @@ describe(BuildConfigParser, () => {
     });
 
     it('parses functions and function calls', async () => {
-      const ctx = createGlobalContextMock({
+      const ctx = createGlobalContextMock();
+      const parser = new BuildConfigParser(ctx, {
         configPath: path.join(__dirname, './fixtures/functions.yml'),
       });
-      const parser = new BuildConfigParser(ctx, {});
       const workflow = await parser.parseAsync();
 
       const { buildSteps } = workflow;
@@ -532,10 +532,10 @@ describe(BuildConfigParser, () => {
     });
 
     it('throws if calling non-existent external functions', async () => {
-      const ctx = createGlobalContextMock({
+      const ctx = createGlobalContextMock();
+      const parser = new BuildConfigParser(ctx, {
         configPath: path.join(__dirname, './fixtures/external-functions.yml'),
       });
-      const parser = new BuildConfigParser(ctx, {});
       const error = await getErrorAsync<BuildConfigError>(async () => {
         await parser.parseAsync();
       });
@@ -546,9 +546,7 @@ describe(BuildConfigParser, () => {
     });
 
     it('works with external functions', async () => {
-      const ctx = createGlobalContextMock({
-        configPath: path.join(__dirname, './fixtures/external-functions.yml'),
-      });
+      const ctx = createGlobalContextMock();
 
       const downloadProjectFn: BuildStepFunction = (ctx) => {
         ctx.logger.info('Downloading project...');
@@ -559,6 +557,7 @@ describe(BuildConfigParser, () => {
       };
 
       const parser = new BuildConfigParser(ctx, {
+        configPath: path.join(__dirname, './fixtures/external-functions.yml'),
         externalFunctions: [
           new BuildFunction({
             namespace: 'eas',

packages/steps/src/__tests__/BuildStepContext-test.ts

@@ -22,8 +22,7 @@ describe(BuildStepGlobalContext, () => {
           '/another/non/existent/path',
           '/working/dir/path'
         ),
-        false,
-        './fake.yml'
+        false
       );
       expect(ctx.stepsInternalBuildDirectory.startsWith(os.tmpdir())).toBe(true);
     });
@@ -39,8 +38,7 @@ describe(BuildStepGlobalContext, () => {
           '/another/non/existent/path',
           workingDirectory
         ),
-        false,
-        './fake.yml'
+        false
       );
       expect(ctx.defaultWorkingDirectory).toBe(workingDirectory);
     });
@@ -60,7 +58,6 @@ describe(BuildStepGlobalContext, () => {
         projectTargetDirectory: '/d/e/f',
         workingDirectory: '/g/h/i',
         staticContextContent: { a: 1 },
-        configPath: '/j/k/l',
       });
       expect(ctx.serialize()).toEqual(
         expect.objectContaining({
@@ -75,7 +72,6 @@ describe(BuildStepGlobalContext, () => {
             env: {},
           },
           skipCleanup: true,
-          configPath: '/j/k/l',
         })
       );
     });
@@ -95,21 +91,18 @@ describe(BuildStepGlobalContext, () => {
             env: {},
           },
           skipCleanup: true,
-          configPath: '/j/k/l',
         },
         createMockLogger()
       );
       expect(ctx.stepsInternalBuildDirectory).toBe('/m/n/o');
       expect(ctx.defaultWorkingDirectory).toBe('/g/h/i');
       expect(ctx.runtimePlatform).toBe(BuildRuntimePlatform.DARWIN);
       expect(ctx.skipCleanup).toBe(true);
-      expect(ctx.configPath).toBe('/j/k/l');
       expect(ctx.projectSourceDirectory).toBe('/a/b/c');
       expect(ctx.projectTargetDirectory).toBe('/d/e/f');
       expect(ctx.staticContext).toEqual({ a: 1 });
       expect(ctx.env).toEqual({});
       expect(ctx.skipCleanup).toBe(true);
-      expect(ctx.configPath).toBe('/j/k/l');
     });
   });
   describe(BuildStepGlobalContext.prototype.getStepOutputValue, () => {

packages/steps/src/__tests__/utils/context.ts

@@ -45,7 +45,6 @@ interface BuildContextParams {
   projectTargetDirectory?: string;
   workingDirectory?: string;
   staticContextContent?: Record<string, any>;
-  configPath?: string;
 }
 
 export function createStepContextMock({
@@ -82,7 +81,6 @@ export function createGlobalContextMock({
   projectTargetDirectory,
   workingDirectory,
   staticContextContent,
-  configPath,
 }: BuildContextParams = {}): BuildStepGlobalContext {
   const resolvedProjectTargetDirectory =
     projectTargetDirectory ?? path.join(os.tmpdir(), 'eas-build', uuidv4());
@@ -95,7 +93,6 @@ export function createGlobalContextMock({
       workingDirectory ?? resolvedProjectTargetDirectory,
       staticContextContent ?? {}
     ),
-    skipCleanup ?? false,
-    configPath ?? './fake.yml'
+    skipCleanup ?? false
   );
 }

packages/steps/src/cli/cli.ts

@@ -47,10 +47,11 @@ async function runAsync(
       relativeProjectDirectory,
       relativeProjectDirectory
     ),
-    false,
-    configPath
+    false
   );
-  const parser = new BuildConfigParser(ctx, {});
+  const parser = new BuildConfigParser(ctx, {
+    configPath,
+  });
   const workflow = await parser.parseAsync();
   await workflow.executeAsync();
 }