Skip to content

Improve Subsystem paths handling on Windows hosts #3304

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

Merged
merged 1 commit into from
Mar 19, 2025

Conversation

arixmkii
Copy link
Contributor

@arixmkii arixmkii commented Mar 4, 2025

Fixes #3303

Implemented changes

  • hardcoded prefixes remove
  • shell now tries to switch directory for WSL2 machines (mounts are controlled outside of Lima, so, we will still try)
  • using wslpath inside instance for WSL2 machine to determine mounted path, fallback to cygpath for other cases
  • adjusted logic for deciding on home directory on Windows hosts - additional code to erase subsystem mount prefix
  • ioutilx APIs now return empty string on error, because returning the original input was sometimes confusing (rarely used approach)

I did local testing with WSL2, plan to test this with QEMU, when I rebase my current experimental branch,

@arixmkii
Copy link
Contributor Author

arixmkii commented Mar 4, 2025

@jandubois please take a look

@jandubois
Copy link
Member

@jandubois please take a look

I'm sorry, I hoped I would be able to review (test, actually) this change before having to leave on a trip, but I've run out of time. Unless some other maintainer picks this up, it will have to wait until I'm back online in April.

@AkihiroSuda AkihiroSuda added this to the v1.1.0 milestone Mar 12, 2025
@AkihiroSuda
Copy link
Member

Needs rebase

@arixmkii arixmkii force-pushed the fix-hardcoded-prefix branch from 61f1e83 to 848f704 Compare March 18, 2025 15:21
@arixmkii
Copy link
Contributor Author

Rebased

home = strings.TrimPrefix(home, prefix)
}
home += ".linux"
}
Copy link
Member

Choose a reason for hiding this comment

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

This complicated logic should have unit tests

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I would like to write a test for this one, but this have some heavy dependencies. It needs cygpath, which is mostly fine for Windows specific cases, but for wslpath - one needs WSL distribution as it is available only inside container.

At some point I had idea to remove this part entirely and rely only on hand coded cygpath alternative in the code below this (the second conditional branch), as for the purpose of creating home directory it was good enough.

I will create a ticket, where I summarize what options are available to improve situation.

Copy link
Member

@AkihiroSuda AkihiroSuda left a comment

Choose a reason for hiding this comment

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

Thanks

@AkihiroSuda AkihiroSuda merged commit 08327ff into lima-vm:master Mar 19, 2025
31 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Support configurable WSL mount paths
3 participants