-
Notifications
You must be signed in to change notification settings - Fork 44
New DSC resource to install tools from dotnet globally #63
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
Conversation
|
@denelon Hi Demitrius, I had gone through the |
|
Yes please. Any additional documentation will be helpful as users become informed about the new resource. |
|
It might also be interesting to get you connected to some of the dotnet folks to discuss making dotnet.exe DSC v3 capable. I saw you're writing an e-book on DSC v3 in your GitHub profile. |
|
We have been discussing a DSC Resource for dotnet recently. I would have thought about using a module name like "Microsoft.dotnet.Dsc" or something like that. I just don't have enough domain knowledge on dotnet to make the best recommendation. |
Will do when I have some time. In the meantime, if you have any suggestions or best practices how you want the docs to look like, just give me a headsup :) |
Would love that Demitrius. Great challenge to look at. Indeed I am. Currently storing the content from preview.10 and asked Gael to have a review/foreword. Then probably when dsc gets GA, I'll put it fully live. |
I would recon a naming change. It was just as first thought that popped up. |
|
Related to: |
| RootModule = 'Microsoft.NET.SDK.ToolInstaller.psm1' | ||
| ModuleVersion = '0.1.0' | ||
| GUID = '2e883e78-1d91-4d08-9fc1-2a968e31009d' | ||
| Author = 'Microsoft Corporation' |
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.
@denelon - Is there an ownership concept with DSCs similar to Winget packages? How do you ensure the validity of the author and that they have sole ownership?
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.
Since this is an OSS repository, we'd consider PRs from the community, but since this is a Microsoft "package" the resource would require our approval to make sure we're good with the implementation. Anything published to the PowerShell gallery from here would need that validation before we would sign and publish the module.
resources/Microsoft.NET.SDK.ToolInstaller/Microsoft.NET.SDK.ToolInstaller.psm1
Outdated
Show resolved
Hide resolved
resources/Microsoft.NET.SDK.ToolInstaller/Microsoft.NET.SDK.ToolInstaller.psm1
Outdated
Show resolved
Hide resolved
| return $dotNetPath | ||
| } | ||
|
|
||
| # TODO: when https://github.com/dotnet/sdk/pull/37394 is documented and version is released with option simple use --format=JSON |
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.
The linked PR is fixed in 9 it looks like. Were you waiting for 9 to be the default to replace the below code?
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.
You are right, I forgot I couldn't install the latest preview from the official website. Everytime when I tried downloading, it gives me 0 bytes. When I used winget install Microsoft.DotNet.SDK.Preview, it did. I guess it installs from a different location.
Eventually, I guess we can move the logic to this:
But we should also be able to support older versions of the SDK?
| process | ||
| { | ||
| $installArgument = Get-DotNetToolArguments @PSBoundParameters | ||
| $arguments = "tool install $installArgument --global --ignore-failed-sources" |
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.
are all of the tools you're installing supported on the current runtimes that are installed? if not, there may be issues with running those tools.
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 agree with your statement here, however I do think users themselves should be aware what they should install. I know the architecture option is also available to be specified. It can be added later I would say. Let me know your thoughts.
| } | ||
|
|
||
| Set() { | ||
| # TODO: validate for upgrade/update scenarios |
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.
what is the update model for this?
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.
Let's imagine the following scenario. The user has already installed PowerShell version 7.4.4. The user now wants version 7.4.5. The user leaves empty the version property. When the user runs the configuration, it needs to check if 7.4.4 is installed and if so, update it with dotnet tool update. @ryfu-msft Would you agree with such logic, or should we add a boolean parameter to upgrade?
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.
Agreed. If a package has a version that does not match the desired version, then this DSC resource should handle updating the package to match the desired state.
| It 'Sets desired package with prerelease version' -Skip:(!$IsWindows) { | ||
| $parameters = @{ | ||
| PackageId = 'PowerShell' | ||
| Version = '7.2.0-preview.5' |
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.
why this version of powershell?
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.
If I'm not mistaken, the logic of the CLI to install packages is as followed:
- Whenever a preview version is higher than the official version, you require the
--prereleaseparameter to be added without version - When the official version is higher than a preview version, you use the
--versionparameter to indicate you want to install a prerelease version that is older. Because you want to test for this scenario, I have included an older version of PowerShell. See list:
|
@denelon In the latest commit, I have added the option for toolsPath. This allows users to choose a directory to install tools towards. However, the latest pull request for I have tried to incorporate as much of what Mikey sketched out in this change. It probably requires updating the P.S. After thinking about it, |
I'd suggest trying with the experimental WinGet feature something like:
I know this is brutal with class-based v2 modules, but it gives us another working example and helps pave the way for DSC v3. We're investing into improvements for the export feature to capture all installed packages, Windows Settings, and any DSC v3 resources (soon to be identified in WinGet package manifests). |
resources/Microsoft.NET.SDK.ToolInstaller/Microsoft.NET.SDK.ToolInstaller.psd1
Outdated
Show resolved
Hide resolved
resources/Microsoft.NET.SDK.ToolInstaller/Microsoft.NET.SDK.ToolInstaller.psm1
Outdated
Show resolved
Hide resolved
| } | ||
|
|
||
| Set() { | ||
| # TODO: validate for upgrade/update scenarios |
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.
Agreed. If a package has a version that does not match the desired version, then this DSC resource should handle updating the package to match the desired state.
resources/Microsoft.NET.SDK.ToolInstaller/Microsoft.NET.SDK.ToolInstaller.psm1
Outdated
Show resolved
Hide resolved
resources/Microsoft.NET.SDK.ToolInstaller/Microsoft.NET.SDK.ToolInstaller.psm1
Outdated
Show resolved
Hide resolved
resources/Microsoft.NET.SDK.ToolInstaller/Microsoft.NET.SDK.ToolInstaller.psm1
Outdated
Show resolved
Hide resolved
resources/Microsoft.NET.SDK.ToolInstaller/Microsoft.NET.SDK.ToolInstaller.psm1
Outdated
Show resolved
Hide resolved
resources/Microsoft.NET.SDK.ToolInstaller/Microsoft.NET.SDK.ToolInstaller.psm1
Outdated
Show resolved
Hide resolved
|
@ryfu-msft Ryan, sorry for the many commits. I had to re-name the module and figure things out with Semantic Versioning. I have tried to prepare the test to run on the ADO agents. Inside the Cheers and have a good weekend already. |
|
/azp run |
|
Azure Pipelines successfully started running 1 pipeline(s). |
| else | ||
| { | ||
| # Don't want to downgrade scenarios | ||
| Throw "The package version cannot be less than the installed version. Please provide a version greater than or equal to the installed version." | ||
| } |
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.
We shouldn't be throwing on a Get() call. I also believe that we should still support downgrade scenarios even if this is not a common scenario. DSC should be able to set any desired state even if that involves downgrading.
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.
Please check once again. The logic is a bit different. Check out the GitHub issue linked at the function. Not all SDKs have the downgrade option ;)
|
/azp run |
|
Azure Pipelines successfully started running 1 pipeline(s). |
|
/azp run |
|
Azure Pipelines successfully started running 1 pipeline(s). |
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.
Look great! Will work on getting this published to the gallery. Thank you so much for all your hard work on this! 😄
|
And thanks for all the feedback! Please let me know when you get it published |
|
@denelon Thanks for letting me know. I'll try to put out some docs tomorrow and spend some love in some upcoming blogs I'm writing. Curious to your thoughts. Cheers. |
|
@ryfu-msft and I were chatting about the best place to put docs here, and a sample over at https://aka.ms/dsc.yaml inside the Dev Home repository. We could also consider centralizing examples here now that this repository is open to the public. That would allow me to change the redirect to this repository. |


This pull request introduces a new DSC resource. It allows users to install .NET tools globally.