feat: support direct 1p links in js_image_layer (#1711)

e2e/js_image_oci/src/BUILD.bazel

@@ -33,6 +33,7 @@ js_binary(
         "ascii.art",
         ":proto",
         "//:node_modules/@mycorp/pkg-a",
+        "//:node_modules/@mycorp/pkg-b",
         "//:node_modules/chalk",
         "@repo//:dir",
         "@repo//:source_txt",

e2e/js_image_oci/src/main.js

@@ -44,6 +44,11 @@ console.log(`@mycorp/pkg-a acorn@${pkgA.getAcornVersion()}`)
 console.log(`@mycorp/pkg-a uuid@${pkgA.getUuidVersion()}`)
 console.log('')
 
+const pkgB = require('@mycorp/pkg-b')
+console.log(`@mycorp/pkg-b acorn@${pkgB.getAcornVersion()}`)
+console.log(`@mycorp/pkg-b uuid@${pkgB.getUuidVersion()}`)
+console.log('')
+
 console.log(
     chalk.bold.bgYellow(' SOURCE CHECK '),
     space,

e2e/js_image_oci/src/test.yaml

@@ -14,6 +14,8 @@ commandTests:
               ' JS_BINARY__RUNFILES    /app/src/bin.runfiles',
               '@mycorp/pkg-a acorn@7.4.0',
               '@mycorp/pkg-a uuid@9.0.1',
+              '@mycorp/pkg-b acorn@7.4.0',
+              '@mycorp/pkg-b uuid@8.3.2',
               ' SOURCE CHECK    true',
               ' DIRECTORY CHECK    true',
               ' PROTO CHECK    true',
@@ -31,6 +33,8 @@ commandTests:
               ' JS_BINARY__RUNFILES    /app/src/bin.runfiles',
               '@mycorp/pkg-a acorn@7.4.0',
               '@mycorp/pkg-a uuid@9.0.1',
+              '@mycorp/pkg-b acorn@7.4.0',
+              '@mycorp/pkg-b uuid@8.3.2',
               ' SOURCE CHECK    true',
               ' DIRECTORY CHECK    true',
               ' PROTO CHECK    true',

js/private/image/index.ts

@@ -36,9 +36,30 @@ type Compression = 'gzip' | 'none'
 
 function findKeyByValue(entries: Entries, value: string): string | undefined {
     for (const [key, { dest: val }] of Object.entries(entries)) {
+        // Check for exact match
         if (val == value) {
             return key
         }
+        // Check matching parent directory (https://stackoverflow.com/a/45242825).
+        // For example, if `value` is a parent directory of `val`:
+        //
+        //   value = bazel-out/darwin_arm64-fastbuild-ST-1072e68bc32d/bin/pkg/b
+        //   val = bazel-out/darwin_arm64-fastbuild-ST-1072e68bc32d/bin/pkg/b/index.js
+        //
+        // then relative is `index.js` and `/index.js` is stripped from `key`:
+        //
+        //   key = /app/src/bin.runfiles/_main/pkg/b/index.js
+        //
+        // which returns `/app/src/bin.runfiles/_main/pkg/b`
+        const relative = path.relative(value, val)
+        if (
+            relative &&
+            !relative.startsWith('..') &&
+            !path.isAbsolute(relative) &&
+            key.length > relative.length + 1
+        ) {
+            return key.substring(0, key.length - relative.length - 1)
+        }
     }
     return undefined
 }
@@ -51,6 +72,11 @@ async function readlinkSafe(p: string) {
         if (e.code == 'EINVAL') {
             return p
         }
+        if (e.code == 'ENOENT') {
+            // That is as far as we can follow this symlink in this layer so we can only
+            // assume the file exists in another layer
+            return p
+        }
         throw e
     }
 }
@@ -210,10 +236,11 @@ function add_file(
 }
 
 export async function build(
+    allEntries: Entries,
     entries: Entries,
     outputPath: string,
     compression: Compression,
-    owner: Owner,
+    owner: Owner
 ) {
     const output = pack()
     const existing_paths = new Set<string>()
@@ -292,7 +319,8 @@ export async function build(
             // see: https://chmodcommand.com/chmod-775/
             // const stats = await stat(dest)
             const stats: HermeticStat = { mode: MODE_FOR_SYMLINK, mtime: MTIME }
-            const linkname = findKeyByValue(entries, output_path)
+            // Look in all entries for symlinks since they may be in other layers
+            const linkname = findKeyByValue(allEntries, output_path)
             if (linkname == undefined) {
                 throw new Error(
                     `Couldn't map symbolic link ${output_path} to a path. please file a bug at https://github.com/aspect-build/rules_js/issues/new/choose\n\n` +
@@ -343,16 +371,15 @@ export async function build(
 }
 
 if (import.meta.url === pathToFileURL(process.argv[1]).href) {
-    const [
-        entriesPath,
-        outputPath,
-        compression,
-        owner,
-    ] = process.argv.slice(2)
-    const raw_entries = await readFile(entriesPath)
-    const entries: Entries = JSON.parse(raw_entries.toString())
+    const [allEntriesPath, entriesPath, outputPath, compression, owner] =
+        process.argv.slice(2)
+    const rawAllEntries = await readFile(allEntriesPath)
+    const allEntries: Entries = JSON.parse(rawAllEntries.toString())
+    const rawEntries = await readFile(entriesPath)
+    const entries: Entries = JSON.parse(rawEntries.toString())
     const [uid, gid] = owner.split(':').map(Number)
     build(
+        allEntries,
         entries,
         outputPath,
         compression as Compression,

js/private/js_image_layer.bzl

@@ -208,21 +208,22 @@ def _write_laucher(ctx, real_binary_path):
 def _runfile_path(ctx, file, runfiles_dir):
     return paths.join(runfiles_dir, to_rlocation_path(ctx, file))
 
-def _build_layer(ctx, type, entries, inputs):
-    entries_output = ctx.actions.declare_file("{}_{}_entries.json".format(ctx.label.name, type))
-    ctx.actions.write(entries_output, content = json.encode(entries))
+def _build_layer(ctx, type, all_entries_json, entries, inputs):
+    entries_json = ctx.actions.declare_file("{}_{}_entries.json".format(ctx.label.name, type))
+    ctx.actions.write(entries_json, content = json.encode(entries))
 
     extension = "tar.gz" if ctx.attr.compression == "gzip" else "tar"
     output = ctx.actions.declare_file("{name}_{type}.{extension}".format(name = ctx.label.name, type = type, extension = extension))
 
     args = ctx.actions.args()
-    args.add(entries_output)
+    args.add(all_entries_json)
+    args.add(entries_json)
     args.add(output)
     args.add(ctx.attr.compression)
     args.add(ctx.attr.owner)
 
     ctx.actions.run(
-        inputs = inputs + [entries_output],
+        inputs = inputs + [all_entries_json, entries_json],
         outputs = [output],
         arguments = [args],
         executable = ctx.executable._builder,
@@ -266,6 +267,8 @@ def _js_image_layer_impl(ctx):
     node_modules_entries = {}
     node_modules_inputs = []
 
+    all_entries = {}
+
     for file in all_files.to_list():
         destination = _runfile_path(ctx, file, runfiles_dir)
         entry = {
@@ -278,15 +281,19 @@ def _js_image_layer_impl(ctx):
         if destination == real_binary_path:
             entry["remove_non_hermetic_lines"] = True
 
+        all_entries[destination] = entry
         if _should_be_in_node_modules_layer(destination, file):
             node_modules_entries[destination] = entry
             node_modules_inputs.append(file)
         else:
             app_entries[destination] = entry
             app_inputs.append(file)
 
-    app = _build_layer(ctx, type = "app", entries = app_entries, inputs = app_inputs)
-    node_modules = _build_layer(ctx, type = "node_modules", entries = node_modules_entries, inputs = node_modules_inputs)
+    all_entries_json = ctx.actions.declare_file("{}_all_entries.json".format(ctx.label.name))
+    ctx.actions.write(all_entries_json, content = json.encode(all_entries))
+
+    app = _build_layer(ctx, type = "app", all_entries_json = all_entries_json, entries = app_entries, inputs = app_inputs)
+    node_modules = _build_layer(ctx, type = "node_modules", all_entries_json = all_entries_json, entries = node_modules_entries, inputs = node_modules_inputs)
 
     return [
         DefaultInfo(files = depset([node_modules, app])),