Fix: Resolving normalPath when it belongs to a separate drive location (as absolute path)#1696
Fix: Resolving normalPath when it belongs to a separate drive location (as absolute path)#1696MxKevinBeqo wants to merge 1 commit into
Conversation
meta-cla
Bot
added
the
CLA Signed
facebook-github-tools
Bot
added
the
Shared with Meta
|
Hi @MxKevinBeqo - have you observed a bug due to this?
This should not be the case. Metro represents normal paths on Windows as if drives were top level directories, ie if
|
|
Hey @robhogan , thank you for the quick response. I did encounter an issue when the project I was building was in drive C and my node_modules were in another drive (e.g. D).
I was then getting an error where some package files could not be found on a path By the way, since you mentioned it, is my scenario possible using metro-s default resolver and only by adding the dependencies in the |
|
@MxKevinBeqo: Hiya 👋 I've filed #1711 with some targeted fixes for this. If you have a project that reproduces this issue, it'd be really helpful if you could give the changes a quick runthrough, if it's not too much work. Cheers! ❤️ |
# Why Reported at facebook/metro#1696. This PR is being upstreamed at: facebook/metro#1711 The intention in Metro is that cross-device paths are represented as relative normal paths. If the root dir is at `C:\project` then a cross-device normal path at `D:\temp` is represented as `..\..\D:\temp`. This assumption is broken in several places, like `normalToAbsolute` printing, leading to `C:\D:\` paths that are invalid. # How - Fix `normalToAbsolute` not cutting off cross-device absolute paths - Fix `resolveSymlinkToNormal` cutting off trailing slash for absolute Windows device paths - Prevent double slash when appending normal path to root `C:\` path - Prevent `TreeFS#remove` from re-normalizing internally built paths, which can lead to excessively clamped paths - Adjust relative root maximum depth for Windows (remove `-1` adjustment for Windows) # Test Plan - Represented in unit test changes - **Note:** Invalid unit tests have been removed for paths that rise above the filesystem root # Checklist <!-- Please check the appropriate items below if they apply to your diff. --> - [x] I added a `changelog.md` entry and rebuilt the package sources according to [this short guide](https://github.com/expo/expo/blob/main/CONTRIBUTING.md#-before-submitting) - [ ] This diff will work correctly for `npx expo prebuild` & EAS Build (eg: updated a module plugin). - [ ] Conforms with the [Documentation Writing Style Guide](https://github.com/expo/expo/blob/main/guides/Expo%20Documentation%20Writing%20Style%20Guide.md)
Summary: Related PR for `expo/expo` fork: expo/expo#45648 Supersedes #1696 Cross-device path normals are intended to be relative paths from the rootDir. If the rootDir is at `C:\project` then a cross-device normal path at `D:\temp` is represented as `..\..\D:\temp`. This assumption is broken in several places, like `normalToAbsolute` printing, leading to `C:\D:\` paths that are invalid. Changelog: [Fix] Fix Windows path handling for cross-drive paths Pull Request resolved: #1711 Test Plan: - Unit tests were adjusted to demonstrate all failing cases - **Note:** Invalid unit tests have been removed for paths that rise above the filesystem root (they're assumed to be impossible, and their output doesn't have to be deterministic/correct) Reviewed By: robhogan Differential Revision: D104700077 Pulled By: CalixTang fbshipit-source-id: e77baa4e66495b50ef41b260cec60bcc832d4a6a
3 participants
Summary
On Windows, path.relative() returns an absolute path when source and target are on different drives (e.g. Z:\ -> C:...). In that case the stored "normal path" is already absolute, so return it as-is instead of prepending rootDir which would produce Z:\C:... paths.
Changelog: [Fix] Fix resolution to absolute paths when the normalPath is an absolute path in another drive.
Test plan
A new test case is added. When running the test case without the added fix, it will fail, showing the incorrect path resolution. If we run the test case after the fix is applied, the test case passes.