I don’t think implementing ShouldProcess on this function is doable, as it downloads and executes an external script via Install-Dotnet.

This pull request has been automatically marked as Review Needed because it has been there has not been any activity for 7 days.
Maintainer, please provide feedback and/or mark it as Waiting on Author

@schuelermine This function could simply gate the running of that script behind a ShouldProcess check itself, no?

True, should I re-add SupportShouldProcess and only launch the script then?

Might be worth doing, yeah. Not sure if this function handles other things that would warrant a similar check for those parts as well?

ok, I will write that when I'm back home (3hrs from now)

/azp rerun

/azp run

@vexx32 While I am in support of this PR being merged, I have also opened another one w/ an attempt at implementing ShouldProcess. An alternate option, if you will.

@schuelermine it looks like all the CIs are failing for this PR and not in the master branch, and I can't merge it until CI passes. I took a brief look and it's not entirely clear what's going on, but it does look possibly related to your change (Start-PSBootstrap isn't properly bootstrapping .NET 6)

Alternately solved by #15548

I don’t think the failing is caused by me—here’s a copy of my reasoning from the commit message:

I’m not sure why the build would fail — I hadn’t changed anything besides removing the CmdletBindingAttribute parameters…
I’ve checked if build.psm1 or ci.psm1 (which is where the errors semm to be happening) ever mention “WhatIf” or “ShouldProcess” — they don’t.

ci.psm1’s Invoke-CIBuild seems to be failing because Get-PSOptions (imported from build.psm1) returns $null.
However, this is because of a global variable set at the top of the script, which was there before my changes occurred.

OK this bothers me - why does the Bootstrap step show passed when it clearly failed. I just spent weeks fixing a similar issue with the Jenkins durable-task-plugin. 🤦‍♂️

Looks like someone will need to update the Action to tweak that -Confirm:$false parameter. Then again, the latest update to this PR put back the ConfirmImpact="High". Maybe there's an interaction between SupportsShouldProcess and ConfirmImpact?

Well this would 'splain it:

This would be a better impl:

Not only does the current impl not fail on missing parameters, it will not fail on a missing command or invalid parameter values:

This is the exact same set of issues I went through with the pwsh step in the Jenkins plugin. Deja vu!

I can't look through the code right now, I'm not at my computer right now - but it seems like some function might've called Start-PSBootstrap w/ -WhatIf:$WhatIfPreference or something, and then the rest of the code mishandled its failed execution.

OK, yeah, that's definitely the case - Start-CIInstall has this line:

Start-PSBootstrap -Confirm:$false

(can't remove it now, will do soonish)

Oh, I just read that you found that, too. Sorry for making that duplicate observation.

I think removing ConfirmImpact in build.psm1 and adding a $ConfirmPreference declaration to ci.psm1 is a good solution - gonna do that when I'm home.

Actually, $ConfirmPreference probably isn't even necessary, since Start-PSBootstrap never calls ShouldProcess or ShouldContinue.
It might've even been because of the incorrect SupportsShouldProcess declaration that the Confirm:$false parameter was added

Weird

Intermittent PSGallery glitch?

Intermittent PSGallery glitch?

Yes, I restarted CI 5 minutes later and it passed.

🎉v7.2.0-preview.7 has been released which incorporates this pull request.:tada:

Handy links:

• Release Notes