Update PS should try to load the assembly from file path first before…#5161
Update PS should try to load the assembly from file path first before…#5161daxian-dbw merged 4 commits intoPowerShell:masterfrom
Conversation
c76f23a to
4aa9b99
Compare
9822a2e to
2777567
Compare
… loading from assemblyName to avoid breaks for existed code
| } | ||
| } | ||
| else if (!String.IsNullOrEmpty(name)) | ||
|
|
There was a problem hiding this comment.
How about we check if the file exists before trying Assembly.LoadFrom? So maybe just change if (!String.IsNullOrEmpty(filename)) to if (File.Exists(filename))?
|
@daxian-dbw your comments are resolved |
| catch (BadImageFormatException badImage) | ||
| { | ||
| error = badImage; | ||
| return null; |
There was a problem hiding this comment.
The return null at this line and line 1269 are not needed.
| @@ -1258,14 +1261,20 @@ internal static Assembly LoadAssembly(string name, string filename, out Exceptio | |||
| catch (BadImageFormatException badImage) | |||
There was a problem hiding this comment.
The comments in the catch (FileLoadException fileLoadException) block right above this line seems not needed anymore, since we now try loading from path first.
| $results[1] | Should BeExactly "BinaryModuleCmdlet1 exported by the ModuleCmdlets module." | ||
| } | ||
|
|
||
| It "PS should try to load the assembly from assembly name if file path doesn't exist" -skip:(!$IsWindows) { |
There was a problem hiding this comment.
Why is this test skipped on Unix plats?
| $module = Import-Module $TESTDRIVE\test.psd1 -PassThru | ||
| $module.NestedModules | Should Not BeNullOrEmpty | ||
| $assemblyLocation = [psobject].Assembly.location | ||
| $module.NestedModules.ImplementingAssembly.Location | Should Be $assemblyLocation |
There was a problem hiding this comment.
Please unload the module after the test.
|
@daxian-dbw your comment is resolved |
| else | ||
| { | ||
| New-ModuleManifest -Path $TESTDRIVE/test.psd1 -NestedModules /NOExistedPath/System.Management.Automation.dll | ||
| } |
There was a problem hiding this comment.
Can you use Join-Path to avoid the if ($IsWindows) else blocks? For example,
$psdFile = Join-Path $TESTDRIVE test.psd1
$nestedModule = Join-Path NOExistedPath System.Management.Automation.dll
New-ModuleManifest -Path $psdFile -NestedModules $nestedModule
| } | ||
| finally | ||
| { | ||
| Remove-Module $TESTDRIVE\test.psd1 -ErrorAction SilentlyContinue |
There was a problem hiding this comment.
Remove-Module $module -ErrorAction SilentlyContinue would do
| } | ||
| try | ||
| { | ||
| $module = Import-Module $TESTDRIVE\test.psd1 -PassThru |
There was a problem hiding this comment.
Once you have $psdFil replace the $TESTDRIVE\test.psd1 with $psdFil.
|
@daxian-dbw your comment is resolved |
689ce0c to
87a9bd7
Compare
daxian-dbw
left a comment
There was a problem hiding this comment.
LGTM. Can you please add [Feature] tag to the commit message of your last commit so that we can validate the fix with a full test run?
|
@chunqingchen I have assigned #5084 to you so you can verify whether this PR fixes that issue. |
fix issue #3325
After we merge #4196, the fix will break some our partner's working code. Although the working code often works with an invalid assembly path.
This fix is to follow the old logic to minimize the breaking change.