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

fix: don't hardlink the /package source directory in npm_package_store #1533

Merged
merged 1 commit into from
Mar 20, 2024

Conversation

gregmagolan
Copy link
Member

@gregmagolan gregmagolan commented Mar 20, 2024

Fixes #1412.


I'm not entirely sure on the reason that this resolves #1412 but I have some loose theories listed below and I've tested it against a consistent and fast repro of the issue in https://github.com/bazelbuild/examples and found that the two consistent fixes are:

  1. set --modify_execution_info=CopyDirectory=+no-remote on the build
  2. don't use hardlink when copying the /package source directory in the copy_directory_bin_action inside npm_package_store (this PR)

Somehow a cache hit for an npm_package_store that has hardlinked files within a tree artifact to the /package directory of the external repository leads to the failure:

Copying directory npm__some_pkg__1.2.3/package failed: Exec failed due to IOException: /.../npm__some_pkg__1.2.3/package (No such file or directory) 

Is bazel is doing something unusual when writing or restoring cached tree artifacts containing hardlinks to a folder in an external repository? Another possibility is some timing issue where the repo rule is being re-run so the /package directory indeed does not exist momentarily and somehow that interacts badly with the hardlinks in the tree artifact that is cached.

NB: hardlinks to source file in external repositories mean that Bazel will inadvertently set permissions of those source files in the external repository to read-only after the after the copy action completes, which is long after the repository rule executes. This happens because Bazel will set the files in the output tree to read-only (as it does for all files in the output tree) but, because hardlinks share an underlying file ACL, doing so will also set the files in the external repository to read-only. The changes to the ACL to files contained within a source directory that is an input to a cached action may be related to the flake. NB: the flake in #1412 was reported as being more common in Bazel 7+.

In any case, the hardlinks here were a minor optimization for npm packages with large files so removing them to fix this flake is worth the minor performance penalty on the small subset of npm packages with large files.

Type of change

  • Bug fix (change which fixes an issue)

Test plan

  • Manual testing; please provide instructions so we can reproduce:

The issue does not repro in rules_js. The https://github.com/bazelbuild/examples frontend example was used to confirm this fix in this DNL PR: bazelbuild/examples#429.

@gregmagolan gregmagolan changed the title fix: don't hardlink source directories fix: don't hardlink the /package source directory in npm_package_store Mar 20, 2024
@gregmagolan gregmagolan marked this pull request as ready for review March 20, 2024 04:51
@gregmagolan gregmagolan force-pushed the dont_hardlink_source_directories branch from 7c3d145 to a95ba08 Compare March 20, 2024 12:18
@gregmagolan gregmagolan merged commit f78e26c into main Mar 20, 2024
97 checks passed
@gregmagolan gregmagolan deleted the dont_hardlink_source_directories branch March 20, 2024 12:35
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
Status: Done
Development

Successfully merging this pull request may close these issues.

[Bug]: Flaky build failure: npm package directory copy fails with "No such file or directory"
2 participants