-
Notifications
You must be signed in to change notification settings - Fork 337
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
Add WSL extension to Dev Home #3365
Conversation
extensions/WSLExtension/DistributionDefinitions/DistributionDefinitionHelper.cs
Outdated
Show resolved
Hide resolved
tools/SetupFlow/DevHome.SetupFlow/Views/Environments/SelectEnvironmentProviderView.xaml
Outdated
Show resolved
Hide resolved
extensions/WSLExtension/Models/WslInstallDistributionOperation.cs
Outdated
Show resolved
Hide resolved
extensions/WSLExtension/DistributionDefinitions/DistributionDefinitionHelper.cs
Outdated
Show resolved
Hide resolved
extensions/WSLExtension/DistributionDefinitions/DistributionDefinitionHelper.cs
Show resolved
Hide resolved
extensions/WSLExtension/DistributionDefinitions/DistributionDefinitionHelper.cs
Outdated
Show resolved
Hide resolved
extensions/WSLExtension/Models/WslInstallDistributionOperation.cs
Outdated
Show resolved
Hide resolved
extensions/WSLExtension/DistributionDefinitions/DistributionDefinition.yaml
Outdated
Show resolved
Hide resolved
extensions/WSLExtension/Models/RegisterAndInstallDistributionSession.cs
Outdated
Show resolved
Hide resolved
|
||
<!--- Data templates for custom adaptive card renders can go here --> | ||
<converters:EmptyObjectToObjectConverter x:Key="EmptyObjectToObjectConverter" NotEmptyValue="Visible" EmptyValue="Collapsed"/> |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
NIT: rename the key name to EmptyObjectToVisibility
Converter
|
||
// Launch terminal with specific profile and log the user into their home directory in the login shell | ||
// Note: this opens a new terminal window in the UI | ||
public static string LaunchDistributionInTerminalWithProfile { get; } = "--profile {0} -- wsl --shell-type login --cd ~ --distribution {1}"; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
NIT for this and the other strings. Explain what the placeholders are for. This way calling code does not need to look at the string.
Another option is to remove the properties all together and change these into methods. Then, the code can describe the format with parameters.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I don't think it would make sense to make them into methods, because in the future when the WSL team provides us with apis then we'd need to remove/change those methods. Today we're using wsl.exe under the hood, but I've made it in a way where, when we get new apis, only the body of the methods that use these strings need to change, not the method themselves.
Ultimately if you typed in wsl --help
in your command line window you'll see what these commands are used for but I can add it to the comments if you feel strongly about it.
extensions/WSLExtension/DistributionDefinitions/DistributionDefinition.yaml
Outdated
Show resolved
Hide resolved
extensions/WSLExtension/DistributionDefinitions/DistributionDefinitionHelper.cs
Show resolved
Hide resolved
extensions/WSLExtension/DistributionDefinitions/DistributionDefinitionHelper.cs
Outdated
Show resolved
Hide resolved
extensions/WSLExtension/DistributionDefinitions/DistributionDefinitionHelper.cs
Show resolved
Hide resolved
extensions/WSLExtension/Models/RegisterAndInstallDistributionSession.cs
Outdated
Show resolved
Hide resolved
Please run through all your examples with text scaling set to 200%. |
@dhoehna , just confirmed at 225%, everything still looks good with no issues. |
Just popping in to say thank you for this :) |
Summary of the pull request
This PR adds the WSL extension to Dev Home. We'd like to thank @crramirez for allowing us to use his original Dev Home WSL extension as a base for this official extension.
Now on to changes:
- Added the WSLExtension project under the extensions folder
- Added the COM extension guid information to the appxmanifest file
- Added the WSL extension guid to the set for internal guids in the extensions library so it can be enabled/disabled.
- Added template specifically for settings cards used in environment creation to allow for bigger icons.
- Added template in environment project to allow a single button to be shown when there are no launch operations available.
WSL extension architecture:
First thing to note, is that the WSL API surface is very limited and so we will be working with the WSL team to provide us with a APIs in the future to perform all WSL tasks. Until then our approach here will be to use a mix of calling wsl.exe (the only way to interact with wsl through code) and reading from the registry. There are technically a small number of apis listed here: Wslapi.h header - Win32 apps | Microsoft Learn, but it is very limited and won't be updated like how wsl.exe is continuously being updated, so we won't be using those.
Sequence diagram that shows flow for retrieving registered WSL distributions (Click the <--> button for a larger view) :
Sequence diagram that shows flow for creating/installing a WSL distribution (Click the <--> button for a larger view):
Videos:
Video of me creating a WSL compute system:
creating.ubuntu.distribution.mp4
Video of me launching a WSL compute system:
ubuntu.launch.mp4
Video of me Terminating all sessions a WSL compute system:
ubuntu.terminate.mp4
Video of me Deleting a WSL compute system:
debian.delete.mp4
References and relevant issues
Detailed description of the pull request / Additional comments
Validation steps performed
Confirmed that the Hyper-v environment and dev box environment are still working as intended. Also confirmed no regressions in other parts of Dev Home.
PR checklist