refactor: simplify git clone logic.

Author: taeold

src/deploy/functions/remoteSource.spec.ts

@@ -13,8 +13,6 @@ describe("remoteSource", () => {
     let cloneStub: sinon.SinonStub;
     let fetchStub: sinon.SinonStub;
     let checkoutStub: sinon.SinonStub;
-    let initSparseStub: sinon.SinonStub;
-    let setSparseStub: sinon.SinonStub;
     let mockGitClient: GitClient;
 
     beforeEach(() => {
@@ -24,14 +22,10 @@ describe("remoteSource", () => {
       cloneStub = sinon.stub().returns({ status: 0 });
       fetchStub = sinon.stub().returns({ status: 0 });
       checkoutStub = sinon.stub().returns({ status: 0 });
-      initSparseStub = sinon.stub().returns({ status: 0 });
-      setSparseStub = sinon.stub().returns({ status: 0 });
       mockGitClient = {
         clone: cloneStub,
         fetch: fetchStub,
         checkout: checkoutStub,
-        initSparseCheckout: initSparseStub,
-        setSparsePaths: setSparseStub,
       } as unknown as GitClient;
     });
 
@@ -66,24 +60,69 @@ describe("remoteSource", () => {
 
     it("should validate subdirectory exists after clone", async () => {
       isGitAvailableStub.returns(true);
-      setSparseStub.returns({
-        status: 1,
-        stderr: "fatal: pathspec 'subdir' did not match any files",
+      // Simulate that the subdirectory does not exist
+      existsSyncStub.callsFake((p: fs.PathLike) => {
+        const s = String(p);
+        if (/[/\\]subdir$/.test(s)) return false; // dir missing
+        if (s.endsWith("functions.yaml")) return true; // avoid manifest error masking
+        return true;
       });
-
       await expect(
         cloneRemoteSource("https://github.com/org/repo", "main", "subdir", mockGitClient),
       ).to.be.rejectedWith(FirebaseError, /Directory 'subdir' not found/);
     });
 
     it("should validate functions.yaml exists", async () => {
       isGitAvailableStub.returns(true);
-      existsSyncStub.withArgs(sinon.match(/firebase-functions-remote/)).returns(true);
-      existsSyncStub.withArgs(sinon.match(/functions\.yaml$/)).returns(false);
+      // Everything exists except the manifest file
+      existsSyncStub.callsFake((p: fs.PathLike) => !String(p).endsWith("functions.yaml"));
 
       await expect(
         cloneRemoteSource("https://github.com/org/repo", "main", undefined, mockGitClient),
       ).to.be.rejectedWith(FirebaseError, /missing a required deployment manifest/);
     });
+
+    it("should successfully clone a repository without a subdirectory", async () => {
+      isGitAvailableStub.returns(true);
+      // Pass manifest check by returning true for any path ending with functions.yaml
+      existsSyncStub.callsFake((p: fs.PathLike) => String(p).endsWith("functions.yaml"));
+
+      const sourceDir = await cloneRemoteSource(
+        "https://github.com/org/repo",
+        "main",
+        undefined,
+        mockGitClient,
+      );
+
+      expect(cloneStub.calledOnceWith("https://github.com/org/repo", sinon.match.string)).to.be
+        .true;
+      expect(fetchStub.calledOnceWith("main", sinon.match.string)).to.be.true;
+      expect(checkoutStub.calledOnceWith("FETCH_HEAD", sinon.match.string)).to.be.true;
+      // No sparse-checkout in MVP path
+      expect(sourceDir).to.be.a("string");
+    });
+
+    it("should successfully clone a repository with a subdirectory", async () => {
+      isGitAvailableStub.returns(true);
+      existsSyncStub.callsFake((p: fs.PathLike) => {
+        const s = String(p);
+        if (/[/\\]functions$/.test(s)) return true; // subdir exists
+        if (s.endsWith("functions.yaml")) return true; // manifest exists
+        return false;
+      });
+
+      const dir = "functions";
+      const sourceDir = await cloneRemoteSource(
+        "https://github.com/org/repo",
+        "main",
+        dir,
+        mockGitClient,
+      );
+
+      expect(fetchStub.calledOnceWith("main", sinon.match.string)).to.be.true;
+      expect(checkoutStub.calledOnceWith("FETCH_HEAD", sinon.match.string)).to.be.true;
+      expect(sourceDir).to.be.a("string");
+      expect(/[/\\]functions$/.test(sourceDir)).to.be.true;
+    });
   });
 });

src/deploy/functions/remoteSource.ts

@@ -11,8 +11,6 @@ export interface GitClient {
   clone(repository: string, destination: string): SpawnSyncReturns<string>;
   fetch(ref: string, cwd: string): SpawnSyncReturns<string>;
   checkout(ref: string, cwd: string): SpawnSyncReturns<string>;
-  initSparseCheckout(cwd: string): SpawnSyncReturns<string>;
-  setSparsePaths(paths: string[], cwd: string): SpawnSyncReturns<string>;
 }
 
 export class DefaultGitClient implements GitClient {
@@ -34,14 +32,6 @@ export class DefaultGitClient implements GitClient {
   checkout(ref: string, cwd: string): SpawnSyncReturns<string> {
     return spawnSync("git", ["checkout", ref], { cwd, encoding: "utf8" });
   }
-
-  initSparseCheckout(cwd: string): SpawnSyncReturns<string> {
-    return spawnSync("git", ["sparse-checkout", "init", "--cone"], { cwd, encoding: "utf8" });
-  }
-
-  setSparsePaths(paths: string[], cwd: string): SpawnSyncReturns<string> {
-    return spawnSync("git", ["sparse-checkout", "set", ...paths], { cwd, encoding: "utf8" });
-  }
 }
 
 export async function cloneRemoteSource(
@@ -84,30 +74,29 @@ export async function cloneRemoteSource(
 
     const cloneResult = await runGitWithRetry(() => gitClient.clone(repository, tmpDir.name));
     if (cloneResult.error || cloneResult.status !== 0) {
-      throw new Error(cloneResult.stderr || cloneResult.stdout || "Clone failed");
-    }
-
-    // If a subdirectory is specified, use sparse checkout to limit the working tree.
-    if (dir) {
-      const initSparse = gitClient.initSparseCheckout(tmpDir.name);
-      if (initSparse.error || initSparse.status !== 0) {
-        throw new Error(initSparse.stderr || initSparse.stdout || "Sparse checkout init failed");
-      }
-      const setSparse = gitClient.setSparsePaths([dir], tmpDir.name);
-      if (setSparse.error || setSparse.status !== 0) {
-        throw new FirebaseError(`Directory '${dir}' not found in repository ${repository}@${ref}`);
-      }
+      throw cloneResult.error ||
+        new Error(
+          cloneResult.stderr || cloneResult.stdout || `Clone failed with status ${cloneResult.status}`,
+        );
     }
 
     // Fetch just the requested ref shallowly, then check it out.
     const fetchResult = await runGitWithRetry(() => gitClient.fetch(ref, tmpDir.name));
     if (fetchResult.error || fetchResult.status !== 0) {
-      throw new Error(fetchResult.stderr || fetchResult.stdout || "Fetch failed");
+      throw fetchResult.error ||
+        new Error(
+          fetchResult.stderr || fetchResult.stdout || `Fetch failed with status ${fetchResult.status}`,
+        );
     }
 
     const checkoutResult = gitClient.checkout("FETCH_HEAD", tmpDir.name);
     if (checkoutResult.error || checkoutResult.status !== 0) {
-      throw new Error(checkoutResult.stderr || checkoutResult.stdout || "Checkout failed");
+      throw checkoutResult.error ||
+        new Error(
+          checkoutResult.stderr ||
+            checkoutResult.stdout ||
+            `Checkout failed with status ${checkoutResult.status}`,
+        );
     }
 
     const sourceDir = dir
@@ -117,6 +106,9 @@ export async function cloneRemoteSource(
           `Subdirectory '${dir}' in remote source must not escape the repository root.`,
         )
       : tmpDir.name;
+    if (dir && !fs.existsSync(sourceDir)) {
+      throw new FirebaseError(`Directory '${dir}' not found in repository ${repository}@${ref}`);
+    }
     requireFunctionsYaml(sourceDir);
     const origin = `${repository}@${ref}${dir ? `/${dir}` : ""}`;
     let sha: string | undefined;