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

Root Level overwrites Installer Level Incorrectly #490

Closed
Trenly opened this issue Dec 6, 2023 · 0 comments · Fixed by #491
Closed

Root Level overwrites Installer Level Incorrectly #490

Trenly opened this issue Dec 6, 2023 · 0 comments · Fixed by #491

Comments

@Trenly
Copy link
Contributor

Trenly commented Dec 6, 2023

I found the flow confusing in case of Root level and Installer level ARP defined side-by-side. I was testing with a manifest where I had 3 installers and 2 had Installer level ARP and then I defined a root level ARP for testing.

The Root level ARP was considered for the installer with no ARP entry, but rest of the installers had old variables set to their own respective installer level ARP entries. After the update, the root level ARP was shifted to the installer that originally had no ARP entry.

I believe (and this is how wingetcreate does it as well) that Installer level fields (not just limited to ARP entries) should be overwritten by Root level fields at the beginning of the update. This means if Root and installer level ARP are defined side-by-side in the manifest, then this should be considered an inconsistency, and the root level ARP should take precedence and overwrite whatever is at the Installer level.

I would say we can leave this in this PR since the implementation isn't strictly related to ARP, but is applicable to all installer fields. Just wanted to call this out here though.


For reference, I was testing this with Coder.Coder version 2.4.0 with the following installer manifest:

Installer manifest
```yaml
# Created using wingetcreate 1.5.7.0
# yaml-language-server: $schema=https://aka.ms/winget-manifest.installer.1.5.0.schema.json

PackageIdentifier: Coder.Coder
PackageVersion: 2.4.0
AppsAndFeaturesEntries:
  - DisplayName: Test
Installers:
- Architecture: x64
  InstallerType: nullsoft
  Scope: machine
  ElevationRequirement: elevatesSelf
  InstallerUrl: https://github.com/coder/coder/releases/download/v2.4.0/coder_2.4.0_windows_amd64_installer.exe
  InstallerSha256: 5C26EC6F89BBE75E89BC4DB9E9DA3681F20BCEEC9DD2DB21176B5559687CCA76
  ProductCode: Coder
- Architecture: x64
  InstallerType: zip
  NestedInstallerType: portable
  NestedInstallerFiles:
    - RelativeFilePath: coder.exe
      PortableCommandAlias: coder
  AppsAndFeaturesEntries:
    - DisplayName: Coder (portable)
  InstallerUrl: https://github.com/coder/coder/releases/download/v2.4.0/coder_2.4.0_windows_amd64.zip
  InstallerSha256: 4E53AEDB33A5B8EEC78A203E5C2B41269BC44E61502F576DDAD7DB94B6076093
- Architecture: arm64
  InstallerType: zip
  NestedInstallerType: portable
  NestedInstallerFiles:
    - RelativeFilePath: coder.exe
      PortableCommandAlias: coder
  AppsAndFeaturesEntries:
    - DisplayName: Coder (portable)
  InstallerUrl: https://github.com/coder/coder/releases/download/v2.4.0/coder_2.4.0_windows_arm64.zip
  InstallerSha256: 8D711A10EF3266317B63A05A47C30619390D04C47765119E3A24A0976B7107F5
ManifestType: installer
ManifestVersion: 1.5.0

Originally posted by @mdanish-kh in microsoft/winget-pkgs#129471 (comment)

If the behavior of wingetcreate is truly as @mdanish-kh described, then it is likely an error. Items at the installer level take precedence over items at the root level in winget-cli.

The flow would ideally be (and how YamlCreate does it) :
Try to move root level items to installer level. If installer level does not exist, root can be applied successfully. If installer level exists, it overrides root level and root is not applied.

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

Successfully merging a pull request may close this issue.

1 participant