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

Managed directories has issue with node_modules/.bin folder on Windows #8491

Closed
gregmagolan opened this issue May 29, 2019 · 11 comments
Closed
Assignees
Labels
P1 I'll work on this now. (Assignee required)

Comments

@gregmagolan
Copy link
Contributor

gregmagolan commented May 29, 2019

Description of the problem:

When running npm_install rule from rules_nodejs 0.30.0+, which relies on the new managed directories feature with Bazel 0.26.0, a truncated BUILD file read error (logs below) was observed on Windows in the node_modules/.bin folder. The error on buildkite: https://buildkite.com/bazel/rules-nodejs-nodejs/builds/2487#26e54aca-4f4b-4626-89a4-f4d68953e53f

Truncated BUILD file reads were observed in the past under two conditions:

  1. multiple yarn_install and/or npm_install rules using the same symlinked node_modules folder at the same time

  2. when node_modules folder being used by yarn_install or npm_install was not added to workspace managed_directories attribute

I suspect the issue here is (2) since all instances of (1) were fixed when that error was first observed.

  • only happens on Windows
  • seems to only happen with npm_install and not yarn_install (although not 100% sure about that as its a flake)
  • node_modules folder for repo fine_grained_deps_npm is under managed_directories
workspace(
    name = "build_bazel_rules_nodejs",
    managed_directories = {
        "@fine_grained_deps_npm": ["internal/e2e/fine_grained_deps/npm/node_modules"],
        ...
    },
)
  • the only BUILD file that causes an issue is internal/e2e/fine_grained_deps/npm/node_modules/.bin/BUILD
  • work-around is to not generate a BUILD file under any folders that start with dot such as .bin; the .bin folder is not an npm package and is not used by most users but there may be a few that depend on it (bazel-contrib/rules_nodejs@10ea9da)

My theory is that managed_directories is somehow skipping the .bin folder on Windows since folders starting with dot are considered hidden when doing a regular directory listing. I can't explain, however, why the error only shows up with npm_install and not yarn_install since they both layout the /node_modules/.bin folder. I suspect that it would happen for both rules and it's a race condition so it's just chance that it doesn't happen for the yarn_install. Locally I haven't been able to reproduce in either case.

Bugs: what's the simplest, easiest way to reproduce this bug? Please provide a minimal example if possible.

Originally seen during development of bazel-contrib/rules_nodejs#704

The branch to try to reproduce on a Windows machine is https://github.com/gregmagolan/rules_nodejs/tree/user-node-modules-proto-windows-flake

Failure can be seen in CI here https://buildkite.com/bazel/rules-nodejs-nodejs/builds/2548#6202a7dd-cb55-41a1-8c2b-277a60008826 from this repo PR bazel-contrib/rules_nodejs#772 which builds the user-node-modules-proto-windows-flake branch.

Clone the above branch and run bazel test ... on a Windows machine.

What operating system are you running Bazel on?

Windows (buildkite).
I was unable to reproduce locally on a Windows machine.

What's the output of bazel info release?

Failure observed on buildkite with bazel: 0.26.0rc13 configured

Logs


ERROR: D:/b/w3ahj22w/external/fine_grained_deps_npm/node_modules/.bin/BUILD.bazel:59:7: syntax error at 'newline': expected ,
--
  | ERROR: D:/b/w3ahj22w/external/fine_grained_deps_npm/node_modules/.bin/BUILD.bazel:59:5: Traceback (most recent call last):
  | File "D:/b/w3ahj22w/external/fine_grained_deps_npm/node_modules/.bin/BUILD.bazel", line 58
  | node_module_library(na, $error$)
  | File "D:/b/w3ahj22w/external/fine_grained_deps_npm/node_modules/.bin/BUILD.bazel", line 59, in node_module_library
  | na
  | name 'na' is not defined
  | ERROR: D:/b/w3ahj22w/external/fine_grained_deps_npm/node_modules/.bin/BUILD.bazel:22:1: Target '@fine_grained_deps_npm//node_modules/.bin:_bazel_marker' contains an error and its package is in error and referenced by '@fine_grained_deps_npm//node_modules/.bin:.bin__files'
  | ERROR: D:/b/w3ahj22w/external/fine_grained_deps_npm/node_modules/.bin/BUILD.bazel:22:1: Target '@fine_grained_deps_npm//node_modules/.bin:atob' contains an error and its package is in error and referenced by '@fine_grained_deps_npm//node_modules/.bin:.bin__files'
  | ERROR: D:/b/w3ahj22w/external/fine_grained_deps_npm/node_modules/.bin/BUILD.bazel:22:1: Target '@fine_grained_deps_npm//node_modules/.bin:hs' contains an error and its package is in error and referenced by '@fine_grained_deps_npm//node_modules/.bin:.bin__files'
  | ERROR: D:/b/w3ahj22w/external/fine_grained_deps_npm/node_modules/.bin/BUILD.bazel:22:1: Target '@fine_grained_deps_npm//node_modules/.bin:http-server' contains an error and its package is in error and referenced by '@fine_grained_deps_npm//node_modules/.bin:.bin__files'
  | ERROR: D:/b/w3ahj22w/external/fine_grained_deps_npm/node_modules/.bin/BUILD.bazel:22:1: Target '@fine_grained_deps_npm//node_modules/.bin:jasmine' contains an error and its package is in error and referenced by '@fine_grained_deps_npm//node_modules/.bin:.bin__files'
  | ERROR: D:/b/w3ahj22w/external/fine_grained_deps_npm/node_modules/.bin/BUILD.bazel:22:1: Target '@fine_grained_deps_npm//node_modules/.bin:mime' contains an error and its package is in error and referenced by '@fine_grained_deps_npm//node_modules/.bin:.bin__files'
  | ERROR: D:/b/w3ahj22w/external/fine_grained_deps_npm/node_modules/.bin/BUILD.bazel:22:1: Target '@fine_grained_deps_npm//node_modules/.bin:mkdirp' contains an error and its package is in error and referenced by '@fine_grained_deps_npm//node_modules/.bin:.bin__files'
  | ERROR: D:/b/w3ahj22w/external/fine_grained_deps_npm/node_modules/.bin/BUILD.bazel:22:1: Target '@fine_grained_deps_npm//node_modules/.bin:opener' contains an error and its package is in error and referenced by '@fine_grained_deps_npm//node_modules/.bin:.bin__files'
  | ERROR: D:/b/w3ahj22w/external/fine_grained_deps_npm/node_modules/.bin/BUILD.bazel:22:1: Target '@fine_grained_deps_npm//node_modules/.bin:test' contains an error and its package is in error and referenced by '@fine_grained_deps_npm//node_modules/.bin:.bin__files'
  | ERROR: D:/b/w3ahj22w/external/fine_grained_deps_npm/node_modules/.bin/BUILD.bazel:22:1: Target '@fine_grained_deps_npm//node_modules/.bin:tsc' contains an error and its package is in error and referenced by '@fine_grained_deps_npm//node_modules/.bin:.bin__files'
  | ERROR: D:/b/w3ahj22w/external/fine_grained_deps_npm/node_modules/.bin/BUILD.bazel:22:1: Target '@fine_grained_deps_npm//node_modules/.bin:tsserver' contains an error and its package is in error and referenced by '@fine_grained_deps_npm//node_modules/.bin:.bin__files'
  | ERROR: D:/b/w3ahj22w/external/fine_grained_deps_npm/node_modules/.bin/BUILD.bazel:52:1: Target '@fine_grained_deps_npm//node_modules/.bin:.bin__files' contains an error and its package is in error and referenced by '@fine_grained_deps_npm//node_modules/.bin:.bin__contents'
  | ERROR: D:/b/w3ahj22w/external/fine_grained_deps_npm/BUILD.bazel:13:1: Target '@fine_grained_deps_npm//node_modules/.bin:.bin__files' contains an error and its package is in error and referenced by '@fine_grained_deps_npm//:node_modules'
  | ERROR: D:/b/w3ahj22w/external/fine_grained_deps_npm/BUILD.bazel:13:1: Target '@fine_grained_deps_npm//node_modules/.bin:.bin__contents' contains an error and its package is in error and referenced by '@fine_grained_deps_npm//:node_modules'

@gregmagolan
Copy link
Contributor Author

/cc @irengrig @alexeagle

@irengrig
Copy link
Contributor

irengrig commented Jun 6, 2019

Hi Greg, thank you for creating the issue.

I experimented with rules_nodejs on Windows VM and currently I see the errors from "bazel test //..." - please see the output in the end of this comment. (there are more errors, I cut only the beginning for the second failing tests)
I can not really understand it; it may be related. Strange that there is no no similar problem on CI.

My HEAD is at:
C:\Users\ichern\rules_nodejs>git log -n 1
commit 3bfa5dae0031b6a0e35bc02c44d8620cc945f64d (HEAD, origin/master, origin/HEAD, master)
Author: Alex Eagle [email protected]
Date: Tue Jun 4 08:43:51 2019 -0700

port #398 to new jasmine_node_test

add a test case for it

Output from "bazel test //...":
Failures:

  1. assembler should handle nested directories
    Message:
    Error: ENOENT: no such file or directory, scandir 'output/to'
    Stack:
    error properties: Object({ errno: -4058, syscall: 'scandir', code: 'ENOENT', path: 'output/to' })
    Error: ENOENT: no such file or directory, scandir 'output/to'
    at Object.readdirSync (fs.js:783:3)
    at UserContext.it (C:\users\ichern\rules_nodejs\internal\web_package\assembler_spec.js:32:19)
    at
    at runCallback (timers.js:705:18)
    at tryOnImmediate (timers.js:676:5)
    at processImmediate (timers.js:658:5)

3 specs, 1 failure
Finished in 0.04 seconds
Randomized with seed 23725 (jasmine --random=true --seed=23725)

FAIL: //internal/npm_install/test:test (see C:/users/ichern/_bazel_ichern/yv2jimq7/execroot/build_bazel_rules_nodejs/bazel-out/x64_windows-fastbuild/testlogs/internal/npm_install/test/test/test.log)
INFO: From Testing //internal/npm_install/test:test:
==================== Test output for //internal/npm_install/test:test:
Randomized with seed 39636
Started
FFFFFFFFFFFFFFFFFFFFFFF...........

Failures:

  1. build file generator integration test should produce a BUILD file for node_modules/zone.js/BUILD.bazel
    Message:
    Error: ENOENT: no such file or directory, open 'C:\users\ichern\rules_nodejs\internal\npm_install\test/package/node_modules/zone.js/BUILD.bazel'
    Stack:
    error properties: Object({ errno: -4058, syscall: 'open', code: 'ENOENT', path: 'C:\users\ichern\rules_nodejs\internal\npm_install\test/package/node_modules/zone.js/BUILD.bazel' })
    Error: ENOENT: no such file or directory, open 'C:\users\ichern\rules_nodejs\internal\npm_install\test/package/node_modules/zone.js/BUILD.bazel'
    at Object.openSync (fs.js:436:3)
    at Object.readFileSync (fs.js:341:35)
    at check (C:\users\ichern\rules_nodejs\internal\npm_install\test\check.js:20:10)
    at UserContext.it (C:\users\ichern\rules_nodejs\internal\npm_install\test\generate_build_file.spec.js:9:9)
    at
    at runCallback (timers.js:705:18)
    at tryOnImmediate (timers.js:676:5)
    at processImmediate (timers.js:658:5)

  2. build file generator integration test should produce a BUILD file for node_modules/jasmine/BUILD.bazel
    Message:
    Error: ENOENT: no such file or directory, open 'C:\users\ichern\rules_nodejs\internal\npm_install\test/package/node_modules/jasmine/BUILD.bazel'
    Stack:
    error properties: Object({ errno: -4058, syscall: 'open', code: 'ENOENT', path: 'C:\users\ichern\rules_nodejs\internal\npm_install\test/package/node_modules/jasmine/BUILD.bazel' })
    Error: ENOENT: no such file or directory, open 'C:\users\ichern\rules_nodejs\internal\npm_install\test/package/node_modules/jasmine/BUILD.bazel'
    at Object.openSync (fs.js:436:3)
    at Object.readFileSync (fs.js:341:35)
    at check (C:\users\ichern\rules_nodejs\internal\npm_install\test\check.js:20:10)
    at UserContext.it (C:\users\ichern\rules_nodejs\internal\npm_install\test\generate_build_file.spec.js:9:9)
    at
    at runCallback (timers.js:705:18)
    at tryOnImmediate (timers.js:676:5)
    at processImmediate (timers.js:658:5)

@irengrig
Copy link
Contributor

Hi Greg, any updates on that?

@gregmagolan
Copy link
Contributor Author

gregmagolan commented Jun 10, 2019

Hi Irina. Thanks for looking into this. Not super high priority as its not blocking anyone but would be interesting to find out the underlying cause.

The web_package test is known to fail on Windows and you can filter it out with "fix-windows"

jasmine_node_test(
    name = "assembler_test",
    srcs = ["assembler_spec.js"],
    tags = [
        "fix-windows",
    ],
    deps = [
        "assembler.js",
        "@npm//jasmine",
    ],
)

The other 2 look like build failures due to locally changes/generated files from previous builds but not 100% sure (this build is tested in Windows CI but rarely developed on with Windows). A clean checkout would answer that question. I'll try it out later on my Windows machine when I'm back home.

@gregmagolan
Copy link
Contributor Author

gregmagolan commented Jun 12, 2019

Yes. The 2nd two failures only happen on Windows on 2nd run of that test. You can avoid it with a clean checkout. I'll fix that up as its annoying and rebase the https://github.com/gregmagolan/rules_nodejs/tree/user-node-modules-proto-windows-flake branch.

I read the description again for this issue and I was actually not able to reproduce even once it locally on my Windows machine. It was only seen on buildkite CI https://buildkite.com/bazel/rules-nodejs-nodejs/builds/2487#26e54aca-4f4b-4626-89a4-f4d68953e53f but it did fail on buildkite consistently without the work-around.

@gregmagolan
Copy link
Contributor Author

Update on my last comment. There is an error in that test that only happens on Windows as described but I took a look at the logs again in your comment and those failures are because bazel run @nodejs//:yarn is a pre-req for that test.

bazel-contrib/rules_nodejs#848 is in progress which removes the pre-req and also fixes the Windows issue above.

@gregmagolan
Copy link
Contributor Author

@irengrig bazel-contrib/rules_nodejs#848 which fixes the //internal/npm_install/test:test failures you saw is done and just waiting to land. In the meaintime I rebased https://github.com/gregmagolan/rules_nodejs/tree/user-node-modules-proto-windows-flake on top of it so if you do a clean clone of that branch you should be able to do a complete build to try to repro.

You'll want to filter out fix-windows tests with bazel test --test_tag_filters=-fix-windows ... to avoid the first failure you saw.

@irengrig
Copy link
Contributor

Thank you for the comments! I was able to reproduce the original problem on the Windows VM as well, but as there were other failures as well, I was not sure.
Will give it another try.

@irengrig
Copy link
Contributor

Great! I cloned your repository & branch, and this is what I got on the second run (so the Bazel caches were alive):

ERROR: C:/users/ichern/_bazel_ichern/yv2jimq7/external/fine_grained_deps_npm/BUILD.bazel:13:1: error loading package '@fine_grained_deps_npm//node_modules/.bin': Unexpected short read from file 'C:/users/ichern/_bazel_ichern/yv2jimq7/external/fine_grained_deps_npm/node_modules/.bin/BUILD.bazel' (expected 1499, got 1498 bytes) and referenced by '@fine_grained_deps_npm//:node_modules'
ERROR: C:/users/ichern/_bazel_ichern/yv2jimq7/external/fine_grained_deps_npm/BUILD.bazel:13:1: error loading package '@fine_grained_deps_npm//node_modules/.bin': Unexpected short read from file 'C:/users/ichern/_bazel_ichern/yv2jimq7/external/fine_grained_deps_npm/node_modules/.bin/BUILD.bazel' (expected 1499, got 1498 bytes) and referenced by '@fine_grained_deps_npm//:node_modules'
ERROR: Analysis of target '//internal/e2e/fine_grained_deps:test_npm' failed; build aborted: error loading package '@fine_grained_deps_npm//node_modules/.bin': Unexpected short read from file 'C:/users/ichern/_bazel_ichern/yv2jimq7/external/fine_grained_deps_npm/node_modules/.bin/BUILD.bazel' (expected 1499, got 1498 bytes)

The tests is not cached for the second invocation, because package-lock.json has changed by the first run, and it is one of the inputs.

So somehow the FileValue of the generated BUILD.bazel file is cached before it is regenerated, and later the cached file size is expected to be read.
I tried to write a test of that situation, but it does not fail for the test project.
I also can only reproduce it on Windows VM with the real data, but quite successfully.
To be continued...

@gregmagolan
Copy link
Contributor Author

Nice! Glad you could reproduce it.

@irengrig
Copy link
Contributor

irengrig commented Jul 3, 2019

I guess I can can close this one, as rules_nodejs are no longer generate BUILD files under node_modules and in particular under node_modules/.bin directory.

@irengrig irengrig closed this as completed Jul 3, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
P1 I'll work on this now. (Assignee required)
Projects
None yet
Development

No branches or pull requests

3 participants