Skip to content
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

Re-Update to partially fix 31 #36

Closed
wants to merge 4 commits into from
Closed

Re-Update to partially fix 31 #36

wants to merge 4 commits into from

Conversation

aledeniz
Copy link
Contributor

@aledeniz aledeniz commented Jun 7, 2023

This fix #31 (probably also #32).
The fix is partial because it only applies to the x64 version. I tested in 4 devices, 2 updates and 2 install.
It is likely something similar could fix also the x86 version, if affected, but I don't have how to test that.

aledeniz added 2 commits June 7, 2023 21:01
This fix #31 (probably also #32).
The fix is partial because it only applies to the x64 version. 
I tested in 4 devices, 2 updates and 2 install.
It is likely something similar could fix also the x86 version, if affected, but I don't have how to test that.
I copied this from an internalised package, this is to remove the non community related bits
@conitrade-as
Copy link

Isn't the problem that the original display name is not correct? Thus the uninstall key is not found which leads to the update portion not being executed. As mentioned in #32.

@aledeniz
Copy link
Contributor Author

aledeniz commented Jun 7, 2023

@conitrade-as I did look at this original display name theory, and I couldn't confirm it. I was able to replicate the issue without Chocolatey involved, just using the x64 deliverables by Adobe.

At some point the package call an Adobe exe with those arguments (the double backtick is a formatting issue, in the code is single):
"/sAll /msi /norestart /quiet PATCH="$mspPath" ALLUSERS=1 EULA_ACCEPT=YES $options" + " /L*v ``"$env:TEMP\$env:chocolateyPackageName.$env:chocolateyPackageVersion.Install.log``""

In some scenario that $options is expanded to include DISABLE_CACHE=1.
If you run that Adobe installer manually, pointing to the target msp file, you will notice it will fail, and if you look at the generated log, you will find that the command line has been modified, Adobe is adding a PATCH parameter pointing to a package on a local cache (even in an install from scratch!) and that parameter wins over the one passed on the actual command line.

MSI (s) (F8:98) [00:13:05:264]: PROPERTY CHANGE: Adding MsiLogFileLocation property. Its value is 'C:\********\Temp\chocolatey\adobereader.2023.3.20201.1.Install.log'. MSI (s) (F8:98) [00:13:05:264]: Command Line: TRANSFORMS= REBOOT=ReallySuppress PATCH=C:\********\Temp\chocolatey\AcroRdrDCx64Upd2300320201_MUI.msp ALLUSERS=1 EULA_ACCEPT=YES DISABLEDESKTOPSHORTCUT=1 UPDATE_MODE=3 DISABLE_CACHE=1 REBOOT=ReallySuppress PATCH=C:\Program Files\Common Files\Adobe\Acrobat\Setup\{AC76BA86-1033-FF00-7760-BC15014EA700}\AcroRdrDCx64Upd2300120093_MUI.msp CURRENTDIRECTORY=C:\Program Files\Common Files\Adobe\Acrobat\Setup\{AC76BA86-1033-FF00-7760-BC15014EA700} CLIENTUILEVEL=3 CLIENTPROCESSID=23808

Please note the 2 PATCH parameters reported in the log (and the 2 REBOOT as well, in this case).

@conitrade-as
Copy link

My point is the following: There is already a code path which does run msiexec.exe on the .msp. But only, if the variable $UpdateOnly is set (

). Which is the case only if:

  1. An existing installation is found via the display name (see https://github.com/open-circle-ltd/chocolatey.adobe-acrobat-reader-dc/blob/684f7929576ffb204183baadff168932bcb9d5c9/package/tools/chocolateyinstall.ps1#LL23C13-L23C13)
  2. There was a MUI Installation (see
    if ($key[0].DisplayName -notmatch 'MUI') {
    )
  3. A new version is indeed available, in which case $UpdateOnly becomes true (see )

But since the DisplayName changed that flow does not work anymore. But the installed version is still MUI capable. Let me try to adapt the code and report back.

@conitrade-as
Copy link

So indeed, the following change works for the update path: conitrade@4980a9a

@aledeniz
Copy link
Contributor Author

aledeniz commented Jun 7, 2023

I have tested the code in this PR to update Adobe Reader in 70 devices. 64 successful updates, the 6 not successful all had logical explanations (not enough disk space, reboot needed, ..).
Tomorrow I will try a few hundreds more.

@aledeniz
Copy link
Contributor Author

aledeniz commented Jun 7, 2023

I went to look at the value of DisplayName in my install base. In 80% of the deployments I found Adobe Acrobat (64-bit), but in 20% of the deployments I found Adobe Acrobat Reader.

This suggests that the condition:

if (($installedProducts | Where-Object {($_.DisplayName -contains 'Adobe Acrobat (64-bit)') -and ($_.DisplayVersion.replace('.', '') -eq $UpdaterVersion)} | measure).Count -lt 1)

may need to be changed as:

if (($installedProducts | Where-Object {(($_.DisplayName -contains 'Adobe Acrobat (64-bit)') -or ($_.DisplayName -contains 'Adobe Acrobat Reader')) -and ($_.DisplayVersion.replace('.', '') -eq $UpdaterVersion)} | measure).Count -lt 1)

@conitrade-as is your change going to support both names?
Also, I wonder if those are really the only 2 display names out there.

This is to update also on computers with DisplayName set to Adobe Acrobat Reader. This was successfully tested in 200 computers.
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.

Incorrect version
2 participants