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

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

Merged
6 commits merged into from
Aug 2, 2021
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
14 changes: 10 additions & 4 deletions src/cascadia/TerminalApp/TerminalPage.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -837,13 +837,19 @@ namespace winrt::TerminalApp::implementation
// construction, because the connection might not spawn the child
// process until later, on another thread, after we've already
// restored the CWD to it's original value.
std::wstring cwdString{ wil::GetCurrentDirectoryW<std::wstring>() };
std::filesystem::path cwd{ cwdString };
cwd /= settings.StartingDirectory().c_str();
winrt::hstring newWorkingDirectory{ settings.StartingDirectory() };
if (newWorkingDirectory.size() <= 1 ||
!(newWorkingDirectory[0] == L'~' || newWorkingDirectory[0] == L'/'))
{ // We only want to resolve the new WD against the CWD if it doesn't look like a Linux path (see GH#592)
std::wstring cwdString{ wil::GetCurrentDirectoryW<std::wstring>() };
std::filesystem::path cwd{ cwdString };
cwd /= settings.StartingDirectory().c_str();
newWorkingDirectory = winrt::hstring{ cwd.wstring() };
}

auto conhostConn = TerminalConnection::ConptyConnection();
conhostConn.Initialize(TerminalConnection::ConptyConnection::CreateSettings(settings.Commandline(),
winrt::hstring{ cwd.wstring() },
newWorkingDirectory,
settings.StartingTitle(),
envMap.GetView(),
::base::saturated_cast<uint32_t>(settings.InitialRows()),
Expand Down
71 changes: 69 additions & 2 deletions src/cascadia/TerminalConnection/ConptyConnection.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -57,6 +57,72 @@ namespace winrt::Microsoft::Terminal::TerminalConnection::implementation
return S_OK;
}

// Function Description:
// - Promotes a starting directory provided to a WSL invocation to a commandline argument.
// This is necessary because WSL has some modicum of support for linux-side directories (!) which
// CreateProcess never will.
static std::tuple<std::wstring, std::wstring> _tryMangleStartingDirectoryForWSL(std::wstring_view commandLine, std::wstring_view startingDirectory)
{
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!
Copy link
Member

Choose a reason for hiding this comment

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

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!

// Find the first space, quote or the end of the string -- we'll look for wsl before that.
const auto terminator{ commandLine.find_first_of(LR"(" )", 1) }; // look past the first character in case it starts with "
const auto start{ til::at(commandLine, 0) == L'"' ? 1 : 0 };
const std::filesystem::path executablePath{ commandLine.substr(start, terminator - start) };
const auto executableFilename{ executablePath.filename().wstring() };
if (executableFilename == L"wsl" || executableFilename == L"wsl.exe")
{
// We've got a WSL -- let's just make sure it's the right one.
if (executablePath.has_parent_path())
{
std::wstring systemDirectory{};
if (FAILED(wil::GetSystemDirectoryW(systemDirectory)))
{
break; // just bail out.
}
if (executablePath.parent_path().wstring() != systemDirectory)
{
break; // it wasn't in system32!
}
}
else
{
// assume that unqualified WSL is the one in system32 (minor danger)
}

const auto arguments{ terminator == std::wstring_view::npos ? std::wstring_view{} : commandLine.substr(terminator + 1) };
if (arguments.find(L"--cd") != std::wstring_view::npos)
{
break; // they've already got a --cd!
}

const auto tilde{ arguments.find_first_of(L'~') };
if (tilde != std::wstring_view::npos)
{
if (tilde + 1 == arguments.size() || til::at(arguments, tilde + 1) == L' ')
{
// 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)
Copy link
Member

Choose a reason for hiding this comment

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

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

Copy link
Member

Choose a reason for hiding this comment

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

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)

}

return {
fmt::format(LR"("{}" --cd "{}" {})", executablePath.wstring(), startingDirectory, arguments),
std::wstring{}
};
}
}
} while (false);

return {
std::wstring{ commandLine },
std::wstring{ startingDirectory }
};
}

// Function Description:
// - launches the client application attached to the new pseudoconsole
HRESULT ConptyConnection::_LaunchAttachedClient() noexcept
Expand Down Expand Up @@ -163,11 +229,12 @@ namespace winrt::Microsoft::Terminal::TerminalConnection::implementation
siEx.StartupInfo.lpTitle = mutableTitle.data();
}

const wchar_t* const startingDirectory = _startingDirectory.size() > 0 ? _startingDirectory.c_str() : nullptr;
auto [newCommandLine, newStartingDirectory] = _tryMangleStartingDirectoryForWSL(cmdline, _startingDirectory);
const wchar_t* const startingDirectory = newStartingDirectory.size() > 0 ? newStartingDirectory.c_str() : nullptr;

RETURN_IF_WIN32_BOOL_FALSE(CreateProcessW(
nullptr,
cmdline.data(),
newCommandLine.data(),
nullptr, // lpProcessAttributes
nullptr, // lpThreadAttributes
false, // bInheritHandles
Expand Down
7 changes: 1 addition & 6 deletions src/cascadia/TerminalSettingsModel/Profile.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -448,11 +448,6 @@ winrt::Microsoft::Terminal::Settings::Model::FontConfig Profile::FontInfo()
// - the function returns an evaluated version of %userprofile% to avoid blocking the session from starting.
std::wstring Profile::EvaluateStartingDirectory(const std::wstring& directory)
{
// First expand path
DWORD numCharsInput = ExpandEnvironmentStrings(directory.c_str(), nullptr, 0);
std::unique_ptr<wchar_t[]> evaluatedPath = std::make_unique<wchar_t[]>(numCharsInput);
THROW_LAST_ERROR_IF(0 == ExpandEnvironmentStrings(directory.c_str(), evaluatedPath.get(), numCharsInput));

// Prior to GH#9541, we'd validate that the user's startingDirectory existed
// here. If it was invalid, we'd gracefully fall back to %USERPROFILE%.
//
Expand All @@ -463,7 +458,7 @@ std::wstring Profile::EvaluateStartingDirectory(const std::wstring& directory)
//
// If the path is eventually invalid, we'll display warning in the
// ConptyConnection when the process fails to launch.
return std::wstring(evaluatedPath.get(), numCharsInput);
return wil::ExpandEnvironmentStringsW<std::wstring>(directory.c_str());
}

// Function Description:
Expand Down