Enhance handling of missing files referenced in build files (#159)

package.json

@@ -1,6 +1,6 @@
 {
   "name": "jazelle",
-  "version": "0.0.0-standalone.94",
+  "version": "0.0.0-standalone.95",
   "main": "index.js",
   "bin": {
     "barn": "bin/bootstrap.sh",

tests/fixtures/find-changed-targets/bazel/changes-missing-files-label-only.txt

@@ -0,0 +1 @@
+test-missing-file-label-only-dep/missing-file-label.js

tests/fixtures/find-changed-targets/bazel/changes-missing-files.txt

@@ -0,0 +1,2 @@
+test-missing-file-dep/some-missing-file.js
+test-missing-file-label-only-dep/missing-file-label.js

tests/fixtures/find-changed-targets/bazel/package.json

@@ -4,6 +4,10 @@
     "a",
     "b",
     "c",
-    "d"
+    "d",
+    "test-missing-file",
+    "test-missing-file-dep",
+    "test-missing-file-label-only",
+    "test-missing-file-label-only-dep"
   ]
 }

tests/fixtures/find-changed-targets/bazel/test-missing-file-dep/package.json

@@ -0,0 +1,4 @@
+{
+  "name": "test-missing-file-dep",
+  "version": "0.0.0"
+}

tests/fixtures/find-changed-targets/bazel/test-missing-file-label-only-dep/BUILD.bazel

@@ -0,0 +1,11 @@
+package(default_visibility = ["//visibility:public"])
+
+load("@jazelle//:build-rules.bzl", "web_library")
+
+web_library(
+    name = "library",
+    deps = [],
+    srcs = [
+        "missing-file-label.js",
+    ],
+)

tests/fixtures/find-changed-targets/bazel/test-missing-file-label-only-dep/package.json

@@ -0,0 +1,4 @@
+{
+  "name": "test-missing-file-label-only-dep",
+  "version": "0.0.0"
+}

tests/fixtures/find-changed-targets/bazel/test-missing-file-label-only/package.json

@@ -0,0 +1,7 @@
+{
+  "name": "test-missing-file-label-only",
+  "version": "0.0.0",
+  "dependencies": {
+    "test-missing-file-label-only-dep": "0.0.0"
+  }
+}

tests/fixtures/find-changed-targets/bazel/test-missing-file/package.json

@@ -0,0 +1,7 @@
+{
+  "name": "test-missing-file",
+  "version": "0.0.0",
+  "dependencies": {
+    "test-missing-file-dep": "0.0.0"
+  }
+}

tests/fixtures/find-changed-targets/bazel/yarn.lock

@@ -37,3 +37,31 @@ __metadata:
   resolution: "root-workspace-0b6124@workspace:."
   languageName: unknown
   linkType: soft
+
+"[email protected], test-missing-file-dep@workspace:test-missing-file-dep":
+  version: 0.0.0-use.local
+  resolution: "test-missing-file-dep@workspace:test-missing-file-dep"
+  languageName: unknown
+  linkType: soft
+
+"[email protected], test-missing-file-label-only-dep@workspace:test-missing-file-label-only-dep":
+  version: 0.0.0-use.local
+  resolution: "test-missing-file-label-only-dep@workspace:test-missing-file-label-only-dep"
+  languageName: unknown
+  linkType: soft
+
+"test-missing-file-label-only@workspace:test-missing-file-label-only":
+  version: 0.0.0-use.local
+  resolution: "test-missing-file-label-only@workspace:test-missing-file-label-only"
+  dependencies:
+    test-missing-file-label-only-dep: 0.0.0
+  languageName: unknown
+  linkType: soft
+
+"test-missing-file@workspace:test-missing-file":
+  version: 0.0.0-use.local
+  resolution: "test-missing-file@workspace:test-missing-file"
+  dependencies:
+    test-missing-file-dep: 0.0.0
+  languageName: unknown
+  linkType: soft

tests/index.js

@@ -923,6 +923,8 @@ async function testFindChangedTargets() {
 
     const root = `${tmp}/tmp/find-changed-targets/bazel`;
     const files = `${tmp}/tmp/find-changed-targets/bazel/changes.txt`;
+    const missingFiles = `${tmp}/tmp/find-changed-targets/bazel/changes-missing-files.txt`;
+    const missingFilesLabelOnly = `${tmp}/tmp/find-changed-targets/bazel/changes-missing-files-label-only.txt`;
     await install({
       root: `${tmp}/tmp/find-changed-targets/bazel`,
       cwd: `${tmp}/tmp/find-changed-targets/bazel`,
@@ -946,6 +948,35 @@ async function testFindChangedTargets() {
         '//d:library', // this should appear here because it depends on non-js/x.txt
       ].sort()
     );
+
+    const recoveredTargetsDirs = await findChangedTargets({
+      root,
+      files: missingFiles,
+      format: 'dirs',
+    });
+    assert.deepEqual(
+      recoveredTargetsDirs.sort(),
+      [
+        'test-missing-file',
+        'test-missing-file-dep',
+        'test-missing-file-label-only',
+        'test-missing-file-label-only-dep',
+      ].sort()
+    );
+
+    // Happy path: underlying bazel query returns known labels for missing files
+    const recoveredTargetsDirsLabelOnly = await findChangedTargets({
+      root,
+      files: missingFilesLabelOnly,
+      format: 'dirs',
+    });
+    assert.deepEqual(
+      recoveredTargetsDirsLabelOnly.sort(),
+      [
+        'test-missing-file-label-only',
+        'test-missing-file-label-only-dep',
+      ].sort()
+    );
   }
   {
     const root = `${__dirname}/fixtures/find-changed-targets/no-target`;

utils/bazel-commands.js

@@ -10,6 +10,7 @@ const startupFlags = ['--host_jvm_args=-Xmx15g'];
 
 /*::
 import type {Stdio} from './node-helpers.js';
+export type {ExecException as BazelQueryException} from './node-helpers.js';
 
 export type BuildArgs = {
   root: string,

utils/find-changed-targets.js

@@ -5,13 +5,15 @@ const {getDownstreams} = require('../utils/get-downstreams.js');
 const {exists, read} = require('../utils/node-helpers.js');
 
 /*::
+import type {BazelQueryException} from './bazel-commands.js';
 export type FindChangedTargetsArgs = {
   root: string,
   files?: string,
   format?: string,
 };
 export type FindChangedTargets = (FindChangedTargetsArgs) => Promise<Array<string>>;
 */
+
 const findChangedTargets /*: FindChangedTargets */ = async ({
   root,
   files,
@@ -101,22 +103,31 @@ const findChangedBazelTargets = async ({root, files}) => {
             query: quoteFilePaths(missing).join(' + '),
             args: ['--keep_going'],
           })
-            .then(() => {
+            .then(stdout => {
               // This should never hit because we're checking for missing files,
               // but if it does, we still want to hit the catch block below
-              throw new Error('');
+              const err /*: BazelQueryException */ = new Error('');
+              err.stdout = stdout;
+              throw err;
             })
-            .catch(e => {
+            .catch((e /*: BazelQueryException */) => {
               // if file doesn't exist, find which package it would've belong to, and take source files in the same package
               // doing so is sufficient, because we just want to find out which targets have changed
               // - in the case the file was deleted but a package still exists, pkg will refer to the package
               // - in the case the package itself was deleted, pkg will refer to the root package (which will typically yield no targets in a typical Jazelle setup)
               const regex = /not declared in package '(.*?)'/g;
-              return Array.from(e.message.matchAll(regex))
-                .map(([, pkg]) =>
-                  pkg ? `kind("source file", "//${pkg}:*")` : ''
-                )
-                .filter(Boolean);
+              const recovered = e.stderr
+                ? Array.from(e.stderr.matchAll(regex))
+                    .map(([, pkg]) =>
+                      pkg ? `kind("source file", "//${pkg}:*")` : ''
+                    )
+                    .filter(Boolean)
+                : [];
+              // Some files may have been deleted, but still have labels in the BUILD files
+              const filesWithLabels = e.stdout
+                ? e.stdout.trim().split('\n')
+                : [];
+              return [...filesWithLabels, ...recovered];
             })
         : [];
       const innerQuery = Array.from(

utils/node-helpers.js

@@ -34,6 +34,11 @@ export type ExecOptions = void | {
   gid?: number,
   windowsHide?:boolean,
 };
+export interface ExecException extends Error {
+  status?: number;
+  stdout?: string;
+  stderr?: string;
+}
 export type StdioOptions = Array<Writable>;
 */
 
@@ -84,15 +89,16 @@ function removeActiveChild(child) {
 
 // use exec if you need stdout as a string, or if you need to explicitly setup shell in some way (e.g. export an env var)
 const exec /*: Exec */ = (cmd, opts = {}, stdio = []) => {
-  const errorWithSyncStackTrace = new Error(); // grab stack trace outside of promise so errors are easier to narrow down
+  const errorWithSyncStackTrace /*: ExecException */ = new Error(); // grab stack trace outside of promise so errors are easier to narrow down
   return new Promise((resolve, reject) => {
     const child = proc.exec(cmd, opts, (err, stdout, stderr) => {
       removeActiveChild(child);
 
       if (err) {
-        // $FlowFixMe
-        errorWithSyncStackTrace.status = err.code;
         errorWithSyncStackTrace.message = err.message;
+        errorWithSyncStackTrace.status = Number(err.code || 0);
+        errorWithSyncStackTrace.stdout = String(stdout);
+        errorWithSyncStackTrace.stderr = String(stderr);
         reject(errorWithSyncStackTrace);
       } else {
         resolve(String(stdout));