Skip to content
Open
Show file tree
Hide file tree
Changes from 1 commit
Commits
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
1 change: 1 addition & 0 deletions MODULE.bazel
Original file line number Diff line number Diff line change
Expand Up @@ -13,6 +13,7 @@ bazel_dep(name = "yq.bzl", version = "0.3.2")
bazel_dep(name = "jq.bzl", version = "0.4.0")
bazel_dep(name = "aspect_tools_telemetry", version = "0.3.3")
bazel_dep(name = "bazel_lib", version = "3.0.0")
bazel_dep(name = "bazel_features", version = "1.21.0")
bazel_dep(name = "bazel_skylib", version = "1.5.0")
bazel_dep(name = "platforms", version = "0.0.5")
bazel_dep(name = "rules_nodejs", version = "6.7.3")
Expand Down
14 changes: 8 additions & 6 deletions examples/npm_package/packages/pkg_a/index.js
Original file line number Diff line number Diff line change
Expand Up @@ -14,32 +14,34 @@ function getAcornVersion() {
}

function sandboxAssert() {
if (!/-sandbox\/\d+\/execroot\//.test(__filename)) {
if (!/[/\\]execroot[/\\]/.test(__filename)) {
throw new Error(`Not in sandbox: ${__filename}`)
}

// Use of npm_package() copies files into the npm package store.
if (!__filename.includes('/node_modules/.aspect_rules_js/')) {
if (!/[/\\]node_modules[/\\]\.aspect_rules_js[/\\]/.test(__filename)) {
throw new Error(`Not in package store: ${__filename}`)
}

// When running under test, files should be in runfiles.
// This package may also be used as a run_binary(tool) and not in a test.
if (process.env.TEST_WORKSPACE) {
// On Windows, Node.js resolves junctions to their real path so __filename
Copy link
Member

Choose a reason for hiding this comment

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

Is that a fs-patch bug? Why does that only happen on windows? 🤔

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Not an fs-patch bug. Node.js on Windows resolves junctions (directory symlinks) to their real path when returning __filename. This is expected Windows junction behavior.

Copy link
Member

Choose a reason for hiding this comment

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

I mean... that is a bug. I'm just not sure if we can ever fix it?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

I think this is just how node.js is with junctions and we are kind of working around it.

What do you have in mind as alternative?

// won't start with RUNFILES_DIR.
if (process.env.TEST_WORKSPACE && process.platform !== 'win32') {
if (!__filename.startsWith(process.env.RUNFILES_DIR)) {
throw new Error(`Not in runfiles: ${__filename}`)
}
}

// Resolve of third-party package 'uuid'
const uuid_path = require.resolve('uuid')
if (!/-sandbox\/\d+\/execroot\//.test(uuid_path)) {
if (!/[/\\]execroot[/\\]/.test(uuid_path)) {
throw new Error(`uuid not in sandbox: ${uuid_path}`)
}
if (!uuid_path.includes('/node_modules/.aspect_rules_js/uuid@')) {
if (!/[/\\]node_modules[/\\]\.aspect_rules_js[/\\]uuid@/.test(uuid_path)) {
throw new Error(`uuid not in package store: ${uuid_path}`)
}
if (process.env.TEST_WORKSPACE) {
if (process.env.TEST_WORKSPACE && process.platform !== 'win32') {
if (!uuid_path.startsWith(process.env.RUNFILES_DIR)) {
throw new Error(
`uuid not in runfiles while __filename is: ${uuid_path}`
Expand Down
14 changes: 8 additions & 6 deletions examples/npm_package/packages/pkg_b/index.cjs
Original file line number Diff line number Diff line change
Expand Up @@ -13,32 +13,34 @@ function getAcornVersion() {
}

function sandboxAssert() {
if (!/-sandbox\/\d+\/execroot\//.test(__filename)) {
if (!/[/\\]execroot[/\\]/.test(__filename)) {
throw new Error(`Not in sandbox: ${__filename}`)
}

// Use of npm_package() copies files into the npm package store.
if (!__filename.includes('/node_modules/.aspect_rules_js/')) {
if (!/[/\\]node_modules[/\\]\.aspect_rules_js[/\\]/.test(__filename)) {
throw new Error(`Not in package store: ${__filename}`)
}

// When running under test, files should be in runfiles.
// This package may also be used as a run_binary(tool) and not in a test.
if (process.env.TEST_WORKSPACE) {
// On Windows, Node.js resolves junctions to their real path so __filename
// won't start with RUNFILES_DIR.
if (process.env.TEST_WORKSPACE && process.platform !== 'win32') {
if (!__filename.startsWith(process.env.RUNFILES_DIR)) {
throw new Error(`Not in runfiles: ${__filename}`)
}
}

// Resolve of third-party package 'uuid'
const uuid_path = require.resolve('uuid')
if (!/-sandbox\/\d+\/execroot\//.test(uuid_path)) {
if (!/[/\\]execroot[/\\]/.test(uuid_path)) {
throw new Error(`uuid not in sandbox: ${uuid_path}`)
}
if (!uuid_path.includes('/node_modules/.aspect_rules_js/uuid@')) {
if (!/[/\\]node_modules[/\\]\.aspect_rules_js[/\\]uuid@/.test(uuid_path)) {
throw new Error(`uuid not in package store: ${uuid_path}`)
}
if (process.env.TEST_WORKSPACE) {
if (process.env.TEST_WORKSPACE && process.platform !== 'win32') {
if (!uuid_path.startsWith(process.env.RUNFILES_DIR)) {
throw new Error(
`uuid not in runfiles while __filename is: ${uuid_path}`
Expand Down
14 changes: 8 additions & 6 deletions examples/npm_package/packages/pkg_b/index.mjs
Original file line number Diff line number Diff line change
Expand Up @@ -18,32 +18,34 @@ export function getAcornVersion() {
export function sandboxAssert() {
const __filename = fileURLToPath(import.meta.url)

if (!/-sandbox\/\d+\/execroot\//.test(__filename)) {
if (!/[/\\]execroot[/\\]/.test(__filename)) {
throw new Error(`Not in sandbox: ${__filename}`)
}

// Use of npm_package() copies files into the npm package store.
if (!__filename.includes('/node_modules/.aspect_rules_js/')) {
if (!/[/\\]node_modules[/\\]\.aspect_rules_js[/\\]/.test(__filename)) {
throw new Error(`Not in package store: ${__filename}`)
}

// When running under test, files should be in runfiles.
// This package may also be used as a run_binary(tool) and not in a test.
if (process.env.TEST_WORKSPACE) {
// On Windows, Node.js resolves junctions to their real path so __filename
// won't start with RUNFILES_DIR.
if (process.env.TEST_WORKSPACE && process.platform !== 'win32') {
if (!__filename.startsWith(process.env.RUNFILES_DIR)) {
throw new Error(`Not in runfiles: ${__filename}`)
}
}

// Resolve of third-party package 'uuid'
const uuid_path = fileURLToPath(import.meta.resolve('uuid'))
if (!/-sandbox\/\d+\/execroot\//.test(uuid_path)) {
if (!/[/\\]execroot[/\\]/.test(uuid_path)) {
throw new Error(`uuid not in sandbox: ${uuid_path}`)
}
if (!uuid_path.includes('/node_modules/.aspect_rules_js/uuid@')) {
if (!/[/\\]node_modules[/\\]\.aspect_rules_js[/\\]uuid@/.test(uuid_path)) {
throw new Error(`uuid not in package store: ${uuid_path}`)
}
if (process.env.TEST_WORKSPACE) {
if (process.env.TEST_WORKSPACE && process.platform !== 'win32') {
if (!uuid_path.startsWith(process.env.RUNFILES_DIR)) {
throw new Error(
`uuid not in runfiles while __filename is: ${uuid_path}`
Expand Down
8 changes: 5 additions & 3 deletions examples/npm_package/packages/pkg_c/src/index.cjs
Original file line number Diff line number Diff line change
@@ -1,18 +1,20 @@
const pkgC = require('./pkg-c.json')

function sandboxAssert() {
if (!/-sandbox\/\d+\/execroot\//.test(__filename)) {
if (!/[/\\]execroot[/\\]/.test(__filename)) {
throw new Error(`Not in sandbox: ${__filename}`)
}

// Use of npm_package() copies files into the npm package store.
if (!__filename.includes('/node_modules/.aspect_rules_js/')) {
if (!/[/\\]node_modules[/\\]\.aspect_rules_js[/\\]/.test(__filename)) {
throw new Error(`Not in package store: ${__filename}`)
}

// When running under test, files should be in runfiles.
// This package may also be used as a run_binary(tool) and not in a test.
if (process.env.TEST_WORKSPACE) {
// On Windows, Node.js resolves junctions to their real path so __filename
// won't start with RUNFILES_DIR.
if (process.env.TEST_WORKSPACE && process.platform !== 'win32') {
if (!__filename.startsWith(process.env.RUNFILES_DIR)) {
throw new Error(`Not in runfiles: ${__filename}`)
}
Expand Down
8 changes: 5 additions & 3 deletions examples/npm_package/packages/pkg_c/src/index.mjs
Original file line number Diff line number Diff line change
Expand Up @@ -6,18 +6,20 @@ export default pkgC
export function sandboxAssert() {
const __filename = fileURLToPath(import.meta.url)

if (!/-sandbox\/\d+\/execroot\//.test(__filename)) {
if (!/[/\\]execroot[/\\]/.test(__filename)) {
throw new Error(`Not in sandbox: ${__filename}`)
}

// Use of npm_package() copies files into the npm package store.
if (!__filename.includes('/node_modules/.aspect_rules_js/')) {
if (!/[/\\]node_modules[/\\]\.aspect_rules_js[/\\]/.test(__filename)) {
throw new Error(`Not in package store: ${__filename}`)
}

// When running under test, files should be in runfiles.
// This package may also be used as a run_binary(tool) and not in a test.
if (process.env.TEST_WORKSPACE) {
// On Windows, Node.js resolves junctions to their real path so __filename
// won't start with RUNFILES_DIR.
if (process.env.TEST_WORKSPACE && process.platform !== 'win32') {
if (!__filename.startsWith(process.env.RUNFILES_DIR)) {
throw new Error(`Not in runfiles: ${__filename}`)
}
Expand Down
12 changes: 7 additions & 5 deletions examples/npm_package/packages/pkg_d/index.cjs
Original file line number Diff line number Diff line change
Expand Up @@ -13,25 +13,27 @@ function getAcornVersion() {
}

function sandboxAssert() {
if (!/-sandbox\/\d+\/execroot\//.test(__filename)) {
if (!/[/\\]execroot[/\\]/.test(__filename)) {
throw new Error(`Not in sandbox: ${__filename}`)
}

// Files are in the runfiles directory via js_library(srcs) instead
// of copies in the npm package store.
if (!__filename.startsWith(process.env.RUNFILES_DIR)) {
// On Windows, Node.js resolves junctions to their real path so __filename
// won't start with RUNFILES_DIR.
if (process.platform !== 'win32' && !__filename.startsWith(process.env.RUNFILES_DIR)) {
throw new Error(`Not runfiles: ${__filename}`)
}

// Resolve of third-party package 'uuid'
const uuid_path = require.resolve('uuid')
if (!/-sandbox\/\d+\/execroot\//.test(uuid_path)) {
if (!/[/\\]execroot[/\\]/.test(uuid_path)) {
throw new Error(`uuid not in sandbox: ${uuid_path}`)
}
if (!uuid_path.includes('/node_modules/.aspect_rules_js/uuid@')) {
if (!/[/\\]node_modules[/\\]\.aspect_rules_js[/\\]uuid@/.test(uuid_path)) {
throw new Error(`uuid not in package store: ${uuid_path}`)
}
if (!uuid_path.startsWith(process.env.RUNFILES_DIR)) {
if (process.platform !== 'win32' && !uuid_path.startsWith(process.env.RUNFILES_DIR)) {
throw new Error(`uuid not in runfiles: ${uuid_path}`)
}
}
Expand Down
12 changes: 7 additions & 5 deletions examples/npm_package/packages/pkg_d/index.mjs
Original file line number Diff line number Diff line change
Expand Up @@ -8,25 +8,27 @@ export { v4 as uuid } from 'uuid'
export function sandboxAssert() {
const __filename = fileURLToPath(import.meta.url)

if (!/-sandbox\/\d+\/execroot\//.test(__filename)) {
if (!/[/\\]execroot[/\\]/.test(__filename)) {
throw new Error(`Not in sandbox: ${__filename}`)
}

// Files are in the runfiles directory via js_library(srcs) instead
// of copies in the npm package store.
if (!__filename.startsWith(process.env.RUNFILES_DIR)) {
// On Windows, Node.js resolves junctions to their real path so __filename
// won't start with RUNFILES_DIR.
if (process.platform !== 'win32' && !__filename.startsWith(process.env.RUNFILES_DIR)) {
throw new Error(`Not runfiles: ${__filename}`)
}

// Resolve of third-party package 'uuid'
const uuid_path = fileURLToPath(import.meta.resolve('uuid'))
if (!/-sandbox\/\d+\/execroot\//.test(uuid_path)) {
if (!/[/\\]execroot[/\\]/.test(uuid_path)) {
throw new Error(`uuid not in sandbox: ${uuid_path}`)
}
if (!uuid_path.includes('/node_modules/.aspect_rules_js/uuid@')) {
if (!/[/\\]node_modules[/\\]\.aspect_rules_js[/\\]uuid@/.test(uuid_path)) {
throw new Error(`uuid not in package store: ${uuid_path}`)
}
if (!uuid_path.startsWith(process.env.RUNFILES_DIR)) {
if (process.platform !== 'win32' && !uuid_path.startsWith(process.env.RUNFILES_DIR)) {
throw new Error(`uuid not in runfiles: ${uuid_path}`)
}
}
Expand Down
10 changes: 6 additions & 4 deletions examples/npm_package/packages/pkg_e/index.cjs
Original file line number Diff line number Diff line change
Expand Up @@ -8,13 +8,15 @@ const {
const { sandboxAssert: bSandboxAssert } = require('@mycorp/pkg-b')

function sandboxAssert() {
if (!/-sandbox\/\d+\/execroot\//.test(__filename)) {
if (!/[/\\]execroot[/\\]/.test(__filename)) {
throw new Error(`Not in sandbox: ${__filename}`)
}

// Files are in the runfiles directory via js_library(srcs) instead
// of copies in the npm package store.
if (!__filename.startsWith(process.env.RUNFILES_DIR)) {
// On Windows, Node.js resolves junctions to their real path so __filename
// won't start with RUNFILES_DIR.
if (process.platform !== 'win32' && !__filename.startsWith(process.env.RUNFILES_DIR)) {
throw new Error(`Not runfiles: ${__filename}`)
}

Expand All @@ -25,10 +27,10 @@ function sandboxAssert() {

// Resolve of pkg-d
const pkgDPath = require.resolve('@mycorp/pkg-d')
if (!/-sandbox\/\d+\/execroot\//.test(pkgDPath)) {
if (!/[/\\]execroot[/\\]/.test(pkgDPath)) {
throw new Error(`pkg-d not in sandbox: ${pkgDPath}`)
}
if (!pkgDPath.startsWith(process.env.RUNFILES_DIR)) {
if (process.platform !== 'win32' && !pkgDPath.startsWith(process.env.RUNFILES_DIR)) {
throw new Error(`pkg-d not in runfiles: ${pkgDPath}`)
}
}
Expand Down
10 changes: 6 additions & 4 deletions examples/npm_package/packages/pkg_e/index.mjs
Original file line number Diff line number Diff line change
Expand Up @@ -12,13 +12,15 @@ export { getAcornVersion, toAst, uuid } from '@mycorp/pkg-d'
export async function sandboxAssert() {
const __filename = fileURLToPath(import.meta.url)

if (!/-sandbox\/\d+\/execroot\//.test(__filename)) {
if (!/[/\\]execroot[/\\]/.test(__filename)) {
throw new Error(`Not in sandbox: ${__filename}`)
}

// Files are in the runfiles directory via js_library(srcs) instead
// of copies in the npm package store.
if (!__filename.startsWith(process.env.RUNFILES_DIR)) {
// On Windows, Node.js resolves junctions to their real path so __filename
// won't start with RUNFILES_DIR.
if (process.platform !== 'win32' && !__filename.startsWith(process.env.RUNFILES_DIR)) {
throw new Error(`Not runfiles: ${__filename}`)
}

Expand All @@ -32,10 +34,10 @@ export async function sandboxAssert() {

// Resolve of pkg-d
const pkgDPath = fileURLToPath(import.meta.resolve('@mycorp/pkg-d'))
if (!/-sandbox\/\d+\/execroot\//.test(pkgDPath)) {
if (!/[/\\]execroot[/\\]/.test(pkgDPath)) {
throw new Error(`pkg-d not in sandbox: ${pkgDPath}`)
}
if (!pkgDPath.startsWith(process.env.RUNFILES_DIR)) {
if (process.platform !== 'win32' && !pkgDPath.startsWith(process.env.RUNFILES_DIR)) {
throw new Error(`pkg-d not in runfiles: ${pkgDPath}`)
}
}
Expand Down
11 changes: 7 additions & 4 deletions npm/private/npm_package_store.bzl
Original file line number Diff line number Diff line change
Expand Up @@ -493,8 +493,11 @@ def _symlink_package_store(ctx, package_store_name, target, name):
name,
)
symlink = ctx.actions.declare_symlink(store_symlink_path)
ctx.actions.symlink(
output = symlink,
target_path = ("../../.." if "/" in name else "../..") + target,
Copy link
Member

Choose a reason for hiding this comment

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

This logic got replaced with a relative_file() invocation? Is that expensive?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

It should be just string concats I think

)
kwargs = {
"output": symlink,
"target_path": ("../../.." if "/" in name else "../..") + target,
}
if utils.supports_symlink_target_type:
kwargs["target_type"] = "directory"
ctx.actions.symlink(**kwargs)
return symlink
22 changes: 18 additions & 4 deletions npm/private/utils.bzl
Original file line number Diff line number Diff line change
@@ -1,9 +1,19 @@
"Utility functions for npm rules"

load("@bazel_features//private:util.bzl", "ge")
load("@bazel_lib//lib:paths.bzl", "relative_file")
load("@bazel_lib//lib:repo_utils.bzl", "repo_utils")
load("@bazel_skylib//lib:paths.bzl", "paths")

# Bazel 9+ supports target_type="directory" on ctx.actions.symlink which creates
# junctions instead of file symlinks on Windows. Without this, Node.js gets EPERM
# when traversing symlinks created by declare_symlink on Bazel 8+/Windows.
# See https://github.com/bazelbuild/bazel/issues/26701
# Seems like it won't be cherry picked: https://github.com/bazelbuild/bazel/issues/27607
# Added in commit https://github.com/bazelbuild/bazel/commit/b9bbda939cddab807e34559cb7ee798febfa3861
# If a backport happens this version just needs to be lowered to the 8.x version that has the backport
_SUPPORTS_SYMLINK_TARGET_TYPE = ge("9.0.0")

INTERNAL_ERROR_MSG = "ERROR: rules_js internal error, please file an issue: https://github.com/aspect-build/rules_js/issues"
DEFAULT_REGISTRY_DOMAIN = "registry.npmjs.org"
DEFAULT_REGISTRY_DOMAIN_SLASH = "{}/".format(DEFAULT_REGISTRY_DOMAIN)
Expand Down Expand Up @@ -133,10 +143,13 @@ def _friendly_name(name, version):

def _make_symlink(ctx, symlink_path, target_path):
symlink = ctx.actions.declare_symlink(symlink_path)
ctx.actions.symlink(
output = symlink,
target_path = relative_file(target_path, symlink.path),
)
kwargs = {
"output": symlink,
"target_path": relative_file(target_path, symlink.path),
}
if _SUPPORTS_SYMLINK_TARGET_TYPE:
kwargs["target_type"] = "directory"
ctx.actions.symlink(**kwargs)
return symlink

def _parse_package_name(package):
Expand Down Expand Up @@ -308,6 +321,7 @@ utils = struct(
reverse_force_copy = _reverse_force_copy,
replace_npmrc_token_envvar = _replace_npmrc_token_envvar,
is_tarball_extension = _is_tarball_extension,
supports_symlink_target_type = _SUPPORTS_SYMLINK_TARGET_TYPE,
)

# Exported only to be tested
Expand Down
Loading