-
Notifications
You must be signed in to change notification settings - Fork 1k
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
Support installing workloads in user folder #18823
Conversation
I couldn't figure out the best area label to add to this PR. If you have write-permissions please help me learn by adding exactly one area label. |
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.
Thanks for getting this started! This looks like it covers the basics of reading workloads from a user local folder.
Were you also planning to update the installation logic so that it would install workloads to the user local folder?
src/Resolvers/Microsoft.DotNet.MSBuildSdkResolver/MSBuildSdkResolver.cs
Outdated
Show resolved
Hide resolved
src/Resolvers/Microsoft.NET.Sdk.WorkloadManifestReader/SdkDirectoryWorkloadManifestProvider.cs
Outdated
Show resolved
Hide resolved
src/Resolvers/Microsoft.NET.Sdk.WorkloadManifestReader/WorkloadResolver.cs
Outdated
Show resolved
Hide resolved
src/Tests/Microsoft.DotNet.TemplateLocator.Tests/GivenAnTemplateLocator.cs
Outdated
Show resolved
Hide resolved
Hey @tmds, any updates on this? Let us know if you need anything. |
Hey, @dsplaisted, just an fyi that @tmds is on vacation for about 2 more weeks. It's unlikely that there will be more progress on this until he gets back. |
@omajid Thanks for the heads up. To give you a sense of schedule, the SDK does not lock down as early as the runtime so we have some more time for .NET 6 but not a ton. We'll be asked to start locking down on changes going into the SDK around mid-September so it'd be good to have this ready by then if you're targeting 6.0.100. |
@dsplaisted I've updated the PR based on your feedback. Can you take a look? There may be some logical mistakes as I don't fully understand the relationship between the I put some things in the "Common" folder but you may prefer them in another location. I've limited the |
@dsplaisted I'm looking forward to get some feedback here. |
src/Tasks/Microsoft.NET.Build.Tasks/ProcessFrameworkReferences.cs
Outdated
Show resolved
Hide resolved
src/Cli/dotnet/commands/dotnet-workload/install/NetSdkManagedInstaller.cs
Outdated
Show resolved
Hide resolved
src/Cli/dotnet/commands/dotnet-workload/install/WorkloadInstallerFactory.cs
Outdated
Show resolved
Hide resolved
src/Cli/dotnet/commands/dotnet-workload/install/WorkloadInstallerFactory.cs
Outdated
Show resolved
Hide resolved
Thanks, I'm going to try to review this soon! |
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.
This is looking great, thanks a bunch!
As discussed, this should be rebased onto the release/6.0.1xx
branch (which currently targets rc2).
@sfoslund Do you think we can / should add automated tests for user local workload installs?
src/Cli/dotnet/commands/dotnet-workload/install/NetSdkManagedInstaller.cs
Outdated
Show resolved
Hide resolved
src/Cli/dotnet/commands/dotnet-workload/install/NetSdkManagedInstaller.cs
Outdated
Show resolved
Hide resolved
src/Cli/dotnet/commands/dotnet-workload/install/NetSdkManagedInstaller.cs
Outdated
Show resolved
Hide resolved
...s/dotnet-workload/install/WorkloadInstallRecords/NetSdkManagedInstallationRecordInstaller.cs
Outdated
Show resolved
Hide resolved
src/Cli/dotnet/commands/dotnet-workload/install/WorkloadInstallerFactory.cs
Outdated
Show resolved
Hide resolved
src/Resolvers/Microsoft.NET.Sdk.WorkloadManifestReader/SdkDirectoryWorkloadManifestProvider.cs
Outdated
Show resolved
Hide resolved
src/Resolvers/Microsoft.NET.Sdk.WorkloadManifestReader/WorkloadResolver.cs
Outdated
Show resolved
Hide resolved
src/Tasks/Microsoft.NET.Build.Tasks/ProcessFrameworkReferences.cs
Outdated
Show resolved
Hide resolved
Yes, I think that would be great. You could model the tests off of this one: https://github.com/dotnet/sdk/blob/main/src/Tests/dotnet-workload-install.Tests/GivenDotnetWorkloadInstall.cs#L249 and feel free to ping me if you have any questions. |
@dsplaisted thanks for the review. |
I have changed |
…nstaller.cs Co-authored-by: Daniel Plaisted <dsplaisted@gmail.com>
…dFileBasedInstall.
@dsplaisted @sfoslund can you do another review? I did an integration test as follows:
This succesfully installed |
@dsplaisted what would be a good place to create the |
|
Under what conditions should the userlocal file be created? |
I believe it should be included in all source-built versions of the .NET SDK. That means that when they install workloads the workloads will always be written to a user-local folder. For .NET 7 we may need to adjust how this works, as there will probably be some workloads that themselves should be source-built, and not necessarily installed in the user folder. |
Yes. The file should be created under the dotnet root at
It would be good to write/update the design doc to be sure different scenarios are covered for source-build. |
Thanks a bunch @tmds. 🎉🎉🎉 Could you write a short description of the paths that this uses? IE what it will use as the user local folder, including the various fallbacks on different OS's, as well as where it looks for the marker file. |
@dsplaisted no problem. I'm happy we can have a nice UX also for source-build .NET when using workloads. Thanks you and @sfoslund for the quick reviews.
When the dotnet installation has a file at The folders that used for workloads under
|
Can the file be created when the sdk layout is created? The way to get it only for source-built versions is to condition it on |
@omajid the
If it can place the What causes the blazor manifest file to be added in source-build? |
Add /opt/dotnet-sdk-bin-6.0/metadata/workloads/6.0.100/userlocal file which used to indicate which workloads are installed. Without that file Nuget think this is broken installation and attempt to create that file which become a problem for ebuilds which depends on dotnet. See: dotnet/sdk#18823 (comment) Closes: #23182 Closes: https://bugs.gentoo.org/827712 Signed-off-by: Andrii Kurdiumov <kant2002@gmail.com> Signed-off-by: Zac Medico <zmedico@gentoo.org>
@tmds I am working in a very restricted environment and need to have workloads stored in my Linux userpath. How can I use this to make that happen? |
@lmcwhirt-insight consider opening an issue for this use-case in the repo. A source-built SDK has a file at |
Fixes #18104
@dsplaisted I've started with the plumbing to get the user location into the commands. Can you take a look and give feedback? Especially about the 4 places I marked with
TODO
.