Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

feat: create symlink for build files present on node modules installed with relative paths #2330

Merged
Merged
6 changes: 6 additions & 0 deletions WORKSPACE
Original file line number Diff line number Diff line change
Expand Up @@ -182,6 +182,9 @@ local_repository(
yarn_install(
name = "fine_grained_deps_yarn",
data = [
"//:tools/npm_packages/local_module/yarn/BUILD.bazel",
"//:tools/npm_packages/local_module/yarn/index.js",
"//:tools/npm_packages/local_module/yarn/package.json",
"//internal/npm_install/test:postinstall.js",
],
environment = {
Expand All @@ -202,6 +205,9 @@ yarn_install(
npm_install(
name = "fine_grained_deps_npm",
data = [
"//:tools/npm_packages/local_module/npm/BUILD.bazel",
"//:tools/npm_packages/local_module/npm/index.js",
"//:tools/npm_packages/local_module/npm/package.json",
"//internal/npm_install/test:postinstall.js",
],
environment = {
Expand Down
48 changes: 45 additions & 3 deletions internal/npm_install/generate_build_file.ts
Original file line number Diff line number Diff line change
Expand Up @@ -93,6 +93,15 @@ function writeFileSync(p: string, content: string) {
fs.writeFileSync(p, content);
}

/**
* Creates a file symlink, first ensuring that the directory to
* create it into exists.
*/
function createFileSymlinkSync(target: string, p: string) {
mkdirp(path.dirname(p));
fs.symlinkSync(target, p, 'file');
}

/**
* Main entrypoint.
*/
Expand Down Expand Up @@ -194,11 +203,37 @@ js_library(
* Generates all BUILD & bzl files for a package.
*/
function generatePackageBuildFiles(pkg: Dep) {
// If a BUILD file was shipped with the package, append its contents to the end of
// what we generate for the package.
// If a BUILD file was shipped with the package we should symlink the generated BUILD file
// instead of append its contents to the end of the one we were going to generate.
// https://github.com/bazelbuild/rules_nodejs/issues/2131
let buildFilePath: string|undefined;
if (pkg._files.includes('BUILD')) buildFilePath = 'BUILD';
if (pkg._files.includes('BUILD.bazel')) buildFilePath = 'BUILD.bazel';

// Recreate the pkg dir inside the node_modules folder
const nodeModulesPkgDir = `node_modules/${pkg._dir}`;
// Check if the current package dep dir is a symlink (which happens when we
// install a node_module with link:)
const isPkgDirASymlink =
fs.existsSync(nodeModulesPkgDir) && fs.lstatSync(nodeModulesPkgDir).isSymbolicLink();
// Check if the current package is also written inside the workspace
// NOTE: It's a corner case but fs.realpathSync(.) will not be the root of
// the workspace if symlink_node_modules = False as yarn & npm are run in the root of the
// external repository and anything linked with a relative path would have to be copied
// over via that data attribute
const isPkgInsideWorkspace = fs.realpathSync(nodeModulesPkgDir).includes(fs.realpathSync(`.`));
mistic marked this conversation as resolved.
Show resolved Hide resolved
// Mark build file as one to symlink instead of generate as the package dir is a symlink, we
// have a BUILD file and the pkg is written inside the workspace
const symlinkBuildFile = isPkgDirASymlink && buildFilePath && isPkgInsideWorkspace;

// Log if a BUILD file was expected but was not found
if (!symlinkBuildFile && isPkgDirASymlink) {
console.log(`[yarn_install/npm_install]: package ${
nodeModulesPkgDir} is local symlink and as such a BUILD file for it is expected but none was found. Please add one at ${
fs.realpathSync(nodeModulesPkgDir)}`)
}

// The following won't be used in a symlink build file case
let buildFile = printPackage(pkg);
if (buildFilePath) {
buildFile = buildFile + '\n' +
Expand Down Expand Up @@ -256,7 +291,14 @@ exports_files(["index.bzl"])
}
}

writeFileSync(path.posix.join(pkg._dir, buildFilePath), generateBuildFileHeader(visibility) + buildFile);
if (!symlinkBuildFile) {
writeFileSync(
path.posix.join(pkg._dir, buildFilePath), generateBuildFileHeader(visibility) + buildFile);
} else {
const realPathBuildFileForPkg =
fs.realpathSync(path.posix.join(nodeModulesPkgDir, buildFilePath));
createFileSymlinkSync(realPathBuildFileForPkg, path.posix.join(pkg._dir, buildFilePath));
}
}

/**
Expand Down
19 changes: 18 additions & 1 deletion internal/npm_install/index.js
Original file line number Diff line number Diff line change
Expand Up @@ -39,6 +39,10 @@ function writeFileSync(p, content) {
mkdirp(path.dirname(p));
fs.writeFileSync(p, content);
}
function createFileSymlinkSync(target, p) {
mkdirp(path.dirname(p));
fs.symlinkSync(target, p, 'file');
}
function main() {
const deps = getDirectDependencySet(PKG_JSON_FILE_PATH);
const pkgs = findPackages('node_modules', deps);
Expand Down Expand Up @@ -113,6 +117,13 @@ function generatePackageBuildFiles(pkg) {
buildFilePath = 'BUILD';
if (pkg._files.includes('BUILD.bazel'))
buildFilePath = 'BUILD.bazel';
const nodeModulesPkgDir = `node_modules/${pkg._dir}`;
const isPkgDirASymlink = fs.existsSync(nodeModulesPkgDir) && fs.lstatSync(nodeModulesPkgDir).isSymbolicLink();
const isPkgInsideWorkspace = fs.realpathSync(nodeModulesPkgDir).includes(fs.realpathSync(`.`));
const symlinkBuildFile = isPkgDirASymlink && buildFilePath && isPkgInsideWorkspace;
if (!symlinkBuildFile && isPkgDirASymlink) {
console.log(`[yarn_install/npm_install]: package ${nodeModulesPkgDir} is local symlink and as such a BUILD file for it is expected but none was found. Please add one at ${fs.realpathSync(nodeModulesPkgDir)}`);
}
let buildFile = printPackage(pkg);
if (buildFilePath) {
buildFile = buildFile + '\n' +
Expand Down Expand Up @@ -154,7 +165,13 @@ exports_files(["index.bzl"])
`;
}
}
writeFileSync(path.posix.join(pkg._dir, buildFilePath), generateBuildFileHeader(visibility) + buildFile);
if (!symlinkBuildFile) {
writeFileSync(path.posix.join(pkg._dir, buildFilePath), generateBuildFileHeader(visibility) + buildFile);
}
else {
const realPathBuildFileForPkg = fs.realpathSync(path.posix.join(nodeModulesPkgDir, buildFilePath));
createFileSymlinkSync(realPathBuildFileForPkg, path.posix.join(pkg._dir, buildFilePath));
}
}
function generateBazelWorkspaces(pkgs) {
const workspaces = {};
Expand Down
42 changes: 40 additions & 2 deletions internal/npm_install/npm_install.bzl
Original file line number Diff line number Diff line change
Expand Up @@ -312,7 +312,26 @@ See npm CLI docs https://docs.npmjs.com/cli/install.html for complete list of su

This rule will set the environment variable `BAZEL_NPM_INSTALL` to '1' (unless it
set to another value in the environment attribute). Scripts may use to this to
check if yarn is being run by the `npm_install` repository rule.""",
check if yarn is being run by the `npm_install` repository rule.


**LOCAL MODULES WITH THE NEED TO BE USED BOTH INSIDE AND OUTSIDE BAZEL**

When using a monorepo it's common to have modules that we want to use locally and
publish to an external package repository. This can be achieved using a `js_library` rule
with a `package_name` attribute defined inside the local package `BUILD` file. However,
if the project relies on the local package dependency with `file:`, this could introduce a
race condition with the `npm_install` rule.

In order to overcome it, a link will be created to the package `BUILD` file from the
npm external Bazel repository, which require us to complete a last step which is writing
the expected targets on that same `BUILD` file to be later used by the `npm_install`
rule, which are: `<package_name__files>`, `<package_name__nested_node_modules>`,
`<package_name__contents>`, `<package_name__typings>` and the last
one just `<package_name>`.

If you doubt what those targets should look like, check the
generated `BUILD` file for a given node module.""",
implementation = _npm_install_impl,
)

Expand Down Expand Up @@ -475,6 +494,25 @@ to yarn so that the local cache is contained within the external repository.

This rule will set the environment variable `BAZEL_YARN_INSTALL` to '1' (unless it
set to another value in the environment attribute). Scripts may use to this to
check if yarn is being run by the `yarn_install` repository rule.""",
check if yarn is being run by the `yarn_install` repository rule.


**LOCAL MODULES WITH THE NEED TO BE USED BOTH INSIDE AND OUTSIDE BAZEL**

When using a monorepo it's common to have modules that we want to use locally and
publish to an external package repository. This can be achieved using a `js_library` rule
with a `package_name` attribute defined inside the local package `BUILD` file. However,
if the project relies on the local package dependency with `link:`, this could introduce a
race condition with the `yarn_install` rule.

In order to overcome it, a link will be created to the package `BUILD` file from the
npm external Bazel repository, which require us to complete a last step which is writing
the expected targets on that same `BUILD` file to be later used by the `yarn_install`
rule, which are: `<package_name__files>`, `<package_name__nested_node_modules>`,
`<package_name__contents>`, `<package_name__typings>` and the last
one just `<package_name>`.

If you doubt what those targets should look like, check the
generated `BUILD` file for a given node module.""",
implementation = _yarn_install_impl,
)
1 change: 1 addition & 0 deletions internal/npm_install/test/BUILD.bazel
Original file line number Diff line number Diff line change
Expand Up @@ -133,6 +133,7 @@ sh_test(
"@fine_grained_deps_%s//jasmine" % pkgmgr,
"@fine_grained_deps_%s//jasmine-core" % pkgmgr,
"@fine_grained_deps_%s//ajv" % pkgmgr,
"@fine_grained_deps_%s//local-module" % pkgmgr,
"@fine_grained_deps_%s//typescript" % pkgmgr,
"@fine_grained_deps_%s//rxjs" % pkgmgr,
# Note, test-b depends on [email protected] which should be
Expand Down
4 changes: 4 additions & 0 deletions internal/npm_install/test/common.spec.js
Original file line number Diff line number Diff line change
Expand Up @@ -15,6 +15,10 @@ describe('dependencies', () => {
require('ajv/lib/$data');
});

it(`should resolve local-module`, () => {
require('local-module');
});

it(`should resolve rxjs/src/tsconfig.json`, () => {
// the BUILD.bazel file in rxjs/src should have been
// deleted by fine grained deps and rxjs/src/tsconfig.json
Expand Down
3 changes: 3 additions & 0 deletions tools/fine_grained_deps_npm/package-lock.json

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

1 change: 1 addition & 0 deletions tools/fine_grained_deps_npm/package.json
Original file line number Diff line number Diff line change
Expand Up @@ -12,6 +12,7 @@
"chokidar": "2.0.4",
"http-server": "github:alexeagle/http-server#97205e945b69091606ed83aa0c8489e9ce65d282",
"klaw": "1.3.1",
"local-module": "file:tools/npm_packages/local_module/npm",
"rxjs": "6.5.0"
},
"scripts": {
Expand Down
1 change: 1 addition & 0 deletions tools/fine_grained_deps_yarn/package.json
Original file line number Diff line number Diff line change
Expand Up @@ -12,6 +12,7 @@
"chokidar": "2.0.4",
"http-server": "github:alexeagle/http-server#97205e945b69091606ed83aa0c8489e9ce65d282",
"klaw": "1.3.1",
"local-module": "link:tools/npm_packages/local_module/yarn",
"rxjs": "6.5.0"
},
"scripts": {
Expand Down
4 changes: 4 additions & 0 deletions tools/fine_grained_deps_yarn/yarn.lock
Original file line number Diff line number Diff line change
Expand Up @@ -751,6 +751,10 @@ [email protected]:
optionalDependencies:
graceful-fs "^4.1.9"

"local-module@link:tools/npm_packages/local_module/yarn":
version "0.0.0"
uid ""

lodash.debounce@^4.0.8:
version "4.0.8"
resolved "https://registry.yarnpkg.com/lodash.debounce/-/lodash.debounce-4.0.8.tgz#82d79bff30a67c4005ffd5e2515300ad9ca4d7af"
Expand Down
35 changes: 35 additions & 0 deletions tools/npm_packages/local_module/npm/BUILD.bazel
Original file line number Diff line number Diff line change
@@ -0,0 +1,35 @@
load("@build_bazel_rules_nodejs//:index.bzl", "js_library")

package(default_visibility = ["//visibility:public"])

SRCS = [
"index.js",
"package.json",
]

filegroup(
name = "local-module__files",
srcs = ["@fine_grained_deps_npm//:node_modules/local-module/%s" % file for file in SRCS],
)

js_library(
name = "local-module",
srcs = [":local-module__files"],
deps = [
":local-module__contents",
],
)

js_library(
name = "local-module__contents",
srcs = [
":local-module__files",
":local-module__nested_node_modules",
],
visibility = ["//:__subpackages__"],
)

filegroup(
name = "local-module__nested_node_modules",
visibility = ["//:__subpackages__"],
)
1 change: 1 addition & 0 deletions tools/npm_packages/local_module/npm/index.js
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
module.exports = 'local_module';
4 changes: 4 additions & 0 deletions tools/npm_packages/local_module/npm/package.json
Original file line number Diff line number Diff line change
@@ -0,0 +1,4 @@
{
"version": "0.0.1",
"main": "index.js"
}
35 changes: 35 additions & 0 deletions tools/npm_packages/local_module/yarn/BUILD.bazel
Original file line number Diff line number Diff line change
@@ -0,0 +1,35 @@
load("@build_bazel_rules_nodejs//:index.bzl", "js_library")

package(default_visibility = ["//visibility:public"])

SRCS = [
"index.js",
"package.json",
]

filegroup(
name = "local-module__files",
srcs = ["@fine_grained_deps_yarn//:node_modules/local-module/%s" % file for file in SRCS],
)

js_library(
name = "local-module",
srcs = [":local-module__files"],
deps = [
":local-module__contents",
],
)

js_library(
name = "local-module__contents",
srcs = [
":local-module__files",
":local-module__nested_node_modules",
],
visibility = ["//:__subpackages__"],
)

filegroup(
name = "local-module__nested_node_modules",
visibility = ["//:__subpackages__"],
)
1 change: 1 addition & 0 deletions tools/npm_packages/local_module/yarn/index.js
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
module.exports = 'local_module';
4 changes: 4 additions & 0 deletions tools/npm_packages/local_module/yarn/package.json
Original file line number Diff line number Diff line change
@@ -0,0 +1,4 @@
{
"version": "0.0.1",
"main": "index.js"
}