Skip to content

Fix for non-default install directory#3

Open
NamedJason wants to merge 2 commits intoPowerCLIGoodies:masterfrom
NamedJason:patch-1
Open

Fix for non-default install directory#3
NamedJason wants to merge 2 commits intoPowerCLIGoodies:masterfrom
NamedJason:patch-1

Conversation

@NamedJason
Copy link

This will allow the module to work if PowerCLI is installed in a non-default directory (or if the workstation is 32 bit Windows).

This will allow the module to work if PowerCLI is installed in a non-default directory (or if the workstation is 32 bit Windows).
@lucdekens
Copy link
Member

Nice fix.
Just one remark, the code assumes there is a module, so it won't work with PowerCLI 5.x.
Would we need to have PowerCLI 5.x support?
Thoughts?
Luc

@mtboren
Copy link
Member

mtboren commented Jul 29, 2016

Yes, good catch -- thanks for creating a fix, @NamedJason.

My thoughts on this not working for PowerCLI 5.x: PowerCLI 6.x has been out since Mar 2015, so it seems that there has been ample time for people to get current with their PowerCLI install. And, if there are still some people that, for some reason, "must" stay on a PowerCLI 5.x version, they can stay on v1.0.1 of the DRSRule module...

As for the bit in the updated code that "creates" the path to the given .dll from the module path, I suggest that we leverage some path-management cmdlets like Split-Path and Join-Path, staying out of the substring() game if we can. Like, instead of:
$pcliDll = "$($modulePath.substring(0,$modulePath.indexOf('Modules')))VMware.Vim.dll"

use something like:
$pcliDll = Join-Path -Path ((Get-Module VMware.VimAutomation.Core).ModuleBase | Split-Path -Parent | Split-Path -Parent) -ChildPath "VMware.Vim.dll"

A bit longer, but keeps us away from manual string manipulation.

Matt

@NamedJason
Copy link
Author

How about this?

$pcliDLL = join-path -path (get-installpath) -childpath "VMware.Vim.dll"

I'm skeptical of it because it seems too simple...

@lucdekens
Copy link
Member

Ok, with the PowerCLI 6.x requirement, but then we should add something like this.

#Requires -Modules VMware.VimAutomation.Core, @{ModuleName="VMware.VimAutomation.Core";ModuleVersion="6.0.0.0"}

This one has the #requires statement that Luc recommended and the simplest path generation technique that we came up with.
@NamedJason
Copy link
Author

I'm not super familiar with GitHub, but I think that I changed this pull request to use that #Requires statement and the join-path (get-installpath) technique to populate pcliDll.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants