Support relative paths that don't begin with '.' #67
Conversation
| @@ -2907,7 +2909,6 @@ function New-SubmissionPackage | |||
| })] | |||
| [string[]] $AppxPath, | |||
|
|
|||
| [ValidateScript({ if (Test-Path -PathType Container $_) { $true } else { throw "$_ cannot be found." } })] | |||
There was a problem hiding this comment.
Removing the existence check for OutPath here and below. Non-existent paths are still valid and can be created during the output step.
|
You should clarify in your commit message that the issue really only can be seen when using a .NET method (like |
StoreBroker/Helpers.ps1
Outdated
| $Path = $resolvedPath | ||
| } | ||
|
|
||
| if (-not (Test-Path -PathType Container -Path $Path )) |
There was a problem hiding this comment.
nit: extra space before the closing parens
StoreBroker/Helpers.ps1
Outdated
| try | ||
| { | ||
| $resolvedPath = Resolve-UnverifiedPath -Path $Path | ||
| if ($null -ne $resolvedPath) |
There was a problem hiding this comment.
What's the scenario when this will be $null? (besides of course the input value being $null)
There was a problem hiding this comment.
This came about from testing with -ErrorAction Ignore, as explained above. I can remove the check.
There was a problem hiding this comment.
We really shouldn't be in a situation where Resolve-UnverifiedPath is returning $null. If it did, I'm not sure that just silently falling back to using the original $Path is the best idea....it just means that the caller is not guaranteed that they'll be getting their file written to the expected location...especially since the .zip file is going to run into trouble since it also depends on a valid value coming back from Resolve-UnverifiedPath
| @@ -539,6 +539,62 @@ function Show-ImageFileNames | |||
| } | |||
| } | |||
|
|
|||
| function Resolve-UnverifiedPath | |||
There was a problem hiding this comment.
I know that we wanted these scripts to work independent of the module, but now I'm not 100% sure. Having to duplicate this logic in here now across three different files seems like a maintenance nightmare. What's the drawback of dot-sourcing Helpers.ps1 here?
There was a problem hiding this comment.
In fact, I forgot we are already dot-sourcing Helpers.ps1 at the top of the file. I'll remove the redefinition of the function.
StoreBroker/Helpers.ps1
Outdated
| @@ -829,7 +829,7 @@ function Resolve-UnverifiedPath | |||
|
|
|||
| $resolvedPath = Resolve-Path -Path $Path -ErrorVariable resolvePathError -ErrorAction SilentlyContinue | |||
|
|
|||
| if ($null -eq $resolvedPath) | |||
| if (($null -eq $resolvedPath) -and ($resolvePathError.Count -gt 0)) | |||
There was a problem hiding this comment.
Can you explain the scenario for this change?
There was a problem hiding this comment.
This came about after testing with -ErrorAction Ignore. In that case, $resolvePathError was still $null and $resolvedPath was also $null. An exception was thrown trying to index into $resolvePathError. After changing back to -ErrorAction SilentlyContinue, I left the additional guard in just to be safe.
I'm fine removing if you don't think we need it.
There was a problem hiding this comment.
It's not going to help anything in this case...you'll just force it to go in to the else, and then it'll try accessing a property off of $null and get back an empty string, which will end up causing issues later on when the caller tries to use that "resolved" path. Better to just let it crash here if it really doesn't have any errors that we can index into.
StoreBroker/Helpers.ps1
Outdated
| @@ -829,7 +829,7 @@ function Resolve-UnverifiedPath | |||
|
|
|||
| $resolvedPath = Resolve-Path -Path $Path -ErrorVariable resolvePathError -ErrorAction SilentlyContinue | |||
|
|
|||
| if ($null -eq $resolvedPath) | |||
| if (($null -eq $resolvedPath) -and ($resolvePathError.Count -gt 0)) | |||
There was a problem hiding this comment.
It's not going to help anything in this case...you'll just force it to go in to the else, and then it'll try accessing a property off of $null and get back an empty string, which will end up causing issues later on when the caller tries to use that "resolved" path. Better to just let it crash here if it really doesn't have any errors that we can index into.
StoreBroker/Helpers.ps1
Outdated
| try | ||
| { | ||
| $resolvedPath = Resolve-UnverifiedPath -Path $Path | ||
| if ($null -ne $resolvedPath) |
There was a problem hiding this comment.
We really shouldn't be in a situation where Resolve-UnverifiedPath is returning $null. If it did, I'm not sure that just silently falling back to using the original $Path is the best idea....it just means that the caller is not guaranteed that they'll be getting their file written to the expected location...especially since the .zip file is going to run into trouble since it also depends on a valid value coming back from Resolve-UnverifiedPath
…t working directory. For example, an -OutPath of "SomeFolder" should represent a sub-folder of the current working directory. While PowerShell can handle path fragments of this type, interacting with .NET methods, such as XmlDocument.Save() can cause the file to be saved to an unintended directory. The fix is to use Resolve-UnverifiedPath in-order to resolve path fragments like this to a full path. Additonally, adds a new function Ensure-Directory, to ensure a directory is created/exists. Bump module version to 1.11.0
Support relative paths that don't begin with '.' to denote the current working directory.
For example, an -OutPath of "SomeFolder" should represent a sub-folder of the current working directory. While PowerShell can handle path fragments of this type, interacting with .NET methods, such as XmlDocument.Save() can cause the file to be saved to an unintended directory.
The fix is to use Resolve-UnverifiedPath in-order to resolve path fragments like this to a full path.
Additonally, adds a new function Ensure-Directory, to ensure a directory is created/exists.
#64