choco icon indicating copy to clipboard operation
choco copied to clipboard

Upgrade all reuses overridden package parameters when useRememberedArgumentsForUpgrades feature is turned on

Open dinfinity opened this issue 7 years ago • 42 comments

What You Are Seeing?

I have a multitude of packages installed via chocolatey, a number of which I have set custom install directories for. Chocolatey regularly shows error messages that indicate that the overridden parameters for another package are used. For instance:

ERROR: Running ["C:\Users\Dual Infinity\AppData\Local\Temp\chocolatey\TotalCommander\9.10\INSTALL.exe" INSTALLDIR=C:\Frameworks\NodeJS] was not successful. Exit code was '1'. See log for possible error messages.
The upgrade of totalcommander was NOT successful.
Error while running 'C:\ProgramData\chocolatey\lib\TotalCommander\tools\chocolateyInstall.ps1'.
 See log for details.

It seems to use the last overridden parameters encountered.

Addendum 2017-11-07: The issue is usually resolved by running the upgrade all command multiple times. It does not seem random, but it does seem that if a package with custom parameters was installed successfully its parameters are not read and do not conflict in the subsequent run of upgrade all.

What is Expected?

Chocolatey should (obviously) not reuse package parameters for other packages.

How Did You Get This To Happen? (Steps to Reproduce)

In essence, this would entail setting custom install dirs for multiple packages and running chocolatey upgrade all.

Output Log

See zip: dinfinity.chocolatey.log.2017-11-06.zip

Related

  • https://github.com/chocolatey/choco/issues/797

dinfinity avatar Nov 06 '17 13:11 dinfinity

This sounds like a bug, and a major one.

Search: useRememberedArgumentsForUpgrades, useRememberedArguments, useRememberedArgs, useRemembered, use remembered args arguments

ferventcoder avatar Nov 06 '17 16:11 ferventcoder

This isn't going to be quite as easy to find the cause of as hoped. Thanks for the log file. The code in question doesn't reuse parameters (on first glance, there could be a small bug in there).

I think to really find this, we're going to need to understand how you installed everything and possibly take a much closer look (if you are up for it).

How did you install things? One at a time or did you run multiple choco.exe processes at the same time?

ferventcoder avatar Nov 06 '17 16:11 ferventcoder

Taking a closer look is no problem. I like chocolatey a lot.

I have been ignoring this issue for a couple of months now, so I can't say for sure I did not run multiple simultaneous choco commands, although I can't remember doing so and deem it quite unlikely.

I could provide a zip of (parts of) my ProgramData\chocolatey directory if that helps.

I'm adding the following to the issue description: "The issue is usually resolved by running the upgrade all command multiple times. It does not seem random, but it does seem that if a package with custom parameters was installed successfully its parameters are not read and do not conflict in the subsequent run of upgrade all."

dinfinity avatar Nov 07 '17 20:11 dinfinity

Without a repo, we'll need to bump this to vNext.

ferventcoder avatar Feb 12 '18 18:02 ferventcoder

As it turns out, this also happened to me: http://disq.us/p/1qp8o56

VLC got "upgraded" from 64 to 32bit on upgrade all after reusing remembered arguments that forced the 32bit upgrade for notepadplusplus earlier.

Vankog avatar Mar 07 '18 21:03 Vankog

I'm wondering if packages getting upgraded as a result of being a dependency of another package are getting hit by this...

ferventcoder avatar May 04 '18 19:05 ferventcoder

Adding a partial adjustment for 0.10.11, but we'll need to see if it alleviates the bigger issue - I don't think it will until we isolate for each package.

ferventcoder avatar May 04 '18 19:05 ferventcoder

In my case this seems very very similar to what is reported here: https://github.com/chocolatey/choco/issues/797

There are 2 packages (out of many) that I installed with -ia "INSTALLDIR=C:\aaa\bbb" (Vagrant and NodeJS) and some other packages (Total Commander, Mysql Workbench, Google Earth Pro) want to install into the NodeJS and Vagrant directories.

I see a fix has been added in 347f09f I am hoping that will solve this very, very annoying issue.

dinfinity avatar Dec 28 '18 18:12 dinfinity

The fix has been implemented, but it won't correct where it is already wrong. That involves running an upgrade for items with the arguments you want to be used. Given this was known to have issues and was in early preview, unfortunately I'm not sure what additional fixes we'll take on for that.

ferventcoder avatar Mar 04 '19 18:03 ferventcoder

can we set this as the default option (useRememberedArgumentsForUpgrades true)?

musm avatar Mar 04 '19 19:03 musm

Closing this as fixed for 0.10.12. We want to watch this closely. If you run into this, consider the bug occurs LONG before it is seen so you may have already had this with an older version of Chocolatey. We are looking specifically for folks who are starting with 0.10.12 and run into this later to know if we've fixed the issue or not.

ferventcoder avatar Mar 13 '19 21:03 ferventcoder

Please add some documentation about how this system works. Specifically how to reset the state.

Here's what I ran into: I set up choco feature enable -n useRememberedArgumentsForUpgrades then did a few botched upgrades (wrong parameters). I disabled the feature thinking I would start over from scratch, that no package parameters would be remembered.

Then after hunting down all the install args for no bloody desktop shortcut for each and every package (the Windows Group Policy conveniently named Prevents users from changing the desktop icons inconveniently fails to stop packages from installing a desktop icon AGAIN on every upgrade) I re-enabled useRememberedArgumentsForUpgrades and started upgrading with valid args.

Except when it came to a package with botched arguments. That first run was unexpectedly remembered! I thought flipping the setting off then on again would get me to a clean slate.

Why isn't this a text config again? Where are these remembered arguments stored?

akaleeroy avatar Apr 25 '19 18:04 akaleeroy

@akaleeroy leave the feature turned off while running the upgrades. Choco always records the args passed, the feature being turned on is just about whether it applies them at upgrade or not.

ferventcoder avatar Apr 26 '19 13:04 ferventcoder

@ferventcoder I am still struggling with this issue.

  1. I have deleted all old versions of package directories in C:\ProgramData\chocolatey\.chocolatey
  2. I have corrected all .arguments files in C:\ProgramData\chocolatey\.chocolatey by decrypting them, removing the spurious install dir and package parameters from them and reencrypting them.
  3. On a run of choco upgrade all, Calibre, the package after Apache2 in that run immediately took on the parameters of Apache2: --package-parameters="'/installLocation:C:\Frameworks\Apache2'" --allow-downgrade --cache-location="'C:\Temp\Windows\Dual Infinity\chocolatey'" That is, the new version of Calibre in C:\ProgramData\chocolatey\.chocolatey contained those arguments in .arguments.

My chocolatey version is 0.10.15 and it is still absolutely unclear to me how to prevent these remembered arguments from being transferred to other packages whilst retaining the remembered arguments for the packages for which I do want customized directories.

I think this issue has not been fixed at all, or the remembered arguments are stored somewhere else than where I fixed them.

(reposted with personal account)

dinfinity avatar May 01 '20 20:05 dinfinity

@dinfinity we are going to be looking much closer at this in the coming weeks. We want to get this ironed out and working.

ferventcoder avatar Jun 15 '20 20:06 ferventcoder

However, if you can, this should be a new issue that links back to this issue.

ferventcoder avatar Jun 15 '20 20:06 ferventcoder

Actually, I'm going to reopen this issue.

ferventcoder avatar Jun 15 '20 20:06 ferventcoder

I am also seeing this issue most of the time running "choco upgrade all". I tried uninstalling and reinstalling the packages that have wrong arguments, force upgraded them, deleted the .arguments file for them - this resolves the problems for a single run of "upgrade all" (I think) but on the next run, the problems reoccur.

ruedigerk avatar Jun 19 '20 18:06 ruedigerk

A reproducible test was documented in this issue: https://github.com/chocolatey/choco/issues/2111 and a fix has been put in place to address that issue.

We believe that this will also address the problems in this issue. As such, I am going to go out this issue, and we can come back to it if there are still issues being reported after we ship 0.10.16 of Chocolatey.

gep13 avatar Apr 21 '21 08:04 gep13

I'm still hitting this issue in 0.11.1

I attempted choco upgrade all and every package after git.install used --package-parameters="'/GitAndUnixToolsOnPath /WindowsTerminal /NoAutoCrlf /NoShellIntegration'"

tylerszabo avatar Sep 03 '21 15:09 tylerszabo

@TheCakeIsNaOH any chance you can take a look at this, based on the test that you put forward in the linked issue?

gep13 avatar Sep 03 '21 15:09 gep13

@gep13 I'm working on it, the test environment is being fussy about upgrading to 0.11.1

@tylerszabo Is there any chance that these parameters could be coming from saved arguments that got "contaminated" when packages were upgraded when choco 0.10.15 was installed? I may be able to come up with a better test in a bit.

TheCakeIsNaOH avatar Sep 03 '21 16:09 TheCakeIsNaOH

@TheCakeIsNaOH the versions prior were set properly in the .arguments files; checked with the following script:

Get-ChildItem -Filter ".arguments" -Path "C:\ProgramData\chocolatey\.chocolatey" -Recurse | ForEach-Object { Write-Output ("{0}: {1}" -f $_.Directory.BaseName, [System.Text.Encoding]::UTF8.GetString([System.Security.Cryptography.ProtectedData]::Unprotect([System.Convert]::FromBase64String(($_ | Get-Content -Encoding UTF8)), [System.Text.Encoding]::UTF8.GetBytes("Chocolatey"), [System.Security.Cryptography.DataProtectionScope]::LocalMachine))) }

tylerszabo avatar Sep 03 '21 16:09 tylerszabo

Ah, interesting. And not really in a good way. I thought I had this one figured out.

The good news is that the issue with packages switching to prerelease (#2111) seems to have been fixed, in my testing. The reproduction I had in that other issue is no longer switching python3 to prerelease.

TheCakeIsNaOH avatar Sep 03 '21 16:09 TheCakeIsNaOH

@gep13 @tylerszabo

I've got a reproduction in the test environment:

Steps:

  1. Setup/start the test environment, and do a fresh vagrant sandbox rollback.
  2. Make sure it is running choco 0.11.1
  3. Download wget 1.21.1, wget 1.21.1.20210323, git.install 2.33.0 and git.install 2.33.0.2 into the packages directory for the test environment.
  4. Add these lines to the vagrantfile in the standard "THIS IS WHAT YOU UNCOMMENT" place:
choco.exe feature enable -n userememberedargumentsforupgrades
choco.exe install -fdvy git.install --version=2.33.0 --package-parameters="'/GitAndUnixToolsOnPath /WindowsTerminal /NoAutoCrlf /NoShellIntegration'" --source "'c:\\packages'"
choco.exe install -fdvy wget --version=1.21.1 --source "'c:\\packages'"
choco.exe upgrade -dvy all --source "'c:\\packages'"
  1. Run vagrant provision
  2. See that wget 1.21.1.20210323 has the git.install package parameters included as well.
Running 'ChocolateyScriptRunner' for Wget v1.21.1.20210323 with packageScript 'C:\ProgramData\chocolatey\lib\Wget\tools\chocolateyinstall.ps1', packageFolder:'C:\ProgramData\chocolatey\lib\Wget', installArguments: '', packageParameters: '/GitAndUnixToolsOnPath /WindowsTerminal /NoAutoCrlf /NoShellIntegration',

TheCakeIsNaOH avatar Sep 03 '21 17:09 TheCakeIsNaOH

I've done a bit more debugging, and this reuse is not just applicable to package parameters, but also to other arguments. Specifically, I've tested --forcex86 and --install-arguments, and both of those get reused as well.

Everything is fine inside chocolatey.infrastructure.app.services.NugetService.upgrade_run(), but once the nuget package has been installed, and it goes onto chocolatey.infrastructure.app.services.ChocolateyPackageService.handle_package_result() to run the powershell script (and other stuff), the arguments from the previous package pop back into the config.

TheCakeIsNaOH avatar Oct 06 '21 21:10 TheCakeIsNaOH

If anyone running into this issue a lot, here is one option:

Function cup-all {
    if (!((New-Object Security.Principal.WindowsPrincipal([Security.Principal.WindowsIdentity]::GetCurrent())).IsInRole([Security.Principal.WindowsBuiltInRole]::Administrator))) {
        & gsudo.exe powershell.exe cup-all
        return
    }
    Write-Information "Checking for outdated packages" -InformationAction continue
    #choco outdated is slow.
    #$outdatedPackages = & choco.exe outdated -r --ignore-pinned
    $outdatedPackages = & roco.exe outdated -r
    if ($outdatedPackages) {
        $outdatedPackages | % {
            if (($_ -split "\|" | select -Last 1) -ne "true") {
                [array]$outdatedIds += $_ -split "\|" | select -First 1
            }
        }
        Write-Information "Finished check; updating these packages:" -InformationAction continue
        $outdatedIds.ForEach({ Write-Information $_ -InformationAction continue })
        $outdatedIds | % {
            & choco.exe upgrade $_
        }
    } else {
        Write-Information "Finished check; no packages outdated"  -InformationAction continue
    }
}

Stick it in your powershell profile. Auto elevates if needed, checks for outdated, then upgrade each outdated package individually, thus avoiding the reuse of arguments when multiple packages are upgraded with one command.

It does require gsudo and rocolatey installed. I'm using rocolatey since choco outdated takes over three minutes on my current main machine while roco is like 15 seconds.

TheCakeIsNaOH avatar Oct 13 '21 17:10 TheCakeIsNaOH

I think I may have found a fix for this, PR opened: #2484

I still have to do more through testing.

TheCakeIsNaOH avatar Dec 05 '21 15:12 TheCakeIsNaOH

For the purposes of tracking may this issue be re-opened? It's not the end of the world to avoid cup all . I'm currently working around by calling one of the following:

choco outdated --ignore-pinned --limitoutput | % { $_ -replace "(^[^|]*).*", '$1' } | % { & cup $_; if($LASTEXITCODE -ne 0) { throw "cup failed $LASTEXITCODE"} }
for /f "delims=|" %i in ('choco outdated --ignore-pinned --limitoutput') do ( cup %i )

This CMD script isn't particularly error safe; if put into a script it'd need to be modified to do call/goto in order to safely handle errors because CMD parameter expansion is wonky.

tylerszabo avatar Jan 18 '22 22:01 tylerszabo

@tylerszabo said... For the purposes of tracking may this issue be re-opened?

Yes, I think that this would make sense. The work that was already done on this fixed part of the problem, but there are obviously still other places where it needs to be addressed.

gep13 avatar Jan 19 '22 13:01 gep13