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 handling of paths longer than 260 chars on Windows during export #847

Merged
merged 1 commit into from
Oct 31, 2023

Conversation

OmegaJak
Copy link
Contributor

@OmegaJak OmegaJak commented Oct 30, 2023

This addresses an issue brought to our attention by the modpack "Mimi Adventure", with the following repro steps, using version 0.6.0 of the app on Windows:

  1. Create a profile using the modpack "Mimi Adventure" (https://modrinth.com/modpack/mimi-adventure)
  2. Unpair the profile
  3. Launch minecraft
  4. Export the modpack with the "pfm" folder included as an override.
    This produces the following error:
Filesystem error: Path "\\\\?\\C:\\Users\\USER\\AppData\\Roaming\\com.modrinth.theseus\\profiles\\Mimi\\pfm\\cache\\resources\\assets\\pfm\\models\\block\\chair_classic\\stripped_luphie_flowering_pink_luphieclutteredmod_chair_classic\\stripped_luphie_flowering_pink_luphieclutteredmod_chair_classic_tucked.json" does not correspond to a profile

This error occurs because the path length is over 260 characters (Windows docs), which caused dunce::canonicalize (lib.rs line 187) to default to std::fs::canonicalize, which canonicalizes to a UNC path. This path begins with "\\?\", so when we try to strip the profile prefix (which isn't UNC) from the file's path, it fails.

My (somewhat clunky) solution is to use std::fs::canonicalize for both the profile path and the one we're canonicalizing, which should behave consistently regardless of length. The UNC path weirdness shouldn't matter to us here as we're only doing this if we can subtract the canonicalized profile path from the input path, canceling out the UNC prefix.

@OmegaJak
Copy link
Contributor Author

We may want to consider using something like relative-path for these intermediate paths we use within Theseus, I'm guess it would take care of issues like this that are created by us doing our own relative path math.

Copy link
Contributor

@triphora triphora left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I am incredibly impressed that you were able to find this.

@Geometrically Geometrically merged commit f5c7f90 into master Oct 31, 2023
@Geometrically Geometrically deleted the fix_too_long_path_export_handling branch October 31, 2023 01:27
Geometrically added a commit that referenced this pull request Oct 16, 2024
Co-authored-by: Geometrically <18202329+Geometrically@users.noreply.github.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Development

Successfully merging this pull request may close these issues.

4 participants