Skip to content
Draft
Show file tree
Hide file tree
Changes from 3 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
70 changes: 69 additions & 1 deletion src/apphosting/localbuilds.spec.ts
Original file line number Diff line number Diff line change
@@ -1,9 +1,11 @@
import * as sinon from "sinon";
import { expect } from "chai";
import * as localBuildModule from "@apphosting/build";
import { localBuild } from "./localbuilds";
import { localBuild, runUniversalMaker } from "./localbuilds";
import * as secrets from "./secrets";
import { EnvMap } from "./yaml";
import * as childProcess from "child_process";
import * as fs from "fs";

describe("localBuild", () => {
afterEach(() => {
Expand All @@ -13,6 +15,7 @@ describe("localBuild", () => {
it("returns the expected output", async () => {
const bundleConfig = {
version: "v1" as const,

runConfig: {
runCommand: "npm run build:prod",
},
Expand Down Expand Up @@ -202,4 +205,69 @@ describe("localBuild", () => {
expect(confirmStub).to.have.been.calledOnce;
});
});

describe("runUniversalMaker", () => {
it("should successfully execute Universal Maker and parse output", () => {
process.env.UNIVERSAL_MAKER_BINARY = "/path/to/universal_maker";
const spawnStub = sinon
.stub(childProcess, "spawnSync")
.returns({} as unknown as childProcess.SpawnSyncReturns<string>);
Comment thread
falahat marked this conversation as resolved.
Outdated
sinon.stub(fs, "existsSync").returns(true);
const readFileSyncStub = sinon.stub(fs, "readFileSync").returns(
JSON.stringify({
command: "npm",
args: ["run", "start"],
language: "nodejs",
runtime: "nodejs22",
envVars: { PORT: 3000 },
}),
);

const output = runUniversalMaker("./", "nextjs");

expect(output).to.deep.equal({
metadata: {
language: "nodejs",
runtime: "nodejs22",
framework: "nextjs",
},
runConfig: {
runCommand: "npm run start",
environmentVariables: [{ variable: "PORT", value: "3000", availability: ["RUNTIME"] }],
},
outputFiles: {
serverApp: {
include: [".apphosting"],
},
},
});

sinon.assert.calledOnce(spawnStub);
sinon.assert.calledOnce(readFileSyncStub);
delete process.env.UNIVERSAL_MAKER_BINARY;
});

it("should raise clear FirebaseError when UNIVERSAL_MAKER_BINARY is undefined", () => {
delete process.env.UNIVERSAL_MAKER_BINARY;

expect(() => runUniversalMaker("./")).to.throw(
"Please specify the path to your Universal Maker binary by establishing the UNIVERSAL_MAKER_BINARY environment variable.",
);
});

it("should raise clear FirebaseError on permission errors within child execution", () => {
process.env.UNIVERSAL_MAKER_BINARY = "/path/to/universal_maker";
sinon.stub(childProcess, "spawnSync").callsFake(() => {
const err = new Error("EACCES exception") as NodeJS.ErrnoException;
err.code = "EACCES";

throw err;
});

expect(() => runUniversalMaker("./")).to.throw(
"Failed to execute the Universal Maker binary due to permission constraints. Please assure you have set chmod +x on your file.",
);
delete process.env.UNIVERSAL_MAKER_BINARY;
});
});
});
146 changes: 143 additions & 3 deletions src/apphosting/localbuilds.ts
Original file line number Diff line number Diff line change
@@ -1,14 +1,144 @@
import * as childProcess from "child_process";
import * as fs from "fs";
import * as path from "path";
import { BuildConfig, Env } from "../gcp/apphosting";
import { localBuild as localAppHostingBuild } from "@apphosting/build";
import { EnvMap } from "./yaml";
import { loadSecret } from "./secrets";
import { confirm } from "../prompt";
import { FirebaseError } from "../error";
import * as experiments from "../experiments";
import { logger } from "../logger";
Comment thread
falahat marked this conversation as resolved.

interface UniversalMakerOutput {
command: string;
args: string[];
language: string;
runtime: string;
envVars?: Record<string, string | number | boolean>;
}

/**
* Runs the Universal Maker binary to build the project.
*/
export function runUniversalMaker(projectRoot: string, framework?: string): AppHostingBuildOutput {
if (!process.env.UNIVERSAL_MAKER_BINARY) {
throw new FirebaseError(
"Please specify the path to your Universal Maker binary by establishing the UNIVERSAL_MAKER_BINARY environment variable.",
);
}

try {
childProcess.spawnSync(
process.env.UNIVERSAL_MAKER_BINARY,
["-application_dir", projectRoot, "-output_dir", projectRoot, "-output_format", "json"],
{
env: {
...process.env,
X_GOOGLE_TARGET_PLATFORM: "fah",
FIREBASE_OUTPUT_BUNDLE_DIR: "bundle_output",
NPM_CONFIG_REGISTRY: "https://registry.npmjs.org/",
},
stdio: "inherit",
},
);
Comment on lines +35 to +47
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

high

The exit status of the spawnSync command should be checked. If the build fails, the process should stop and throw a FirebaseError to prevent the CLI from attempting to use incomplete or missing build artifacts. Additionally, consider removing the hardcoded NPM_CONFIG_REGISTRY as it may interfere with users' custom registry configurations.

    const result = childProcess.spawnSync(
      process.env.UNIVERSAL_MAKER_BINARY,
      ["-application_dir", projectRoot, "-output_dir", projectRoot, "-output_format", "json"],
      {
        env: {
          ...process.env,
          X_GOOGLE_TARGET_PLATFORM: "fah",
          FIREBASE_OUTPUT_BUNDLE_DIR: "bundle_output",
        },
        stdio: "inherit",
      },
    );

    if (result.status !== 0) {
      throw new FirebaseError(`Universal Maker build failed with status ${result.status}`, {
        exit: result.status ?? 1,
      });
    }


const bundleOutput = path.join(projectRoot, "bundle_output");
const targetAppHosting = path.join(projectRoot, ".apphosting");
if (fs.existsSync(bundleOutput)) {
if (!fs.existsSync(targetAppHosting)) {
fs.mkdirSync(targetAppHosting, { recursive: true });
}
const files = fs.readdirSync(bundleOutput);
for (const file of files) {
fs.renameSync(path.join(bundleOutput, file), path.join(targetAppHosting, file));
}
fs.rmdirSync(bundleOutput);
}
Comment on lines +53 to +62
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

medium

The .apphosting directory should be cleared before moving new artifacts to prevent stale files from previous builds from persisting. Also, using fs.rmSync with recursive: true is more robust than fs.rmdirSync for cleaning up the temporary output directory.

    if (fs.existsSync(bundleOutput)) {
      fs.rmSync(targetAppHosting, { recursive: true, force: true });
      fs.mkdirSync(targetAppHosting, { recursive: true });
      const files = fs.readdirSync(bundleOutput);
      for (const file of files) {
        fs.renameSync(path.join(bundleOutput, file), path.join(targetAppHosting, file));
      }
      fs.rmSync(bundleOutput, { recursive: true, force: true });
    }

} catch (e) {
if (e && typeof e === "object" && "code" in e && e.code === "EACCES") {
throw new FirebaseError(
"Failed to execute the Universal Maker binary due to permission constraints. Please assure you have set chmod +x on your file.",
);
}
throw e;
}

const outputFilePath = path.join(projectRoot, "build_output.json");
if (!fs.existsSync(outputFilePath)) {
throw new FirebaseError(
`Universal Maker did not produce the expected output file at ${outputFilePath}`,
);
}

const outputRaw = fs.readFileSync(outputFilePath, "utf-8");
let umOutput: UniversalMakerOutput;
try {
umOutput = JSON.parse(outputRaw) as UniversalMakerOutput;
} catch (e) {
throw new FirebaseError(`Failed to parse build_output.json: ${(e as Error).message}`);
}
Comment on lines +79 to +85
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

medium

The build_output.json file is a temporary metadata file and should be deleted after it has been read and parsed to keep the project directory clean.

Suggested change
const outputRaw = fs.readFileSync(outputFilePath, "utf-8");
let umOutput: UniversalMakerOutput;
try {
umOutput = JSON.parse(outputRaw) as UniversalMakerOutput;
} catch (e) {
throw new FirebaseError(`Failed to parse build_output.json: ${(e as Error).message}`);
}
const outputRaw = fs.readFileSync(outputFilePath, "utf-8");
let umOutput: UniversalMakerOutput;
try {
umOutput = JSON.parse(outputRaw) as UniversalMakerOutput;
} catch (e) {
throw new FirebaseError(`Failed to parse build_output.json: ${(e as Error).message}`);
} finally {
fs.rmSync(outputFilePath, { force: true });
}


let finalRunCommand = `${umOutput.command} ${umOutput.args.join(" ")}`;
const bundleYamlPath = path.join(projectRoot, ".apphosting", "bundle.yaml");
if (fs.existsSync(bundleYamlPath)) {
try {
const bundleRaw = fs.readFileSync(bundleYamlPath, "utf-8");
// Safely parse the YAML string
const bundleData = require("yaml").parse(bundleRaw);
Comment thread
falahat marked this conversation as resolved.
Outdated
if (bundleData?.runConfig?.runCommand) {
finalRunCommand = bundleData.runConfig.runCommand;
}
} catch (e) {
// Fall back gracefully if parser fails
}
}

return {
metadata: {
language: umOutput.language,
runtime: umOutput.runtime,
framework: framework || "nextjs",
},
runConfig: {
runCommand: finalRunCommand,
environmentVariables: Object.entries(umOutput.envVars || {}).map(([k, v]) => ({
variable: k,
value: String(v),
availability: ["RUNTIME"],
})),
},
outputFiles: {
serverApp: {
include: [".apphosting"],
},
},
};
}

export interface AppHostingBuildOutput {
metadata: Record<string, string | number | boolean>;

runConfig: {
runCommand?: string;
environmentVariables?: Array<{
variable: string;
value: string;
availability: string[];
}>;
};
outputFiles?: {
serverApp: {
include: string[];
};
};
}

/**
* Triggers a local build of your App Hosting codebase.
*
* This function orchestrates the build process using the App Hosting build adapter.
*
* It detects the framework (though currently defaults/assumes 'nextjs' in some contexts),
* generates the necessary build artifacts, and returns metadata about the build.
* @param projectId - The project ID to use for resolving secrets.
Expand Down Expand Up @@ -62,9 +192,19 @@ export async function localBuild(
process.env[key] = value;
}

let apphostingBuildOutput;
let apphostingBuildOutput: AppHostingBuildOutput;
try {
apphostingBuildOutput = await localAppHostingBuild(projectRoot, framework);
if (experiments.isEnabled("universalMaker")) {
apphostingBuildOutput = runUniversalMaker(projectRoot, framework);
logger.debug(
`[apphosting] Universal Maker build outputFiles include: ${JSON.stringify(apphostingBuildOutput.outputFiles?.serverApp?.include ?? [])}`,
);
} else {
apphostingBuildOutput = (await localAppHostingBuild(
projectRoot,
framework,
)) as unknown as AppHostingBuildOutput;
Comment thread
falahat marked this conversation as resolved.
Outdated
}
} finally {
for (const key in process.env) {
if (!(key in originalEnv)) {
Expand All @@ -87,7 +227,7 @@ export async function localBuild(
value,
availability,
}),
);
) as unknown as Env[] | undefined;
Comment thread
falahat marked this conversation as resolved.
Outdated

return {
outputFiles: apphostingBuildOutput.outputFiles?.serverApp.include ?? [],
Expand Down
86 changes: 66 additions & 20 deletions src/deploy/apphosting/util.ts
Original file line number Diff line number Diff line change
Expand Up @@ -27,47 +27,93 @@ export async function createLocalBuildTarArchive(
): Promise<string> {
const tmpFile = tmp.fileSync({ prefix: `${config.backendId}-`, postfix: ".tar.gz" }).name;

const targetDir = targetSubDir ? path.join(rootDir, targetSubDir) : rootDir;
const isAppHostingDir =
targetSubDir === ".apphosting" ||
(!!targetSubDir && path.basename(targetSubDir) === ".apphosting");
const targetDir = targetSubDir
? path.isAbsolute(targetSubDir)
? targetSubDir
: path.join(rootDir, targetSubDir)
: rootDir;
const ignore = ["firebase-debug.log", "firebase-debug.*.log", ".git"];
const rdrFiles = await fsAsync.readdirRecursive({
path: targetDir,
ignore: ignore,
isGitIgnore: true,
});
const allFiles: string[] = rdrFiles.map((rdrf) => path.relative(rootDir, rdrf.name));

if (targetSubDir) {
const defaultFiles = fs.readdirSync(rootDir).filter((file) => {
return APPHOSTING_YAML_FILE_REGEX.test(file);
let archiveCwd = rootDir;
let pathsToPack: string[];

if (isAppHostingDir) {
// create temporary directory to bundle things flattened
const tempDir = tmp.dirSync({ unsafeCleanup: true }).name;
fs.cpSync(targetDir, tempDir, { recursive: true });

const rootPackageJson = path.join(rootDir, "package.json");
if (fs.existsSync(rootPackageJson)) {
fs.copyFileSync(rootPackageJson, path.join(tempDir, "package.json"));
}
const rootFiles = fs.readdirSync(rootDir);
for (const file of rootFiles) {
if (APPHOSTING_YAML_FILE_REGEX.test(file)) {
fs.copyFileSync(path.join(rootDir, file), path.join(tempDir, file));
}
}
const rootNext = path.join(rootDir, ".next");
if (fs.existsSync(rootNext)) {
fs.cpSync(rootNext, path.join(tempDir, ".next"), { recursive: true });
}
const rootNodeModules = path.join(rootDir, "node_modules");
if (fs.existsSync(rootNodeModules)) {
fs.cpSync(rootNodeModules, path.join(tempDir, "node_modules"), { recursive: true });
Comment thread
falahat marked this conversation as resolved.
Outdated
}
Comment thread
falahat marked this conversation as resolved.
Outdated

const rdrFiles = await fsAsync.readdirRecursive({
path: tempDir,
ignore: ignore,
isGitIgnore: false,
});
pathsToPack = rdrFiles.map((rdrf) => path.relative(tempDir, rdrf.name));
archiveCwd = tempDir;
} else {
const rdrFiles = await fsAsync.readdirRecursive({
path: targetDir,
ignore: ignore,
isGitIgnore: !targetSubDir, // Disable gitignore if we are anchored to a build output subdirectory
});
for (const file of defaultFiles) {
if (!allFiles.includes(file)) {
allFiles.push(file);
pathsToPack = rdrFiles.map((rdrf) => path.relative(rootDir, rdrf.name));

if (targetSubDir) {
const defaultFiles = fs.readdirSync(rootDir).filter((file) => {
return APPHOSTING_YAML_FILE_REGEX.test(file);
});
for (const file of defaultFiles) {
const relativePath = path.relative(rootDir, path.join(rootDir, file));
if (!pathsToPack.includes(relativePath)) {
pathsToPack.push(relativePath);
}
}
}
}

// `tar` returns a `TypeError` if `allFiles` is empty. Let's check a feww things.
try {
fs.statSync(rootDir);
fs.statSync(archiveCwd);
} catch (err: unknown) {
if (err instanceof Error && "code" in err && err.code === "ENOENT") {
throw new FirebaseError(`Could not read directory "${rootDir}"`);
throw new FirebaseError(`Could not read directory "${archiveCwd}"`);
}
throw err;
}
if (!allFiles.length) {
throw new FirebaseError(`Cannot create a tar archive with 0 files from directory "${rootDir}"`);
if (!pathsToPack.length) {
throw new FirebaseError(
`Cannot create a tar archive with 0 files from directory "${archiveCwd}"`,
);
}

await tar.create(
{
gzip: true,
file: tmpFile,
cwd: rootDir,
cwd: archiveCwd,
portable: true,
},
allFiles,
pathsToPack,
);
return tmpFile;
}
Comment on lines 27 to 73
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

medium

The temporary directory created by tmp.dirSync should be explicitly cleaned up using the removeCallback provided by the tmp library. This ensures that temporary files are removed even if the process stays alive for other tasks. Wrapping the logic in a try...finally block is the recommended pattern for this.

export async function createLocalBuildTarArchive(
  config: AppHostingSingle,
  rootDir: string,
  targetSubDir?: string,
): Promise<string> {
  const tmpFile = tmp.fileSync({ prefix: `${config.backendId}-`, postfix: ".tar.gz" }).name;

  const isAppHostingDir =
    targetSubDir === ".apphosting" ||
    (!!targetSubDir && path.basename(targetSubDir) === ".apphosting");
  const targetDir = targetSubDir
    ? path.isAbsolute(targetSubDir)
      ? targetSubDir
      : path.join(rootDir, targetSubDir)
    : rootDir;
  const ignore = ["firebase-debug.log", "firebase-debug.*.log", ".git"];

  let archiveCwd = rootDir;
  let pathsToPack: string[];
  let tempDirObj: tmp.DirResult | undefined;

  try {
    if (isAppHostingDir) {
      // create temporary directory to bundle things flattened
      tempDirObj = tmp.dirSync({ unsafeCleanup: true });
      const tempDir = tempDirObj.name;
      fs.cpSync(targetDir, tempDir, { recursive: true });

      const rootPackageJson = path.join(rootDir, "package.json");
      if (fs.existsSync(rootPackageJson)) {
        fs.copyFileSync(rootPackageJson, path.join(tempDir, "package.json"));
      }
      const rootFiles = fs.readdirSync(rootDir);
      for (const file of rootFiles) {
        if (APPHOSTING_YAML_FILE_REGEX.test(file)) {
          fs.copyFileSync(path.join(rootDir, file), path.join(tempDir, file));
        }
      }

      const rdrFiles = await fsAsync.readdirRecursive({
        path: tempDir,
        ignore: ignore,
        isGitIgnore: false,
      });
      pathsToPack = rdrFiles.map((rdrf) => path.relative(tempDir, rdrf.name));
      archiveCwd = tempDir;
    } else {
      const rdrFiles = await fsAsync.readdirRecursive({
        path: targetDir,
        ignore: ignore,
        isGitIgnore: !targetSubDir, // Disable gitignore if we are anchored to a build output subdirectory
      });
      pathsToPack = rdrFiles.map((rdrf) => path.relative(rootDir, rdrf.name));

      if (targetSubDir) {
        const defaultFiles = fs.readdirSync(rootDir).filter((file) => {
          return APPHOSTING_YAML_FILE_REGEX.test(file);
        });
        for (const file of defaultFiles) {
          const relativePath = path.relative(rootDir, path.join(rootDir, file));
          if (!pathsToPack.includes(relativePath)) {
            pathsToPack.push(relativePath);
          }
        }
      }
    }

    try {
      fs.statSync(archiveCwd);
    } catch (err: unknown) {
      if (err instanceof Error && "code" in err && err.code === "ENOENT") {
        throw new FirebaseError(`Could not read directory "${archiveCwd}"`);
      }
      throw err;
    }
    if (!pathsToPack.length) {
      throw new FirebaseError(
        `Cannot create a tar archive with 0 files from directory "${archiveCwd}"`,
      );
    }

    await tar.create(
      {
        gzip: true,
        file: tmpFile,
        cwd: archiveCwd,
        portable: true,
      },
      pathsToPack,
    );
    return tmpFile;
  } finally {
    tempDirObj?.removeCallback();
  }
}

Expand Down
6 changes: 6 additions & 0 deletions src/experiments.ts
Original file line number Diff line number Diff line change
Expand Up @@ -148,6 +148,12 @@ export const ALL_EXPERIMENTS = experiments({
default: false,
public: false,
},
universalMaker: {
shortDescription: "Opt-in to Universal Maker standalone binary local builds",
default: false,
public: false,
},

abiu: {
shortDescription: "Enable App Hosting ABIU and runtime selection",
default: false,
Expand Down
Loading