8000 Support importing module paths that end in trailing directory separator by SteveL-MSFT · Pull Request #6602 · PowerShell/PowerShell · GitHub
[go: up one dir, main page]
Skip to content

Support importing module paths that end in trailing directory separator #6602

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 3 commits into from
Apr 13, 2018
Merged

Support importing module paths that end in trailing directory separator #6602

Show file tree
Hide file tree
Changes from 1 commit
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
6d906b2
support importing module paths that end in trailing directory separator
SteveL-MSFT Apr 9, 2018
db797f4
address PR feedback
SteveL-MSFT Apr 9, 2018
26c72e2
address Andrew's feedback
SteveL-MSFT Apr 12, 2018
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Next Next commit
support importing module paths that end in trailing directory separator
  • Loading branch information
@SteveL-MSFT
SteveL-MSFT committed Apr 9, 2018
commit 6d906b2e224d2519ec33f577cf2ef4949ee764f0
5 changes: 5 additions & 0 deletions src/System.Management.Automation/engine/Modules/ImportModuleCommand.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -638,6 +638,11 @@ private PSModuleInfo ImportModule_LocallyViaName(ImportModuleOptions importModul

if (!found)
{
// If the path ends with a directory separator, remove it
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

By looking at the code in the file, I get a feeling that it is better to place this new change (that removes trailing separator) before the call to LoadUsingMultiVersionModuleBase that is earlier in the file:
foundModule = LoadUsingMultiVersionModuleBase(rootedPath,
This way version selection in the multi-version case will still work even with trailing separator in module path.

Copy link
Member Author
@SteveL-MSFT SteveL-MSFT Apr 12, 2018 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Good catch. I'll move it up and add a test.

if (rootedPath.EndsWith(Path.DirectorySeparatorChar))
{
rootedPath = Path.GetDirectoryName(rootedPath);
}
// If the path is a directory, double up the end of the string
// then try to load that using extensions...
rootedPath = Path.Combine(rootedPath, Path.GetFileName(rootedPath));
Expand Down
6 changes: 6 additions & 0 deletions test/powershell/Modules/Microsoft.PowerShell.Core/Import-Module.Tests.ps1
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -30,6 +30,12 @@ Describe "Import-Module" -Tags "CI" {
(Get-Module -Name $moduleName).Name | Should -BeExactly $moduleName
}

It "should be able to load a module with a trailing directory separator" {
$modulePath = (Get-Module -ListAvailable $moduleName).ModuleBase + [System.IO.Path]::DirectorySeparatorChar
Import-Module -Name $modulePath
(Get-Module -Name $moduleName).Name | Should -BeExactly $moduleName
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The module already was loaded in previous test.
Seems we could check:

{ Import-Module -Name $modulePath -ErrorAction Stop } | Should -Not -Throw
``

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

And maybe add -Force to really load the module and test whole code.
819C

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Perhaps a Remove-Module would make more sense here

}

It "should be able to add a module with using ModuleInfo switch" {
$a = Get-Module -ListAvailable $moduleName
{ Import-Module -ModuleInfo $a } | Should -Not -Throw
Expand Down
0