fix: include package store directory in default output of npm_link_package_store #1762
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Having a dangling symlink as the default output of
:node_modules/foo
npm_link_package_store
targets breaks genrules that worked in rules_js 1.0 since they needed to also depend on:node_modules/foo
. This change fixes that use case.It also resolves an
npm_package
bug where if a:node_modules/foo
was included in sources, the underlying copy_to_directory action would fail to resolve the realpath of the dangling symlink and fail. This bug was reported in RELEASE 2.0 tracking issue: #1671 (comment). The issue was due to:node_modules/foo
targets only having the symlink to package store in their default output and not the package store itself. This caused copy_to_directory to fail to follow the symlink since the package store directory was not in the output tree. NB: since the exclude srcs pattern includes**/node_modules/**
the underlyingcopy_to_directory
tool should not try to realpath that file since it is excluded from copying. That upstream fix is in progress here: bazel-contrib/bazel-lib#857.Test plan
//npm/private/test/npm_package:pkg_2
fails withwithout this patch