dahlbyk / posh-git

A PowerShell environment for Git
http://dahlbyk.github.io/posh-git/
MIT License
7.62k stars 799 forks source link

Prompt script block System.NullReferenceException #602

Open annahosanna opened 6 years ago

annahosanna commented 6 years ago

Posh-git installed via Chocolatey

After posh-git install Powershell prompt appeared as: PS>

It seems that there was an issue setting currentPath within the prompt script block: $currentPath = $ExecutionContext.SessionState.InvokeCommand.ExpandString($GitPromptSettings.DefaultPromptPath);

Object reference not set to an instance of an object.

  • CategoryInfo : OperationStopped: (:) [], NullReferenceException
  • FullyQualifiedErrorId : System.NullReferenceException

However DefaultPromptPath seems to be defined: write-host $global:GitPromptSettings.DefaultPromptPath $(Get-PromptPath)

To resolve the issue I was able to redefine the prompt function with an updated prompt script block in the profile, after importing the posh-git module, by updating this single line: $currentPath = $(Get-PromptPath)

Is there a better way to resolve this?

Thank you

dahlbyk commented 6 years ago

Are you able to get a full stack trace? Try setting $GitPromptSettings.Debug = $true.

dahlbyk commented 6 years ago

If you restore posh-git back to the original code, do you get the same error if you set $GitPromptSettings.DefaultPromptPath = '$pwd'?

annahosanna commented 6 years ago

Updating GitPrompt.ps1 to set GitPromptSettings.Debug to $true had no effect (prompt was still "PS>" )

annahosanna commented 6 years ago

Your second suggestion worked:

PS>cd .\myrepo PS>$GitPromptSettings.DefaultPromptPath = '$pwd' C:\Users\me\Documents\GitHub\myrepo [master ≡]>

dahlbyk commented 6 years ago

Is calling Get-PromptPath directly successful as well? I'm struggling to reconcile that '$pwd' can be expanded but '$(Get-PromptPath)' can't be.

annahosanna commented 6 years ago

PS>$(Get-PromptPath) C:\Users\me\Documents\GitHub\myrepo PS> PS>$GitPromptSettings.DefaultPromptPath = $(Get-PromptPath) C:\Users\me\Documents\GitHub\myrepo [master ≡]>

annahosanna commented 6 years ago

Seems like it is using the literal value "$(Get-PromptPath)" rather than the result from evaluating that function.

PS>write-host $Global:GitPromptSettings.DefaultPromptPath $(Get-PromptPath)

PS>$GitPromptSettings.DefaultPromptPath = $(Get-PromptPath) C:\Users\me\Documents\GitHub\myrepo [master ≡]> write-host $Global:GitPromptSettings.DefaultPromptPath C:\Users\me\Documents\GitHub\myrepo

annahosanna commented 6 years ago

I think it is from GitPrompt.ps1. (in this version). DefaultPromptPath should be using quotes not ticks.

DefaultPromptPath = '$(Get-PromptPath)' should be: DefaultPromptPath = "$(Get-PromptPath)"

rkeithhill commented 6 years ago

Indeed, you have to use single quotes when setting the value. I think we should update that to make it also handle a scriptblock. This would be less susceptible to error:

# NOT SUPPORTED YET
$GitPromptSettings.DefaultPromptPath = { Get-PromptPath }
dahlbyk commented 6 years ago

Handling a ScriptBlock would be neat.

I still don't know what the nullref is from, though.

annahosanna commented 6 years ago

I'm guessing it is because $ExecutionContext.SessionState is not defined/null when expanded from a string, but defined when it is called as part of a script/script block.

This does not work for me either:

$Global:GitPromptSettings.DefaultPromptPath='$($ExecutionContext.SessionState.Path.CurrentLocation.Path)'

Nor this:

C:\Users\me\Documents\GitHub\myrepo [master ≡]> $ExecutionContext.SessionState.InvokeCommand.ExpandString('$($ExecutionContext.SessionState.Path.CurrentLocation.Path)')
Exception calling "ExpandString" with "1" argument(s): "Object reference not set to aninstance of an object."
At line:1 char:1
+ $ExecutionContext.SessionState.InvokeCommand.ExpandString('$($ExecutionContext.S ...
+ ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
    + CategoryInfo          : NotSpecified: (:) [], MethodInvocationException
    + FullyQualifiedErrorId : NullReferenceException
annahosanna commented 6 years ago

But this works:

C:\Users\me\Documents\GitHub\myrepo [master ≡]> $ExecutionContext.SessionState.InvokeCommand.InvokeScript($ExecutionContext.SessionState.InvokeCommand.NewScriptBlock('$($ExecutionContext.SessionState.Path.CurrentLocation.Path)'))

This works too:

PS>$Global:GitPromptSettings.DefaultPromptPath='$(Get-PromptPath)'
PS>$ExecutionContext.SessionState.InvokeCommand.InvokeScript($ExecutionContext.SessionState.InvokeCommand.NewScriptBlock($Global:GitPromptSettings.DefaultPromptPath))
C:\Users\me\Documents\GitHub\myrepo

This works in the GitPromptScritpBlock replacing:

$currentPath = $ExecutionContext.SessionState.InvokeCommand.ExpandString($GitPromptSettings.DefaultPromptPath)

With:

$currentPath = $ExecutionContext.SessionState.InvokeCommand.InvokeScript($ExecutionContext.SessionState.InvokeCommand.NewScriptBlock($GitPromptSettings.DefaultPromptPath))
rkeithhill commented 6 years ago

This:

$Global:GitPromptSettings.DefaultPromptPath='$(Get-PromptPath)'

Is what we expect folks to use and is how it is set by default: https://github.com/dahlbyk/posh-git/blob/5dcdb9df9ffd9a10c65dd02c75352fee979ae019/src/GitPrompt.ps1#L112

The single quotes defers interpolation until we call ExpandString in the posh-git provided prompt function. And this is what we do in our prompt function: https://github.com/dahlbyk/posh-git/blob/5dcdb9df9ffd9a10c65dd02c75352fee979ae019/src/posh-git.psm1#L55

So, when you have the property set like this do you still the get NullRefEx?

annahosanna commented 6 years ago

It works with single quotes, and performs dynamic expansion. It creates a script block from the text in single quotes, and then evaluates the script block. However, because this is a script block plain strings need extra quotes now. Roughly '"test"' becomes { "test" } whereas "test" becomes { test } (which is an error because the variable named test is undefined). So these work:

$Global:GitPromptSettings.DefaultPromptPath='"test"' $Global:GitPromptSettings.DefaultPromptPath='$(Get-PromptPath)'

and this does not: $Global:GitPromptSettings.DefaultPromptPath="$(Get-PromptPath)"

rkeithhill commented 6 years ago

It creates a script block from the text in single quotes

That's not correct. By using single quotes, you're telling PowerShell not to do interpolation. When you use double quotes, PowerShell interpolates first and then assigns the result of the interpolation to the setting e.g.:

$Global:GitPromptSettings.DefaultPromptPath='$(Get-PromptPath)'
$Global:GitPromptSettings.DefaultPromptPath
'$(Get-PromptPath)'

$Global:GitPromptSettings.DefaultPromptPath="$(Get-PromptPath)" # Currently in C:\Users\hillr
$Global:GitPromptSettings.DefaultPromptPath
C:\User\hillr

The point of calling $ExecutionContext.SessionState.InvokeCommand.ExpandString() is that it will do the interpolation for us at the point in time we call it.

Try this:

$foo = '$(2+2)'
$foo
$(2+2)

$ExecutionContext.SessionState.InvokeCommand.ExpandString($foo)
4

But let me ask this again, when you have the property set like this (in single quotes) do you still the get NullRefEx?

dahlbyk commented 6 years ago

This works in the GitPromptScritpBlock replacing:

$currentPath = $ExecutionContext.SessionState.InvokeCommand.ExpandString($GitPromptSettings.DefaultPromptPath) With:

$currentPath = $ExecutionContext.SessionState.InvokeCommand.InvokeScript($ExecutionContext.SessionState.InvokeCommand.NewScriptBlock($GitPromptSettings.DefaultPromptPath))

So the hypothesis is that some part of $ExecutionContext.SessionState.Path.CurrentLocation is returning $null, but only when called via ExpandString()? You can use $ExecutionContext.SessionState.InvokeCommand.ExpandString('$($ExecutionContext.SessionState.Path.CurrentLocation)') to narrow down what part is problematic. Does $ExecutionContext.SessionState.InvokeCommand.ExpandString('$(Get-Location)') work?

https://github.com/dahlbyk/posh-git/blob/5dcdb9df9ffd9a10c65dd02c75352fee979ae019/src/Utils.ps1#L282-L287

rkeithhill commented 6 years ago

FYI, you need to put that expression (that accesses properties) inside PowerShell sub-expression syntax e.g.:

$ExecutionContext.SessionState.InvokeCommand.ExpandString('$($ExecutionContext.SessionState.Path.CurrentLocation)')
annahosanna commented 6 years ago

@rkeithhill Sorry I must have miscommunicated something. 1) I no longer get the nullref using $ExecutionContext.SessionState.InvokeCommand.InvokeScript instead of $ExecutionContext.SessionState.InvokeCommand.ExpandString. 2) I understood that you were using ' to control when interpolation was done.

annahosanna commented 6 years ago

@dahlbyk No

PS>$ExecutionContext.SessionState.InvokeCommand.ExpandString('$(Get-Location)')
Object reference not set to an instance of an object.
    + CategoryInfo          : OperationStopped: (:) [], NullReferenceException
    + FullyQualifiedErrorId : System.NullReferenceException
annahosanna commented 6 years ago

@dahlbyk

PS>$ExecutionContext.SessionState.InvokeCommand.ExpandString('$ExecutionContext.SessionState.Path.CurrentLocation')
System.Management.Automation.EngineIntrinsics.SessionState.Path.CurrentLocation
dahlbyk commented 6 years ago

@annahosanna sorry, try this:

$ExecutionContext.SessionState.InvokeCommand.ExpandString('$($ExecutionContext.SessionState.Path.CurrentLocation)')

Or even this?

$ExecutionContext.SessionState.InvokeCommand.ExpandString('$pwd')
annahosanna commented 6 years ago
PS>$ExecutionContext.SessionState.InvokeCommand.ExpandString('$($ExecutionContext.SessionState.Path.CurrentLocation)')
Exception calling "ExpandString" with "1" argument(s): "Object reference not set to an
instance of an object."
At line:1 char:1
+ $ExecutionContext.SessionState.InvokeCommand.ExpandString('$($ExecutionContext.S ...
+ ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
    + CategoryInfo          : NotSpecified: (:) [], MethodInvocationException
    + FullyQualifiedErrorId : NullReferenceException
dahlbyk commented 6 years ago
$ExecutionContext.SessionState.InvokeCommand.ExpandString('$($ExecutionContext.SessionState)')

or

$ExecutionContext.SessionState.InvokeCommand.ExpandString('$pwd')

annahosanna commented 6 years ago
PS>$ExecutionContext.SessionState.InvokeCommand.ExpandString('$pwd')
C:\Users\me\Documents\GitHub
annahosanna commented 6 years ago
PS>$ExecutionContext.SessionState.InvokeCommand.ExpandString('$($ExecutionContext.SessionState)')
Exception calling "ExpandString" with "1" argument(s): "Object reference not set to an
instance of an object."
At line:1 char:1
+ $ExecutionContext.SessionState.InvokeCommand.ExpandString('$($ExecutionContext.S ...
+ ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
    + CategoryInfo          : NotSpecified: (:) [], MethodInvocationException
    + FullyQualifiedErrorId : NullReferenceException
annahosanna commented 6 years ago

@rkeithhill , @dahlbyk

I'm happy to answer questions; however, this issue does not need to remain open since I can use the defaults with the solution I mentioned earlier.

$currentPath = $ExecutionContext.SessionState.InvokeCommand.InvokeScript($ExecutionContext.SessionState.InvokeCommand.NewScriptBlock($GitPromptSettings.DefaultPromptPath))
dahlbyk commented 6 years ago

@annahosanna we appreciate your assistance troubleshooting!

@rkeithhill can you think of a reason not to switch to the InvokeScript(NewScriptBlock(...)) workaround instead of ExpandString() for v0.x, since that seems not to work as expected in PS 4.0?

rkeithhill commented 6 years ago

No but I would like to spin up a PS v4 VM to see if I can replicate this (later tonight). BTW we can cache the scriptblock that gets created. It doesn't need to be recreated each time. Strike-that - in v0 we have no good way update the scriptblock if the user changes the setting. :-(

annahosanna commented 6 years ago

Googling around, the examples I could find seem like this may be specific to certain versions of Windows (I'm using Windows 7 enterprise, but I've also seen mention of this with 2012/2012 R2) with PS 4.

dahlbyk commented 6 years ago

The other workaround would be to add:

if (-not $pathInfo) {
  $pathInfo = $pwd
}
rkeithhill commented 6 years ago

I was able to easily repro this on a Windows 7 VM updated to PS 4. It seems the parser chokes on any sub-expression passed to ExpandString():

image

Below you can see the full stack trace of where it chokes in our prompt function. Looks like it chokes reading a Line property on some InternalScriptPosition object.

image

Unfortunately the suggestion solution doesn't always work:

image

But it does work with the default case and can be made to work in the scenario above with an extra set of double quotes e.g.:

$GitPromptSettings.DefaultPromptPath.Text = '"MyPath is $(Get-PromptPath)"'

However, this change could break folks on PS >= 5 who have customized their DefaultPromptPath setting e.g.:

07-28 12:39:55 11> $GitPromptSettings.DefaultPromptPath.Text = 'MyPath is $(Get-PromptPath)'
07-28 12:41:16 12> $ExecutionContext.SessionState.InvokeCommand.InvokeScript($ExecutionContext.SessionState.InvokeCommand.NewScriptBlock($GitPromptSettings.DefaultPromptPath.Text))
Exception calling "InvokeScript" with "1" argument(s): "The term 'MyPath' is not recognized as the name of a cmdlet,
function, script file, or operable program. Check the spelling of the name, or if a path was included, verify that the
path is correct and try again."
At line:1 char:1
+ $ExecutionContext.SessionState.InvokeCommand.InvokeScript($ExecutionC ...
+ ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
    + CategoryInfo          : NotSpecified: (:) [], MethodInvocationException
    + FullyQualifiedErrorId : CommandNotFoundException

So I'm thinking we should conditionally apply this fix to PS <= v4. Thoughts? I suppose we should also verify the "fix" works on PS v3.

dahlbyk commented 6 years ago

How about a string wrapper inside NewScriptBlock? Roughly (on phone, untested):

$ExecutionContext.SessionState.InvokeCommand.InvokeScript(
  $ExecutionContext.SessionState.InvokeCommand.NewScriptBlock(
  "`"`$($($GitPromptSettings.DefaultPromptPath))`""
  ))

(Note this bug is for v0, so we don't want .Text)

rkeithhill commented 6 years ago

(Note this bug is for v0, so we don't want .Text)

Got that. But on my dev system (running V5 / V6.1) it is easier to test $ExecutionContext with my currently loaded posh-git v1 beta2. Ultimately, the text passed into NewScriptBlock is the same.

The string wrapper, as-is, still fails:

image

dahlbyk commented 6 years ago

Oh, I was overthinking it. This is what I meant:

$ExecutionContext.SessionState.InvokeCommand.InvokeScript(
  $ExecutionContext.SessionState.InvokeCommand.NewScriptBlock(
    "`"$($GitPromptSettings.DefaultPromptPath)`""
  ))

Which should resolve to:

$ExecutionContext.SessionState.InvokeCommand.InvokeScript(
  $ExecutionContext.SessionState.InvokeCommand.NewScriptBlock(
    "MyPath is $(Get-PromptPath)"
  ))

FWIW, I've been testing this by launching with -NoProfile and adding:

$GitPromptSettings = @{ DefaultPromptPath = 'MyPath is $(Get-Location)' }