Skip to content
Open
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
50 changes: 50 additions & 0 deletions src/functions/python.ts
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,8 @@ import { spawn } from "cross-spawn";
import * as cp from "child_process";
import { logger } from "../logger";
import { IS_WINDOWS } from "../utils";
import * as supported from "../deploy/functions/runtimes/supported";
import { getPythonBinary } from "../deploy/functions/runtimes/python";

/**
* Default directory for python virtual environment.
Expand Down Expand Up @@ -45,3 +47,51 @@ export function runWithVirtualEnv(
env: envs as any,
});
}

/**
* Check if a python binary is available and return its version.
*/
export async function checkPythonVersion(binary: string): Promise<string | undefined> {
return new Promise((resolve) => {
const child = spawn(binary, ["--version"], { stdio: "pipe" });
let output = "";
child.stdout?.on("data", (data: Buffer) => {
output += data.toString();
});
child.stderr?.on("data", (data: Buffer) => {
output += data.toString();
});
child.on("close", (code: number) => {
if (code === 0) {
resolve(output.trim());
} else {
resolve(undefined);
}
});
child.on("error", () => {
resolve(undefined);
});
});
}
Comment on lines +54 to +75

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 checkPythonVersion function spawns a child process to check the Python version but does not enforce a timeout. If a binary hangs indefinitely (e.g., due to a broken installation or interactive prompt), the entire firebase init flow will block forever.

Additionally, the code parameter in the close event listener can be null if the process is terminated by a signal. Specifying code: number explicitly might cause TypeScript compilation errors under strict type checking.

Adding a timeout and letting TypeScript infer the code type improves robustness and type safety.

export async function checkPythonVersion(binary: string): Promise<string | undefined> {
  return new Promise((resolve) => {
    const child = spawn(binary, ["--version"], { stdio: "pipe" });
    let output = "";
    const timeout = setTimeout(() => {
      child.kill();
      resolve(undefined);
    }, 1500);
    child.stdout?.on("data", (data: Buffer) => {
      output += data.toString();
    });
    child.stderr?.on("data", (data: Buffer) => {
      output += data.toString();
    });
    child.on("close", (code) => {
      clearTimeout(timeout);
      if (code === 0) {
        resolve(output.trim());
      } else {
        resolve(undefined);
      }
    });
    child.on("error", () => {
      clearTimeout(timeout);
      resolve(undefined);
    });
  });
}


/**
* Get all available python runtimes on the user machine.
*/
export async function getAvailablePythonRuntimes(): Promise<
{ runtime: supported.Runtime & supported.RuntimeOf<"python">; binary: string; version: string }[]
> {
const pythonRuntimes = (Object.keys(supported.RUNTIMES) as supported.Runtime[]).filter(
(runtime): runtime is supported.Runtime & supported.RuntimeOf<"python"> =>
runtime.startsWith("python"),
);

const results = await Promise.all(
pythonRuntimes.map(async (runtime) => {
const binary = getPythonBinary(runtime);
const version = await checkPythonVersion(binary);
return version ? { runtime, binary, version } : undefined;
}),
);

return results.filter((r): r is NonNullable<typeof r> => !!r);
}
Comment on lines +80 to +97

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

On Windows, getPythonBinary always returns "python.exe" for all Python runtimes. This causes getAvailablePythonRuntimes to query the same "python.exe" binary multiple times in parallel, falsely reporting that all Python runtimes (e.g., python310, python311, python312, etc.) are available with the exact same version (the version of the active "python.exe").

To fix this, we should parse the returned version string and verify that it matches the expected major and minor version of the runtime.

export async function getAvailablePythonRuntimes(): Promise<
  { runtime: supported.Runtime & supported.RuntimeOf<"python">; binary: string; version: string }[]
> {
  const pythonRuntimes = (Object.keys(supported.RUNTIMES) as supported.Runtime[]).filter(
    (runtime): runtime is supported.Runtime & supported.RuntimeOf<"python"> =>
      runtime.startsWith("python"),
  );

  const results = await Promise.all(
    pythonRuntimes.map(async (runtime) => {
      const binary = getPythonBinary(runtime);
      const version = await checkPythonVersion(binary);
      if (!version) {
        return undefined;
      }
      const parts = runtime.match(/^python(\d)(\d+)$/);
      const match = version.match(/(?:Python\s+)?(\d+)\.(\d+)/i);
      if (parts && match) {
        const [_, expectedMajor, expectedMinor] = parts;
        const [__, major, minor] = match;
        if (major !== expectedMajor || minor !== expectedMinor) {
          return undefined;
        }
      }
      return { runtime, binary, version };
    }),
  );

  return results.filter((r): r is NonNullable<typeof r> => !!r);
}

7 changes: 7 additions & 0 deletions src/init/features/functions.spec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -10,6 +10,7 @@
import * as experiments from "../../experiments";
import * as spawn from "cross-spawn";
import * as initSpawn from "../spawn";
import * as pythonUtils from "../../functions/python";

const TEST_SOURCE_DEFAULT = "functions";
const TEST_CODEBASE_DEFAULT = "default";
Expand Down Expand Up @@ -91,7 +92,7 @@
predeploy: ['npm --prefix "$RESOURCE_DIR" run lint'],
disallowLegacyRuntimeConfig: true,
});
expect(askWriteProjectFileStub.getCalls().map((call) => call.args[0])).to.have.members([

Check warning on line 95 in src/init/features/functions.spec.ts

View workflow job for this annotation

GitHub Actions / lint (24)

Unsafe return of an `any` typed value
`${TEST_SOURCE_DEFAULT}/package.json`,
`${TEST_SOURCE_DEFAULT}/.eslintrc.js`,
`${TEST_SOURCE_DEFAULT}/index.js`,
Expand Down Expand Up @@ -122,7 +123,7 @@
],
disallowLegacyRuntimeConfig: true,
});
expect(askWriteProjectFileStub.getCalls().map((call) => call.args[0])).to.have.members([

Check warning on line 126 in src/init/features/functions.spec.ts

View workflow job for this annotation

GitHub Actions / lint (24)

Unsafe return of an `any` typed value
`${TEST_SOURCE_DEFAULT}/package.json`,
`${TEST_SOURCE_DEFAULT}/.eslintrc.js`,
`${TEST_SOURCE_DEFAULT}/tsconfig.dev.json`,
Expand All @@ -135,16 +136,22 @@
describe("python project", () => {
let spawnStub: sinon.SinonStub;
let wrapSpawnStub: sinon.SinonStub;
let getAvailablePythonRuntimesStub: sinon.SinonStub;

beforeEach(() => {
spawnStub = sandbox.stub(spawn, "spawn");
wrapSpawnStub = sandbox.stub(initSpawn, "wrapSpawn");
getAvailablePythonRuntimesStub = sandbox.stub(pythonUtils, "getAvailablePythonRuntimes");
getAvailablePythonRuntimesStub.resolves([
{ runtime: "python314", binary: "python3.14", version: "Python 3.14.0" },
]);
});

it("creates a new python codebase with the correct configuration", async () => {
const config = new Config("{}", { projectDir: "test", cwd: "test" });
const setup = { config: { functions: [] }, rcfile: {} };
prompt.select.onFirstCall().resolves("python");
prompt.select.onSecondCall().resolves("python314");
// do not install dependencies
prompt.confirm.onFirstCall().resolves(false);
askWriteProjectFileStub = sandbox.stub(config, "askWriteProjectFile");
Expand All @@ -161,7 +168,7 @@
runtime: "python314",
disallowLegacyRuntimeConfig: true,
});
expect(askWriteProjectFileStub.getCalls().map((call) => call.args[0])).to.have.members([

Check warning on line 171 in src/init/features/functions.spec.ts

View workflow job for this annotation

GitHub Actions / lint (24)

Unsafe return of an `any` typed value
`${TEST_SOURCE_DEFAULT}/requirements.txt`,
`${TEST_SOURCE_DEFAULT}/.gitignore`,
`${TEST_SOURCE_DEFAULT}/main.py`,
Expand All @@ -183,11 +190,11 @@
let err: Error | null = null;
try {
await actuate(setup, config);
} catch (e: any) {

Check warning on line 193 in src/init/features/functions.spec.ts

View workflow job for this annotation

GitHub Actions / lint (24)

Unexpected any. Specify a different type
err = e;

Check warning on line 194 in src/init/features/functions.spec.ts

View workflow job for this annotation

GitHub Actions / lint (24)

Unsafe assignment of an `any` value
}
expect(err).to.not.be.null;
expect(err!.message).to.contain("Failed to create virtual environment");

Check warning on line 197 in src/init/features/functions.spec.ts

View workflow job for this annotation

GitHub Actions / lint (24)

Forbidden non-null assertion
});

it("installs dependencies successfully if user confirms", async () => {
Expand Down Expand Up @@ -237,11 +244,11 @@
let err: Error | null = null;
try {
await actuate(setup, config);
} catch (e: any) {

Check warning on line 247 in src/init/features/functions.spec.ts

View workflow job for this annotation

GitHub Actions / lint (24)

Unexpected any. Specify a different type
err = e;

Check warning on line 248 in src/init/features/functions.spec.ts

View workflow job for this annotation

GitHub Actions / lint (24)

Unsafe assignment of an `any` value
}
expect(err).to.not.be.null;
expect(err!.message).to.contain("Failed to upgrade pip inside virtual environment");

Check warning on line 251 in src/init/features/functions.spec.ts

View workflow job for this annotation

GitHub Actions / lint (24)

Forbidden non-null assertion
});

it("throws FirebaseError if dependency installation fails", async () => {
Expand Down Expand Up @@ -274,7 +281,7 @@
let err: Error | null = null;
try {
await actuate(setup, config);
} catch (e: any) {

Check warning on line 284 in src/init/features/functions.spec.ts

View workflow job for this annotation

GitHub Actions / lint (24)

Unexpected any. Specify a different type
err = e;
}
expect(err).to.not.be.null;
Expand Down
86 changes: 72 additions & 14 deletions src/init/features/functions/python.ts
Original file line number Diff line number Diff line change
@@ -1,13 +1,16 @@
import { ChildProcess } from "child_process";
import * as ora from "ora";
import { wrapSpawn } from "../../spawn";

import { Config } from "../../../config";
import { getPythonBinary } from "../../../deploy/functions/runtimes/python";
import { runWithVirtualEnv } from "../../../functions/python";
import { confirm } from "../../../prompt";
import { latest } from "../../../deploy/functions/runtimes/supported";
import { getAvailablePythonRuntimes, runWithVirtualEnv } from "../../../functions/python";
import { confirm, select } from "../../../prompt";
import { latest, Runtime, RuntimeOf } from "../../../deploy/functions/runtimes/supported";
import { readTemplateSync } from "../../../templates";
import { FirebaseError } from "../../../error";
import { logger } from "../../../logger";
import { configForCodebase } from "../../../functions/projectConfig";

const MAIN_TEMPLATE = readTemplateSync("init/functions/python/main.py");
const REQUIREMENTS_TEMPLATE = readTemplateSync("init/functions/python/requirements.txt");
Expand All @@ -27,9 +30,12 @@ function waitForProcess(p: ChildProcess): Promise<number | null> {
* Helper to spawn a process and create a python virtual environment.
*/
async function createVirtualEnv(pythonBinary: string, cwd: string): Promise<void> {
const spinner = ora("Creating Python virtual environment...").start();
try {
await wrapSpawn(pythonBinary, ["-m", "venv", "venv"], cwd);
spinner.succeed("Created Python virtual environment.");
} catch (err) {
spinner.fail("Failed to create Python virtual environment.");
throw new FirebaseError(
`Failed to create virtual environment. Please make sure Python is installed and in your PATH.`,
);
Expand Down Expand Up @@ -66,6 +72,47 @@ async function installDependencies(pythonBinary: string, cwd: string): Promise<v
}
}

/**
* Prompt the user to select a Python runtime if multiple runtimes are available, or if the latest runtime is not available.
*/
async function promptSelectRuntime(
availableRuntimes: { runtime: Runtime & RuntimeOf<"python">; version: string }[],
): Promise<Runtime & RuntimeOf<"python">> {
const latestPythonRuntime = latest("python");
let selectedRuntime: Runtime & RuntimeOf<"python">;

const isLatestRuntimeAvailable = availableRuntimes.some((r) => r.runtime === latestPythonRuntime);
// If the latest runtime is available, use it by default
if (isLatestRuntimeAvailable) {
selectedRuntime = latestPythonRuntime;
} else if (availableRuntimes.length >= 1) {
const choices = availableRuntimes.map((r) => ({
name: `${r.runtime} (${r.version})`,
value: r.runtime,
}));
if (!choices.some((c) => c.value === latestPythonRuntime)) {
choices.push({
name: `${latestPythonRuntime} (not available on your machine)`,
value: latestPythonRuntime,
});
}
const runtime = await select({
message: "Which Python runtime would you like to use for this codebase?",
default: choices[0].value,
choices,
});

selectedRuntime = runtime;
} else {
logger.warn(
"No Python runtimes were detected on your machine. " +
`The latest supported runtime ${latestPythonRuntime} will be set in your config, but you may encounter issues deploying or emulating functions.`,
);
selectedRuntime = latestPythonRuntime;
}
return selectedRuntime;
}

/**
* Create a Python Firebase Functions project.
*/
Expand All @@ -79,21 +126,32 @@ export async function setup(setup: any, config: Config): Promise<void> {
await config.askWriteProjectFile(`${setup.functions.source}/.gitignore`, GITIGNORE_TEMPLATE);
await config.askWriteProjectFile(`${setup.functions.source}/main.py`, MAIN_TEMPLATE);

// Write the latest supported runtime version to the config.
config.set("functions.runtime", latest("python"));
const availableRuntimes = await getAvailablePythonRuntimes();
const selectedRuntime = await promptSelectRuntime(availableRuntimes);
const cbconfig = configForCodebase(setup.config.functions, setup.functions.codebase);
cbconfig.runtime = selectedRuntime;

// Write the selected runtime version to the config.
config.set("functions.runtime", selectedRuntime);
// Add python specific ignores to config.
config.set("functions.ignore", ["venv", "__pycache__"]);

// We resolve the version-specific Python binary (e.g. python3.10) to ensure we execute
// the correct version of Python chosen for the functions codebase.
const pythonBinary = getPythonBinary(latest("python"));
await createVirtualEnv(pythonBinary, sourceDir);

const install = await confirm({
message: "Do you want to install dependencies now?",
default: true,
});
if (install) {
await installDependencies(pythonBinary, sourceDir);
const pythonBinary = getPythonBinary(selectedRuntime);
const runtimeDetected = availableRuntimes.some((r) => r.runtime === selectedRuntime);
if (runtimeDetected) {
await createVirtualEnv(pythonBinary, sourceDir);
const install = await confirm({
message: "Do you want to install dependencies now?",
default: true,
});
if (install) {
await installDependencies(pythonBinary, sourceDir);
}
} else {
logger.info(
`Skipping virtual environment creation because ${selectedRuntime} is not available on your machine.`,
);
}
}
Loading