fix: npm_deps tests on Windows, add a fix for Bazel 9.0.0 #2261 #2700
fix: npm_deps tests on Windows, add a fix for Bazel 9.0.0 #2261 #2700jbedard merged 7 commits intoaspect-build:mainfrom
Conversation
|
|
|
| // 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 |
There was a problem hiding this comment.
Is that a fs-patch bug? Why does that only happen on windows? 🤔
There was a problem hiding this comment.
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.
There was a problem hiding this comment.
I mean... that is a bug. I'm just not sure if we can ever fix it?
There was a problem hiding this comment.
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?
|
@Mivr can we fix the failing tests as well? |
…files - Rename _make_symlink to _make_directory_symlink since it's only used for directory symlinks (npm package store) - Update callers in npm_package_store.bzl and npm_link_package_store.bzl - Regenerate image test golden files (file sizes changed due to Windows sandbox compatibility changes in index.js)
|
Replaces #2325? |
Mivr
changed the title
Mivr
changed the title
…places Keep the fast manual relative-path computation in _symlink_package_store (string concat only, no splits/arrays) and add the _SUPPORTS_SYMLINK_TARGET_TYPE conditional directly there, per reviewer feedback.
Mivr
force-pushed
the
#2261-fix-symlinks-with-bazel-upgrade
branch
2 times, most recently
from
b4404e2 to
2c08e06
Compare
5 participants
Bazel 8.x is not compatible with Windows at the moment due to: bazelbuild/bazel#26701.
This branch fixes some common Windows issues so that the bug formentioned can be observed at all.
Then it introduces a fix for Bazel 9.0.0+ where the needed attribute is present.
Bazel 8.x will not work with rules_js until the fix is backported.
For now this seems unlikely: bazelbuild/bazel#27607
Fixes: #2261
Changes are visible to end-users: no
Test plan