When launching wsl, promote the starting directory to --cd #9223

DHowett · 2021-02-19T19:22:55Z

This commit introduces a hack to ConptyConnection for launching WSL.
When we detect that WSL is being launched (either "wsl" or "wsl.exe",
unqialified or specifically from the current OS's System32 directory),
we will promote the startingDirectory specified at launch time into a
commandline argument.

Why do we want to switch to --cd?
With the current design of ConptyConnection and WSL, there are some
significant limitations:

startingDirectory cannot be a WSL path, which forces users to
use weird tricks such as setting the starting directory to
\\wsl$\Distro\home\user.
WSL occasionally fails to launch in time to handle a \\wsl$ path,
which makes us spawn in a strange location (or no location at all).

(This fix will only address the second one until a WSL update is
released that adds support for --cd $LINUX_PATH.)

We will not do the promotion if any of the following are true:

the commandline contains --cd already
the commandline contains a bare ~
- This was a commonly-used workaround that forced wsl to start in the
  user's home directory. It conflicts with --cd.
wsl is not spelled properly (WSL and WSL.EXE are unacceptable)
an absolute path to wsl outside the system32 directory is provided

We chose the do this trick in the connection layer, the latest possible
point, because it captures the most use cases.

We could have done it earlier, but the options were quite limiting.
They are:

Generate WSL profiles with startingDirectory set to the home folder
- We can't do this because we do not know the user's home folder
  path.
Generate WSL profiles with --cd in them.
- This only works for unmodified profiles.
- This only works for generated profiles.
- Users cannot override the commandline without breaking it.
- Users cannot specify a startingDirectory (!) since the one on the
  commandline wins.
Set a flag on generated WSL profiles to request this trick
- This only works for generated profiles. Users who create their own
  WSL profiles couldn't set startingDirectory and have it work the
  same.

Patching the commandline, hacky though it may be, seemed to be the most
compatible option. Eventually, we can even support wt -d ~ wsl!

Validation Steps Performed

Manual validation for the following cases:

// MUST MANGLE
auto a01 = _tryMangleStartingDirectoryForWSL(LR"(wsl)", L"SENTINEL");
auto a02 = _tryMangleStartingDirectoryForWSL(LR"(wsl -d X)", L"SENTINEL");
auto a03 = _tryMangleStartingDirectoryForWSL(LR"(wsl -d X ~/bin/sh)", L"SENTINEL");
auto a04 = _tryMangleStartingDirectoryForWSL(LR"(wsl.exe)", L"SENTINEL");
auto a05 = _tryMangleStartingDirectoryForWSL(LR"(wsl.exe -d X)", L"SENTINEL");
auto a06 = _tryMangleStartingDirectoryForWSL(LR"(wsl.exe -d X ~/bin/sh)", L"SENTINEL");
auto a07 = _tryMangleStartingDirectoryForWSL(LR"("wsl")", L"SENTINEL");
auto a08 = _tryMangleStartingDirectoryForWSL(LR"("wsl.exe")", L"SENTINEL");
auto a09 = _tryMangleStartingDirectoryForWSL(LR"("wsl" -d X)", L"SENTINEL");
auto a10 = _tryMangleStartingDirectoryForWSL(LR"("wsl.exe" -d X)", L"SENTINEL");
auto a11 = _tryMangleStartingDirectoryForWSL(LR"("C:\Windows\system32\wsl.exe" -d X)", L"SENTINEL");
auto a12 = _tryMangleStartingDirectoryForWSL(LR"("C:\windows\system32\wsl" -d X)", L"SENTINEL");
auto a13 = _tryMangleStartingDirectoryForWSL(LR"(wsl ~/bin)", L"SENTINEL");

// MUST NOT MANGLE
auto a14 = _tryMangleStartingDirectoryForWSL(LR"("C:\wsl.exe" -d X)", L"SENTINEL");
auto a15 = _tryMangleStartingDirectoryForWSL(LR"(C:\wsl.exe)", L"SENTINEL");
auto a16 = _tryMangleStartingDirectoryForWSL(LR"(wsl --cd C:\)", L"SENTINEL");
auto a17 = _tryMangleStartingDirectoryForWSL(LR"(wsl ~)", L"SENTINEL");
auto a18 = _tryMangleStartingDirectoryForWSL(LR"(wsl ~ -d Ubuntu)", L"SENTINEL");

We don't have anywhere to put TerminalConnection unit tests :|

Closes #592.

This commit introduces a hack to ConptyConnection for launching WSL. When we detect that WSL is being launched (either "wsl" or "wsl.exe", unqialified or _specifically_ from the current OS's System32 directory), we will promote the startingDirectory specified at launch time into a commandline argument. Why do we want to switch to `--cd`? With the current design of ConptyConnection and WSL, there are some significant limitations: * `startingDirectory` cannot be a WSL path, which forces users to use weird tricks such as setting the starting directory to `\\wsl$\Distro\home\user`. * WSL occasionally fails to launch in time to handle a `\\wsl$` path, which makes us spawn in a strange location (or no location at all). (This fix will only address the second one until a WSL update is released that adds support for `--cd $LINUX_PATH`.) We will not do the promotion if any of the following are true: * the commandline contains `--cd` already * the commandline contains a bare `~` * This was a commonly-used workaround that forced wsl to start in the user's home directory. It conflicts with --cd. * wsl is not spelled properly (`WSL` and `WSL.EXE` are unacceptable) * an absolute path to wsl outside the system32 directory is provided We chose the do this trick in the connection layer, the latest possible point, because it captures the most use cases. We could have done it earlier, but the options were quite limiting. They are: * Generate WSL profiles with startingDirectory set to the home folder * We can't do this because we do not know the user's home folder path. * Generate WSL profiles with `--cd` in them. * This only works for unmodified profiles. * This only works for generated profiles. * Users cannot override the commandline without breaking it. * Users cannot specify a startingDirectory (!) since the one on the commandline wins. * Set a flag on generated WSL profiles to request this trick * This only works for generated profiles. Users who create their own WSL profiles couldn't set startingDirectory and have it work the same. Patching the commandline, hacky though it may be, seemed to be the most compatible option. Eventually, we can even support `wt -d ~ wsl`!

github-actions · 2021-02-19T19:27:39Z

Misspellings found, please review:

Descrption

To accept these changes, run the following commands from this repository on this branch

pushd $(git rev-parse --show-toplevel)
perl -e '
my @expect_files=qw('".github/actions/spelling/expect/alphabet.txt
.github/actions/spelling/expect/expect.txt
.github/actions/spelling/expect/web.txt"');
@ARGV=@expect_files;
my @stale=qw('"aspnet boostorg COINIT dahall fde fea fmtlib isocpp mintty NVDA pinam QOL remoting unte vcrt what3words xamarin "');
my $re=join "|", @stale;
my $suffix=".".time();
my $previous="";
sub maybe_unlink { unlink($_[0]) if $_[0]; }
while (<>) {
  if ($ARGV ne $old_argv) { maybe_unlink($previous); $previous="$ARGV$suffix"; rename($ARGV, $previous); open(ARGV_OUT, ">$ARGV"); select(ARGV_OUT); $old_argv = $ARGV; }
  next if /^(?:$re)(?:(?:\r|\n)*$| .*)/; print;
}; maybe_unlink($previous);'
perl -e '
my $new_expect_file=".github/actions/spelling/expect/e6aa902224e5a22998a533ba217ba43457fb6849.txt";
use File::Path qw(make_path);
make_path ".github/actions/spelling/expect";
open FILE, q{<}, $new_expect_file; chomp(my @words = <FILE>); close FILE;
my @add=qw('"coinit Descrption Remoting "');
my %items; @items{@words} = @words x (1); @items{@add} = @add x (1);
@words = sort {lc($a) cmp lc($b)} keys %items;
open FILE, q{>}, $new_expect_file; for my $word (@words) { print FILE "$word\n" if $word =~ /\w/; };
close FILE;'
popd

✏️ Contributor please read this

By default the command suggestion will generate a file named based on your commit. That's generally ok as long as you add the file to your commit. Someone can reorganize it later.

⚠️ The command is written for posix shells. You can copy the contents of each perl command excluding the outer ' marks and dropping any '"/"' quotation mark pairs into a file and then run perl file.pl from the root of the repository to run the code. Alternatively, you can manually insert the items...

If the listed items are:

... misspelled, then please correct them instead of using the command.
... names, please add them to .github/actions/spelling/dictionary/names.txt.
... APIs, you can add them to a file in .github/actions/spelling/dictionary/.
... just things you're using, please add them to an appropriate file in .github/actions/spelling/expect/.
... tokens you only need in one place and shouldn't generally be used, you can add an item in an appropriate file in .github/actions/spelling/patterns/.

See the README.md in each directory for more information.

🔬 You can test your commits without appending to a PR by creating a new branch with that extra change and pushing it to your fork. The check-spelling action will run in response to your push -- it doesn't require an open pull request. By using such a branch, you can limit the number of typos your peers see you make. 😉

🗜️ If you see a bunch of garbage and it relates to a binary-ish string, please add a file path to the .github/actions/spelling/excludes.txt file instead of just accepting the garbage.

File paths are Perl 5 Regular Expressions - you can test yours before committing to verify it will match your files.

^ refers to the file's path from the root of the repository, so ^README\.md$ would exclude README.md (on whichever branch you're using).

driver1998 · 2021-03-30T01:46:00Z

It sounds unlikely, but what'll happen if I have a different wsl.exe somewhere else in %PATH%?

DHowett · 2021-03-30T01:47:17Z

My official stance is, “it had better be compatible with wsl, because otherwise it should not be masquerading as wsl.”

miniksa · 2021-03-30T17:36:01Z

src/cascadia/TerminalConnection/ConptyConnection.cpp

+                            // We want to suppress --cd if they have added a bare ~ to their commandline (they conflict).
+                            break;
+                        }
+                        // Tilde followed by non-space should be okay (like, wsl -d Debian ~/blah.sh)


Mildly worried this will bite us slightly but it's still worth trying as it SEEMS ok.

Yea I'm mildly worried about someone doing some silly thing like wsl -- bash -c vim ~ ; ping whatever.com

(that's made up but you get the point)

miniksa

Good enough. Just get the typo.

zadjii-msft

this feels dangerous, but hey let's ship it to Preview and see what happens
We should really really have connection tests with this kind of code getting added. That of course means another project (😢), but at least it's a project that's not dependent on WinUI?

zadjii-msft · 2021-03-30T20:50:26Z

src/cascadia/TerminalConnection/ConptyConnection.cpp

@@ -56,6 +56,72 @@ namespace winrt::Microsoft::Terminal::TerminalConnection::implementation
        return S_OK;
    }

+    // Function Descrption:


Suggested change

// Function Descrption:

// Function Description:

zadjii-msft · 2021-03-30T20:50:41Z

src/cascadia/TerminalConnection/ConptyConnection.cpp

+        do
+        {
+            if (startingDirectory.size() > 0 && commandLine.size() >= 3)
+            { // "wsl" is three characters; this is a safe bet. no point in doing it if there's no starting directory though!


Suggested change

{ // "wsl" is three characters; this is a safe bet. no point in doing it if there's no starting directory though!

{

// "wsl" is three characters; this is a safe bet. no point in doing it if there's no starting directory though!

zadjii-msft · 2021-03-30T20:54:13Z

src/cascadia/TerminalConnection/ConptyConnection.cpp

+                            // We want to suppress --cd if they have added a bare ~ to their commandline (they conflict).
+                            break;
+                        }
+                        // Tilde followed by non-space should be okay (like, wsl -d Debian ~/blah.sh)


Yea I'm mildly worried about someone doing some silly thing like wsl -- bash -c vim ~ ; ping whatever.com

(that's made up but you get the point)

ghost · 2021-04-21T10:52:54Z

Hello @zadjii-msft!

Because this pull request has the AutoMerge label, I will be glad to assist with helping to merge this pull request once all check-in policies pass.

p.s. you can customize the way I help with merging this pull request, such as holding this pull request until a specific person approves. Simply @mention me (`@msftbot`) and give me an instruction to get started! Learn more here.

zadjii-msft · 2021-04-23T10:14:50Z

@DHowett fix the typo and yeet this for the bug bash this afternoon

DHowett · 2021-04-23T15:26:18Z

@zadjii-msft thx for the reminder. this is building in selfhost-1.9 right now. I'll merge after the bash :)

DHowett · 2021-04-23T16:19:47Z

Somebody else -- probably the connection directory validator -- is failing this and setting it to %USERPROFILE% lol

It gets promoted perfectly. My commandline ends up being wsl -d "%USERPROFILE%" ...

This is promarily being done to unblock #9223. Prior to this, we'd validate that the user's `startingDirectory` existed here. If it was invalid, we'd gracefully fall back to `%USERPROFILE%`. However, that could cause hangs when combined with WSL. When the WSL filesystem is slow to respond, we'll end up waiting indefinitely for their filesystem driver to respond. This can result in the whole terminal becoming unresponsive. Similarly, with #9223 we want users to be able to specify WSL paths in a profile, but this bit of validation logic totally prevents that from working, because it'll just replace the path with `%USERPROFILE%`. If the path is eventually invalid, we'll display warning in the `ConptyConnection`, when the process fails to launch. Closes #9541 Closes #9114

This is primarily being done to unblock #9223. Prior to this, we'd validate that the user's `startingDirectory` existed here. If it was invalid, we'd gracefully fall back to `%USERPROFILE%`. However, that could cause hangs when combined with WSL. When the WSL filesystem is slow to respond, we'll end up waiting indefinitely for their filesystem driver to respond. This can result in the whole terminal becoming unresponsive. Similarly, with #9223 we want users to be able to specify WSL paths in a profile, but this bit of validation logic totally prevents that from working, because it'll just replace the path with `%USERPROFILE%`. If the path is eventually invalid, we'll display warning in the `ConptyConnection`, when the process fails to launch. Closes #9541 Closes #9114 ![image](https://user-images.githubusercontent.com/18356694/117318675-426d2d00-ae50-11eb-9cc0-0b23c397472c.png)

This is primarily being done to unblock #9223. Prior to this, we'd validate that the user's `startingDirectory` existed here. If it was invalid, we'd gracefully fall back to `%USERPROFILE%`. However, that could cause hangs when combined with WSL. When the WSL filesystem is slow to respond, we'll end up waiting indefinitely for their filesystem driver to respond. This can result in the whole terminal becoming unresponsive. Similarly, with #9223 we want users to be able to specify WSL paths in a profile, but this bit of validation logic totally prevents that from working, because it'll just replace the path with `%USERPROFILE%`. If the path is eventually invalid, we'll display warning in the `ConptyConnection`, when the process fails to launch. Closes #9541 Closes #9114 ![image](https://user-images.githubusercontent.com/18356694/117318675-426d2d00-ae50-11eb-9cc0-0b23c397472c.png) (cherry picked from commit bfc4838)

zadjii-msft · 2021-08-02T16:06:34Z

@DHowett do we want to try this on for 1.11? maybe even behind velocity if we're nervous?

DHowett · 2021-08-02T18:24:04Z

We are still normalizing these somewhere.

1:23:41.2299792 PM	WindowsTerminal.exe	77076
Process Create
C:\WINDOWS\system32\wsl.exe
SUCCESS
PID: 78492, Command line: "wsl.exe" --cd "C:/etc" -d Debian
                                         ^^^^^^^^

… already checked operators

fixes a stray \0

ghost · 2021-08-31T17:11:33Z

🎉Windows Terminal Preview v1.11.2421.0 has been released which incorporates this pull request.:tada:

Handy links:

LuanVSO · 2021-12-27T20:00:43Z

this change makes osc 9;9 work with linux paths but breaks \\wsl$ ones
docs at https://docs.microsoft.com/windows/terminal/tutorials/new-tab-same-directory will need to be updated

…2102) This PR does two things, which are best viewed as atomic commits: * e64ae7d: Move the `MangleStartingDirectoryForWSL` to `types/utils`. It doesn't _really_ make sense in `types`, since it's only really being used in a single place in TerminalConnection. However, TerminalConnection doesn't have tests, and types does. So this commit move the function there, and adds tests from #9223 to the types tests. * 42036c5: This actually fixes the bug in #11994. Unfortunately, `wsl --cd` will try to treat paths starting with `//wsl$` as a linux-relative path, when the user almost certainly wanted a windows-relative one. So we'll mangle that back into a path that looks like `\\wsl$\foo\bar`. * [x] closes #11994 * [x] I work here * [x] tests added 🎉

…2102) This PR does two things, which are best viewed as atomic commits: * e64ae7d: Move the `MangleStartingDirectoryForWSL` to `types/utils`. It doesn't _really_ make sense in `types`, since it's only really being used in a single place in TerminalConnection. However, TerminalConnection doesn't have tests, and types does. So this commit move the function there, and adds tests from #9223 to the types tests. * 42036c5: This actually fixes the bug in #11994. Unfortunately, `wsl --cd` will try to treat paths starting with `//wsl$` as a linux-relative path, when the user almost certainly wanted a windows-relative one. So we'll mangle that back into a path that looks like `\\wsl$\foo\bar`. * [x] closes #11994 * [x] I work here * [x] tests added 🎉 (cherry picked from commit b87b809)

zadjii-msft self-assigned this Mar 3, 2021

miniksa reviewed Mar 30, 2021

View reviewed changes

miniksa approved these changes Mar 30, 2021

View reviewed changes

zadjii-msft approved these changes Mar 30, 2021

View reviewed changes

zadjii-msft removed their assignment Mar 30, 2021

zadjii-msft assigned DHowett Apr 21, 2021

zadjii-msft added the AutoMerge Marked for automatic merge by the bot when requirements are met label Apr 21, 2021

This was referenced May 6, 2021

Disable path validation, add warning #10045

Merged

WSL home directory behaviour unexpected #7197

Closed

This was referenced May 27, 2021

startingDirectory being invalid shouldn't cause silent failures #10225

Closed

error when launching..Could not access starting directory #10245

Closed

DHowett added 2 commits August 2, 2021 12:00

Merge remote-tracking branch 'origin/main' into dev/duhowett/wsl_--cd

7bb34c3

fix the speeling

ad7b736

DHowett removed the AutoMerge Marked for automatic merge by the bot when requirements are met label Aug 2, 2021

DHowett added 3 commits August 2, 2021 13:46

Don't try to relative-resolve paths if they look WSLy

5c53dca

til::at doesn't work for hstring, and well... hstring's operators are…

4c82087

… already checked operators

Expand env using wil helpers

13a16f9

fixes a stray \0

zadjii-msft approved these changes Aug 2, 2021

View reviewed changes

DHowett added the AutoMerge Marked for automatic merge by the bot when requirements are met label Aug 2, 2021

ghost merged commit a2a6050 into main Aug 2, 2021

ghost deleted the dev/duhowett/wsl_--cd branch August 2, 2021 20:39

ghost mentioned this pull request Aug 31, 2021

Support linux paths for startingDirectory of WSL distros #592

Closed

zadjii-msft mentioned this pull request Sep 16, 2021

error 0x8007010b when launching `wsl -d Ubuntu-18.04 #11245

Closed

zadjii-msft mentioned this pull request Oct 6, 2021

~ not work as startingDirectory for WSL #11432

Closed

vbrozik mentioned this pull request Dec 20, 2021

After upgrade to 1.12.3472.0 startingDirectory with forward slashes does not work for WSL any more #11994

Closed

This was referenced Jan 3, 2022

We broke \\wsl$ paths with osc 9;9? #12085

Closed

Clarify the usage of Linux-style paths in 21H1 MicrosoftDocs/terminal#455

Closed

Fix //wsl$ paths not working in MangleStartingDirectoryForWSL #12102

Merged

zadjii-msft mentioned this pull request Feb 2, 2022

Changing Ubuntu WSL starting directory keeps it set to the defualt #2743

Closed

zadjii-msft mentioned this pull request May 2, 2022

Unable to launch WSL Ubuntu - The system cannot find the file specified. #12474

Closed

zadjii-msft mentioned this pull request May 12, 2023

Use a "virtual CWD" for each terminal window #15280

Merged

This pull request was closed.

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

When launching wsl, promote the starting directory to --cd #9223

When launching wsl, promote the starting directory to --cd #9223

DHowett commented Feb 19, 2021 •

edited

Loading

github-actions bot commented Feb 19, 2021

driver1998 commented Mar 30, 2021

DHowett commented Mar 30, 2021

miniksa Mar 30, 2021

zadjii-msft Mar 30, 2021

miniksa left a comment

zadjii-msft left a comment

zadjii-msft Mar 30, 2021

zadjii-msft Mar 30, 2021

zadjii-msft Mar 30, 2021

ghost commented Apr 21, 2021

zadjii-msft commented Apr 23, 2021 •

edited

Loading

DHowett commented Apr 23, 2021

DHowett commented Apr 23, 2021

zadjii-msft commented Aug 2, 2021

DHowett commented Aug 2, 2021 •

edited

Loading

ghost commented Aug 31, 2021

LuanVSO commented Dec 27, 2021

	{ // "wsl" is three characters; this is a safe bet. no point in doing it if there's no starting directory though!
	{
	// "wsl" is three characters; this is a safe bet. no point in doing it if there's no starting directory though!

When launching wsl, promote the starting directory to --cd #9223

When launching wsl, promote the starting directory to --cd #9223

Conversation

DHowett commented Feb 19, 2021 • edited Loading

Validation Steps Performed

github-actions bot commented Feb 19, 2021

Misspellings found, please review:

driver1998 commented Mar 30, 2021

DHowett commented Mar 30, 2021

miniksa Mar 30, 2021

Choose a reason for hiding this comment

zadjii-msft Mar 30, 2021

Choose a reason for hiding this comment

miniksa left a comment

Choose a reason for hiding this comment

zadjii-msft left a comment

Choose a reason for hiding this comment

zadjii-msft Mar 30, 2021

Choose a reason for hiding this comment

zadjii-msft Mar 30, 2021

Choose a reason for hiding this comment

zadjii-msft Mar 30, 2021

Choose a reason for hiding this comment

ghost commented Apr 21, 2021

p.s. you can customize the way I help with merging this pull request, such as holding this pull request until a specific person approves. Simply @mention me (@msftbot) and give me an instruction to get started! Learn more here.

zadjii-msft commented Apr 23, 2021 • edited Loading

DHowett commented Apr 23, 2021

DHowett commented Apr 23, 2021

zadjii-msft commented Aug 2, 2021

DHowett commented Aug 2, 2021 • edited Loading

ghost commented Aug 31, 2021

LuanVSO commented Dec 27, 2021

DHowett commented Feb 19, 2021 •

edited

Loading

p.s. you can customize the way I help with merging this pull request, such as holding this pull request until a specific person approves. Simply @mention me (`@msftbot`) and give me an instruction to get started! Learn more here.

zadjii-msft commented Apr 23, 2021 •

edited

Loading

DHowett commented Aug 2, 2021 •

edited

Loading