-
Notifications
You must be signed in to change notification settings - Fork 905
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.
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
(#396) Get-Installed helper #441
base: master
Are you sure you want to change the base?
Conversation
This is a good start. However, instead of rewriting files, I would see this as a setting that is for not using the wmi objects, then overriding the function itself and using the setting to determine which way to go. For an example of overriding the function, see this prior art https://github.com/chocolatey/chocolatey/blob/master/src/helpers/functions/Write-Host.ps1 |
But do we really need a setting? |
You can start out with no setting if you want. The setting is not the important part, it's not rewriting package files. |
|
||
function Get-WmiObject { | ||
|
||
$oc = Get-Command 'Get-WmiObject' -Module 'Microsoft.PowerShell.Management' |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Is this the name of where the module is?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
C:\15_09_30>powershell -command "get-command get-wmiobject"
CommandType Name ModuleName
----------- ---- ----------
Cmdlet Get-WmiObject Microsoft.PowerShell.Management
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
👍
Would you mind updating that second commit to conform to our contributing guidelines? Unless you are planning to squash that? |
Direct replacement for "Get-WmiObject -Class Win32_Product". Win32_Product does lots of unnecessary work and may be VERY slow. Some of packages use it to check if a package is already installed. This change adds Get-Installed helper and a hook that processes Win32_Product class requests.
Should I change anything else? |
Thanks. I think it's ready for a deeper review now. |
FYI Win32_Product enumeration triggers a some minor processing on every installed MSI package. You can see it if you enabled MSI verbose logging. I blogged this here: http://csi-windows.com/toolkit/win32product-replacement (the code in the article is not appropriate for this). |
$w32item | Add-Member -type NoteProperty -Name "IdentifyingNumber" -Value $app.PsChildname | ||
$w32item | Add-Member -type NoteProperty -Name "Name" -Value $propitems.DisplayName | ||
$w32item | Add-Member -type NoteProperty -Name "Vendor" -Value $propitems.Publisher | ||
$w32item | Add-Member -type NoteProperty -Name "Version" -Value $propitems.DisplayVersion |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This is likely going to need a bit of work. DisplayVersion is not required.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
InnoSetup doesn't use it.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This fix is mainly intended to be a replacement for Win32_Product which doesn't know about innosetup at all. Win32_Product works with Windows Installer (MSI).
Hooked Get-WmiObject function returns a list fully compatible with original function's output except an items order. Fields returned by Win32_Product:
- IdentifyingNumber
- Name
- Vendor
- Version
- Caption (allways the same as Name)
Get-Installed (called from Get_WmiObject) is able to return non-MSI applications and it does so when -GWMICompatible switch is not used. It had a switch with the opposite meaning in previous commits but this has been changed during a conversation (see outdated diffs).
Also InnoSetup MAY use DisplayVersion. Here is a part of output:
IdentifyingNumber : Git_is1
Name : Git version 2.5.0
Vendor : The Git Development Community
Version : 2.5.0
Caption : Git version 2.5.0
WindowsInstaller :
UninstallString : "C:\Program Files\Git\unins000.exe"
Why two functions?
- Get-Installed can return MSI and non-MSI records. It may be used in packages installation/uninstallation scripts.
- Get_WmiObject can return only MSI records. It transparently replaces Win32_Product calls with Get-Installed so there is no need to fix more than 60 existing (and may be some future) packages which use Win32_Product.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I took a deeper look - MSIs can use Version
and/or DisplayVersion
. Not all MSIs I queried on my system use DisplayVersion
.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The proper order for the query is probably going to be converting Version
to a string, DisplayVersion
if Version
is not available, then converting VersionMajor``VersionMinor
to strings if the other information is not available.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Starting to look at what is necessary so this change can come in for 0.9.10. What happens when those prop items are missing? I'm assuming it doesn't error?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@ferventcoder - I have done code for this before - ran and was debugged for 14000 machines in 17 MUI language on Win XP and Win 7.
In that code I tried to reconcile win32_product with this information - you basically have to link it by product code which can be parsed out of the ARP data. But for my purposes it looked like that road was going to be very bumpy. For one thing if there is a master MSI and child MSIs - more than one ARP item can carry the product same code. Sometimes the same was true when a setup.exe contains an MSI. So for my more limited purposes I just parsed Product Code from ARP data and didn't bother with using MSI APIs. I found the incidence of an MSI registered in Win32_Product not having an Uninstall key to be very low - I believe because the MSI API does the Uninstall key automatically with RegisterProduct standard action.
Here are some other challenges / issues I ran into and codeded around (with no connotations of what would be the appropriate approach for chocolatey):
- there is a third per-user location for uninstall. Kindle reader is one thing I've found in it before. Exist check it and pull it into the record list if it exists.
- raw uninstall keys contain some hidden items - created a switch to include hidden items rather than have them in the list all the time
- raw uninstall keys contain parent / child relationships not normally shown in Programs and Features - created a switch to include them rather than retrieve all the time.
- raw uninstall keys can contain windows updates KB article updates that don't normall show in Programs and Features - created a switch
- occasionally got blank records when retrieving from this key - used a filter to drop them after retrieving all three locations.
- it is possible, but infrequent, for this code to choke when poorly formed uninstall keys have been laid down by a vendor. There was some java dev kit that was laying down "size" in a bizzare format and it would choke. Found others had reported it to the vendor. We only had about 4 affected machines in 14,000 - so just corrected those. (this is probably extremely rare - but when the code chokes on it - it's hard for the individual to figure out why) - so in at least a debug message it should be possible to list what key one is processing when the blow up happens. Wow - actually found the post - I'm on this thread as djsnetbeans: [https://netbeans.org/bugzilla/show_bug.cgi?id=251538]
- The code provides three derived properties for GUIDfromUninstallString, KBGUIDFromKeyName, KBIDFromKeyName
For KBs the DisplayName will be null. For childitems ParentKeyName will be populated. Many times the parent item accomplishes an uninstall of child items - this is also what the "show updates" checkbox in Programs and Features" uses.
So I agree with your suspicion that this code would need some work to not generate any errors on the millions of nodes running Chocolatey. I agree that the code should be updated - but you probably want to get a prototype out there to flush out the above issues and any others and transition to the new function after some debug time has occurred in the real world.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
A prototype, and as opt-in. Exactly what I had stated before. Allow folks to opt into newer behavior that may not fully work as expected.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I took a deeper look - MSIs can use Version and/or DisplayVersion. Not all MSIs I queried on my system use DisplayVersion.
Could you give examples of such MSIs, how their records are stored in the registry and how they are returned from Win32_Product?
#396
Direct replacement for "Get-WmiObject -Class Win32_Product".
Win32_Product does lots of unnecessary work and may be VERY slow. Some
of packages use it to check if a package is already installed.
This change adds Get-Installed helper and a piece of code that
automatically fixes install/uninstall scripts which use Win32_Product.