Added ShouldProcess support to Start-PSBootstrap #15548
Closed
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Another fix for #15490. For your consideration.
Previously, the command sometimes did declare SupportsShouldProcess, but didn’t actually implement it.
Because of the fact that Start-NativeExecution is used with complex script blocks, it wasn’t possible to simply implement SupportsShouldProcess in Start-NativeExecution.
By implementing the ShouldProcess logic for each case individually, we’re also able to provide more helpful messages to the user.
I have not implemented SupportsShouldProcess for the other scripts that are called by this function. I do not have the patience, and I do not believe it is worth it.
In particular, I think that this function is in dire need of -WhatIf support, since its name (“Start-PSBootstrap”) doesn’t initially suggest to the user that (potentially irreversible) changes to the system configuration will be made. This is not the case with things like Install-Dotnet and install-powershell.ps1, which have far more revealing names.
I believe the current code is pretty DRY. Maybe the call to $PSCmdlet.ShouldProcess could be implemented in a subroutine, since text is often repeated?
I’ve also made some changes in some places to make the code better suitable for this feature. In particulare, the Gem installation routine is now run using a loop over a table.
In looking at the code, I have come to question the necessity of many of the calls to Start-NativeExecution. It’s unclear what error handling it provides and if it should be used to run more complex script blocks.
AMEND:
I have tested this code on Pop!_OS 20.10. I can’t test it on Windows since I don’t believe my Windows license is valid anymore, and I’m not in the mood to comit crimes for software development. This is of course suboptimal as the parent commits don’t seem to pass the unit tests either.
However, since much of the code is simply repeated with only messages swapped out, I feel that it is likely to work.
I haven’t tested the code rigorously. Please treat with caution.