vexx32 / PSKoans

A simple, fun, and interactive way to learn the PowerShell language through Pester unit testing.
GNU General Public License v3.0
1.72k stars 176 forks source link

Fix koan AboutPsProviders #436

Closed peetrike closed 3 years ago

peetrike commented 3 years ago

PR Summary

Fixes 3 problems in koan AboutPsProviders

Context

Changes

Checklist

vexx32 commented 3 years ago

@peetrike I had a look at it, and I think the "neatest" way around the error here is to use Set-Variable to create the variable instead of using the normal syntax. That seems to have a similar effect, and doesn't require adding the conditional Remove-Variable that you have in your current solution. I think that will be less confusing to users in the long run.

Something in the way PSKoans is invoking that code is causing PowerShell to optimize that code path in certain cases. @seeminglyscience could probably explain what on earth is going on with it, but it's a bit beyond me as to exactly why it's occurring.

peetrike commented 3 years ago

will change the last solution. It is indeed cleaner :)

SeeminglyScience commented 3 years ago

Something in the way PSKoans is invoking that code is causing PowerShell to optimize that code path in certain cases. @SeeminglyScience could probably explain what on earth is going on with it, but it's a bit beyond me as to exactly why it's occurring.

So there are basically two ways a variable is stored internally:

  1. in a Dictionary<string, PSVariable>. This is the "unoptimized" route, taken when dot sourcing or when the debugger is active. Maybe also when code is interpreted rather than JIT compiled
  2. In a generated MutableTuple<..> that is basically a ITuple implementation with a property representing every local for the scope

Those errors occur most often when trying to remove a variable from a previous scope that is represented by a mutable tuple entry.

peetrike commented 3 years ago

does it mean that in scriptblock, directory assigning to variable makes it "optimized"? And later when you try to remove it through Variable: drive, it will be prevented?

vexx32 commented 3 years ago

I've seen it before removing variables from other scopes, but there even if it used a completely different variable name than anything else it still hit it. Really weird.

peetrike commented 3 years ago

seems so:

& { $MyTest = 11; $MyTest ; Remove-Item variable:MyTest }
Remove-Item: Cannot remove variable MyTest because the variable has been optimized and is not removable. Try using the Remove-Variable cmdlet (without any aliases), or dot-sourcing the command that you are using to remove the variable.
peetrike commented 3 years ago

and same in PowerShell 5.1

peetrike commented 3 years ago

maybe that could be included as another It block in that Koan?

vexx32 commented 3 years ago

Sounds like we'll need to, yeah 😁

peetrike commented 3 years ago

or maybe it fits better into AboutVariables koan?

vexx32 commented 3 years ago

I think it's better off in this one, because Remove-Variable will still work, it's only the provider access via variable: with Remove-Item that fails.

SeeminglyScience commented 3 years ago

seems so:

& { $MyTest = 11; $MyTest ; Remove-Item variable:MyTest }

It's less to do with the fact that it's in a scriptblock and more about the invocation operator. The compiler won't optimize when dot sourcing, which is the default in an interactive session. So forcing it out of dot sourcing using & rather than . is what makes the difference there.

vexx32 commented 3 years ago

Hmm. Do you think there's a way to modify https://github.com/vexx32/PSKoans/blob/main/PSKoans/Private/Invoke-Koan.ps1 to avoid this issue in future?

SeeminglyScience commented 3 years ago

Not really, I'd just enclose that it in a dot source to force not optimized I guess.

        . {
            $Test = 123

            __ | Should -Be $Test

            Remove-Item -Path 'Variable:\Test'
            $____ | Should -Be $Test
            { Get-Item -Path 'Variable:\Test' -ErrorAction Stop } | Should -Throw -ExceptionType ____
        }

And outside of demo purposes never use ri with the variable provider ofc 😁

vexx32 commented 3 years ago

That would probably be even less clear for users 😁

Might be good as an example for a different topic though lol. Ah well, at least we can help clarify how that works for folks to some degree 🤔

SeeminglyScience commented 3 years ago

Yeah I should have clarified that example is more meant as inspiration than something that should be actually done. It's pretty incomprehensible even with a lengthy explanation

peetrike commented 3 years ago

and that works without problems:

& {New-Item -Path variable: -Name Test -Value 11; $Test; Remove-Item variable:Test }
peetrike commented 3 years ago

not that someone shoud do that :)

SeeminglyScience commented 3 years ago

And if you're wondering why that works, it's because the compiler doesn't know it's a local. New-Item could be proxied, drive could be replaced (maybe?) etc etc. All it knows is literal variable assignments.

peetrike commented 3 years ago

so does it mean that only using assignment in separate scope triggers that "optimization" of variable?

peetrike commented 3 years ago

Yeah I should have clarified that example is more meant as inspiration than something that should be actually done. It's pretty incomprehensible even with a lengthy explanation

it would be more practical to give people "good" examples, not "bad" ones :)

vexx32 commented 3 years ago

Honestly my biggest point of confusion here is why does Remove-Variable work when remove-item doesn't? Why can't Remove-Item do what the other cmdlet is doing for that provider?

peetrike commented 3 years ago

Honestly my biggest point of confusion here is why does Remove-Variable work when remove-item doesn't? Why can't Remove-Item do what the other cmdlet is doing for that provider?

probably because Remove-Variable is meant to deal with variables, but going through provider makes it different

SeeminglyScience commented 3 years ago

so does it mean that only using assignment in separate scope triggers that "oütimization" of variable?

It needs to not be dot sourced if that's what you mean.

Honestly my biggest point of confusion here is why does Remove-Variable work when remove-item doesn't?

Either oversight or harder to do with the extra abstraction. Not sure which it is.

Why can't Remove-Item do what the other cmdlet is doing for that provider?

Probably can

peetrike commented 3 years ago

hm:

& {new-variable -name test -Scope local -Value 11; $test2=12; $test2; Remove-Item variable:test2 }
12
SeeminglyScience commented 3 years ago

So it looks like the compiler sees a *variable command and just nopes out of optimization. That explains why one works and the other doesn't, and I think you're right @peestrike, can't fix ri the same way.

Especially since this throws:

& {& (gcm new-variable) test; $test2=12; $test2; Remove-Item variable:test2 }
vexx32 commented 3 years ago

So it's just completely cursed. Good to know, thanks!

peetrike commented 3 years ago

They have been thought about that use case:

& {$ErrorActionPreference = 'SilentlyContinue'; $test2=12; Remove-Item variable:test2 }
$error[0].FullyQualifiedErrorId
VariableNotRemovableRare,Microsoft.PowerShell.Commands.RemoveItemCommand

VariableNotRemovableRare :)