Skip to content

Commit

Permalink
fix(builtin): support for scoped modules in linker (bazel-contrib#1199)
Browse files Browse the repository at this point in the history
  • Loading branch information
gregmagolan authored and Fabian Wiles committed Oct 12, 2019
1 parent 3eb3b41 commit 4f3c27c
Show file tree
Hide file tree
Showing 9 changed files with 85 additions and 19 deletions.
30 changes: 23 additions & 7 deletions internal/linker/index.js

Large diffs are not rendered by default.

29 changes: 23 additions & 6 deletions internal/linker/link_node_modules.ts
Original file line number Diff line number Diff line change
Expand Up @@ -24,6 +24,17 @@ Include as much of the build output as you can without disclosing anything confi
`);
}

/**
* Create a new directory and any necessary subdirectories
* if they do not exist.
*/
function mkdirp(p: string) {
if (!fs.existsSync(p)) {
mkdirp(path.dirname(p));
fs.mkdirSync(p);
}
}

async function symlink(target: string, path: string) {
log_verbose(`symlink( ${path} -> ${target} )`);
// Use junction on Windows since symlinks require elevated permissions.
Expand All @@ -46,7 +57,7 @@ async function symlink(target: string, path: string) {
if (!fs.existsSync(path)) {
log_verbose(
'ERROR\n***\nLooks like we created a bad symlink:' +
`\n pwd ${process.cwd()}\n target ${target}\n***`);
`\n pwd ${process.cwd()}\n target ${target}\n path ${path}\n***`);
}
}
}
Expand Down Expand Up @@ -179,8 +190,6 @@ export class Runfiles {
if (this.manifest) {
return this.lookupDirectory(modulePath);
}
// how can we avoid this FS lookup every time? we don't know when process.cwd changed...
// const runfilesRelative = runfiles.dir ? path.relative('.', runfiles.dir) : undefined;
if (runfiles.dir) {
return path.join(runfiles.dir, modulePath);
}
Expand Down Expand Up @@ -262,7 +271,7 @@ export async function main(args: string[], runfiles: Runfiles) {
process.chdir(rootDir);

// Symlinks to packages need to reach back to the workspace/runfiles directory
const workspaceRelative = path.relative('.', workspaceDir);
const workspaceAbs = path.resolve(workspaceDir);

// Now add symlinks to each of our first-party packages so they appear under the node_modules tree
const links = [];
Expand All @@ -273,7 +282,7 @@ export async function main(args: string[], runfiles: Runfiles) {
switch (root) {
case 'bin':
// FIXME(#1196)
target = path.join(workspaceRelative, bin, toWorkspaceDir(modulePath));
target = path.join(workspaceAbs, bin, toWorkspaceDir(modulePath));
// Spend an extra FS lookup to give better error in this case
if (!await exists(target)) {
// TODO: there should be some docs explaining how users are
Expand All @@ -287,7 +296,7 @@ export async function main(args: string[], runfiles: Runfiles) {
}
break;
case 'src':
target = path.join(workspaceRelative, toWorkspaceDir(modulePath));
target = path.join(workspaceAbs, toWorkspaceDir(modulePath));
break;
case 'runfiles':
target = runfiles.resolve(modulePath) || '<runfiles resolution failed>';
Expand All @@ -298,6 +307,14 @@ export async function main(args: string[], runfiles: Runfiles) {
}

for (const m of Object.keys(modules)) {
const segments = m.split('/');
if (segments.length > 2) {
throw new Error(`module ${m} has more than 2 segments which is not a valid node module name`);
}
if (segments.length == 2) {
// ensure the scope exists
mkdirp(segments[0]);
}
const [kind, modulePath] = modules[m];
links.push(linkModule(m, kind, modulePath));
}
Expand Down
2 changes: 2 additions & 0 deletions internal/linker/test/integration/BUILD.bazel
Original file line number Diff line number Diff line change
Expand Up @@ -21,6 +21,7 @@ sh_binary(
":program.js",
"//internal/linker:index.js",
"//internal/linker/test/integration/static_linked_pkg",
"//internal/linker/test/integration/static_linked_scoped_pkg",
"@bazel_tools//tools/bash/runfiles",
"@build_bazel_rules_nodejs//toolchains/node:node_bin",
],
Expand All @@ -35,6 +36,7 @@ linked(
"//%s/absolute_import:index.js" % package_name(),
":run_program",
"//internal/linker/test/integration/dynamic_linked_pkg",
"//internal/linker/test/integration/dynamic_linked_scoped_pkg",
"@npm//semver",
],
)
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,10 @@
load("//internal/js_library:js_library.bzl", "js_library")

package(default_visibility = ["//internal/linker/test:__subpackages__"])

js_library(
name = "dynamic_linked_scoped_pkg",
srcs = ["index.js"],
module_from_src = True,
module_name = "@linker_scoped/dynamic_linked",
)
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
function addD(str) {
return `${str}_d`;
}

exports.addD = addD;
2 changes: 1 addition & 1 deletion internal/linker/test/integration/golden.txt
Original file line number Diff line number Diff line change
@@ -1 +1 @@
1.2.3_c_b_a
1.2.3_a_b_c_d_e
12 changes: 7 additions & 5 deletions internal/linker/test/integration/program.js
Original file line number Diff line number Diff line change
@@ -1,14 +1,16 @@
// First-party package from ./static_linked_pkg
// it should get resolved through runfiles
// First-party "static linked" packages
// they should get resolved through runfiles
const a = require('static_linked');
// First-party package from ./dynamic_linked_pkg
// it should get resolved from the execroot
const e = require('@linker_scoped/static_linked');
// First-party "dynamic linked" packages
// they should get resolved from the execroot
const b = require('dynamic_linked');
const d = require('@linker_scoped/dynamic_linked');
// We've always supported `require('my_workspace')` for absolute imports like Google does it
const c = require('build_bazel_rules_nodejs/internal/linker/test/integration/absolute_import');

// Third-party package installed in the root node_modules
const semver = require('semver');

// This output should match what's in the golden.txt file
console.log(a.addA(b.addB(c.addC(semver.clean(' =v1.2.3 ')))));
console.log(e.addE(d.addD(c.addC(b.addB(a.addA(semver.clean(' =v1.2.3 ')))))));
Original file line number Diff line number Diff line change
@@ -0,0 +1,9 @@
load("//internal/js_library:js_library.bzl", "js_library")

package(default_visibility = ["//internal/linker/test:__subpackages__"])

js_library(
name = "static_linked_scoped_pkg",
srcs = ["index.js"],
module_name = "@linker_scoped/static_linked",
)
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
function addE(str) {
return `${str}_e`;
}

exports.addE = addE;

0 comments on commit 4f3c27c

Please sign in to comment.