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.

Sign up for GitHub

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

Jump to bottom

fix: invalidate installed npm repositories correctly (#1200) #1205

Merged
merged 1 commit into from
Oct 2, 2019
Merged

fix: invalidate installed npm repositories correctly (#1200) #1205

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 0 additions & 4 deletions internal/copy_repository/copy_repository.bzl
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -34,10 +34,6 @@ def _copy_repository_impl(rctx):
copy_repository = repository_rule(
implementation = _copy_repository_impl,
attrs = {
"lock_file": attr.label(
allow_single_file = True,
doc = "Though unused, this attribute is necessary to cause this rule to be re-executed anytime the node_modules changes.",
),
"marker_file": attr.label(allow_single_file = True),
},
)
13 changes: 7 additions & 6 deletions internal/npm_install/generate_build_file.js
View file
Open in desktop

Large diffs are not rendered by default.

9 changes: 5 additions & 4 deletions internal/npm_install/generate_build_file.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -42,6 +42,7 @@

import * as fs from 'fs';
import * as path from 'path';
import * as crypto from 'crypto';

function log_verbose(...m: any[]) {
if (!!process.env['VERBOSE_LOGS']) console.error('[generate_build_file.js]', ...m);
Expand All @@ -59,7 +60,7 @@ const args = process.argv.slice(2);
const WORKSPACE = args[0];
const RULE_TYPE = args[1];
const ERROR_ON_BAZEL_FILES = parseInt(args[2]);
const LOCK_FILE_LABEL = args[3];
const LOCK_FILE_PATH = args[3];
const INCLUDED_FILES = args[4] ? args[4].split(',') : [];
const DYNAMIC_DEPS = JSON.parse(args[5] || '{}');

Expand Down Expand Up @@ -333,17 +334,17 @@ def _maybe(repo_rule, name, **kwargs):
path.posix.join(workspaceSourcePath, 'BUILD.bazel'),
'# Marker file that this directory is a bazel package');
}
const sha256sum = crypto.createHash('sha256');
sha256sum.update(fs.readFileSync(LOCK_FILE_PATH, {encoding: 'utf8'}));
writeFileSync(
path.posix.join(workspaceSourcePath, '_bazel_workspace_marker'),
'# Marker file to used by custom copy_repository rule');
`# Marker file to used by custom copy_repository rule\n${sha256sum.digest('hex')}`);

bzlFile += `def install_${workspace}():
_maybe(
copy_repository,
name = "${workspace}",
marker_file = "@${WORKSPACE}//_workspaces/${workspace}:_bazel_workspace_marker",
# Ensure that changes to the node_modules cause the copy to re-execute
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Apparently this doesn't actually work, so we need to force it by including the hash of the lock file.

lock_file = "@${WORKSPACE}${LOCK_FILE_LABEL}",
)
`;

Expand Down
10 changes: 5 additions & 5 deletions internal/npm_install/npm_install.bzl
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -28,7 +28,7 @@ load("//internal/node:node_labels.bzl", "get_node_label", "get_npm_label", "get_
COMMON_ATTRIBUTES = dict(dict(), **{
"always_hide_bazel_files": attr.bool(
doc = """Always hide Bazel build files such as `BUILD` and BUILD.bazel` by prefixing them with `_`.

Defaults to False, in which case Bazel files are _not_ hidden when `symlink_node_modules`
is True. In this case, the rule will report an error when there are Bazel files detected
in npm packages.
Expand Down Expand Up @@ -78,7 +78,7 @@ Note that the pattern used by many packages, which have plugins in the form pkg-
added as implicit dependencies. Thus for example, `rollup` will automatically get `rollup-plugin-json` included in its
dependencies without needing to use this attribute.

The keys in the dict are npm package names, and the value may be a particular package, or a prefix ending with *.
The keys in the dict are npm package names, and the value may be a particular package, or a prefix ending with *.
For example, `dynamic_deps = {"@bazel/typescript": "tsickle", "karma": "my-karma-plugin-*"}`

Note, this may sound like "optionalDependencies" but that field in package.json actually means real dependencies
Expand Down Expand Up @@ -134,7 +134,7 @@ fine grained npm dependencies.
),
"symlink_node_modules": attr.bool(
doc = """Turn symlinking of node_modules on

This requires the use of Bazel 0.26.0 and the experimental
managed_directories feature.

Expand Down Expand Up @@ -163,7 +163,7 @@ def _create_build_files(repository_ctx, rule_type, node, lock_file):
repository_ctx.attr.name,
rule_type,
"1" if error_on_build_files else "0",
str(lock_file),
repository_ctx.path(lock_file),
",".join(repository_ctx.attr.included_files),
str(repository_ctx.attr.dynamic_deps),
])
Expand Down Expand Up @@ -413,7 +413,7 @@ yarn_install = repository_rule(
"frozen_lockfile": attr.bool(
default = False,
doc = """Passes the --frozen-lockfile flag to prevent updating yarn.lock.

Note that enabling this option will require that you run yarn outside of Bazel
when making changes to package.json.
""",
Expand Down