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

Add Installer Url Matching for Update Command and Improve Error Messages #108

Merged
merged 18 commits into from
Jul 16, 2021

Conversation

ryfu-msft
Copy link
Contributor

@ryfu-msft ryfu-msft commented Jul 13, 2021

This pull request addresses the following:

Fixes #105
Fixes #107

We have discovered that there are some installers that are incorrectly identified by their manifest. When looking at the installers, they may have an architecture or installer type that differs from what is specified in the manifest, leading to an update command failure due to mismatched installers.

Since many installer URLs follow the pattern of including their architecture in their URL, we try to mitigate this issue by looking for any architectures stated in the installer URL. Winget-Create will attempt to match new installer URLs with their existing installers based on whether an architecture could be identified from the installer URL. If no architecture is identified, Winget-Create will resort to its normal flow and compare installers based on the architecture & installer type parsed from the package.

Changes:

  • Fixes a bug where arm and arm64 were not being parsed from MSI installers
  • Adds logic to check for a regex string match to find the architecture in the installer URL for matching updated installers.
  • Improves error message for more clarity as to why an installer update might fail.

Testing:

  • Adds a unit test to check whether the update command can determine the installer arch based on the installer URL using regex matching.
Microsoft Reviewers: Open in CodeFlow

@ryfu-msft ryfu-msft requested a review from palenshus July 13, 2021 00:49
@ryfu-msft ryfu-msft requested a review from a team as a code owner July 13, 2021 00:49
@ghost ghost added the Issue-Bug label Jul 13, 2021
@palenshus
Copy link
Contributor

palenshus commented Jul 13, 2021

Check Firefox to see how we handle apps that differ only by locale #Closed

@ryfu-msft ryfu-msft requested a review from palenshus July 13, 2021 23:45
@palenshus
Copy link
Contributor

palenshus commented Jul 14, 2021

Check Firefox to see how we handle apps that differ only by locale

@ryfu-msft, how'd this go?


In reply to: 879612294


In reply to: 879612294

src/WingetCreateCLI/Commands/UpdateCommand.cs Outdated Show resolved Hide resolved
src/WingetCreateCore/Common/PackageParser.cs Show resolved Hide resolved
src/WingetCreateCore/Common/PackageParser.cs Outdated Show resolved Hide resolved
@ghost ghost removed the Needs-Author-Feedback label Jul 14, 2021
@ryfu-msft
Copy link
Contributor Author

ryfu-msft commented Jul 14, 2021

Check Firefox to see how we handle apps that differ only by locale

@ryfu-msft, how'd this go?

As expected, installer nodes that differed only by locale had a similar behavior to installer nodes that only differed by scope. We will need to investigate the heuristics to see if we can catch these cases.


In reply to: 880261055


In reply to: 880261055


In reply to: 880261055

@ryfu-msft ryfu-msft requested a review from palenshus July 14, 2021 23:14
@palenshus palenshus dismissed their stale review July 15, 2021 02:58

revoking review

@ryfu-msft ryfu-msft requested a review from palenshus July 15, 2021 23:17
@ryfu-msft
Copy link
Contributor Author

ryfu-msft commented Jul 15, 2021

/azp run


In reply to: 881071702


In reply to: 881071702


In reply to: 881071702

@azure-pipelines
Copy link

Azure Pipelines successfully started running 1 pipeline(s).

@azure-pipelines
Copy link

Command 'run

In' is not supported by Azure Pipelines.



Supported commands

  • help:
    • Get descriptions, examples and documentation about supported commands
    • Example: help "command_name"
  • list:
    • List all pipelines for this repository using a comment.
    • Example: "list"
  • run:
    • Run all pipelines or specific pipelines for this repository using a comment. Use this command by itself to trigger all related pipelines, or specify specific pipelines to run.
    • Example: "run" or "run pipeline_name, pipeline_name, pipeline_name"
  • where:
    • Report back the Azure DevOps orgs that are related to this repository and org
    • Example: "where"

See additional documentation.

1 similar comment
@azure-pipelines
Copy link

Command 'run

In' is not supported by Azure Pipelines.



Supported commands

  • help:
    • Get descriptions, examples and documentation about supported commands
    • Example: help "command_name"
  • list:
    • List all pipelines for this repository using a comment.
    • Example: "list"
  • run:
    • Run all pipelines or specific pipelines for this repository using a comment. Use this command by itself to trigger all related pipelines, or specify specific pipelines to run.
    • Example: "run" or "run pipeline_name, pipeline_name, pipeline_name"
  • where:
    • Report back the Azure DevOps orgs that are related to this repository and org
    • Example: "where"

See additional documentation.

@ryfu-msft ryfu-msft requested a review from palenshus July 16, 2021 02:07
@palenshus
Copy link
Contributor

palenshus commented Jul 16, 2021

    private bool VerifyUpdatedInstallerHash(Manifests oldManifest, InstallerManifest newManifest)

This can be static


In reply to: 881127753


In reply to: 881127753


In reply to: 881127753


Refers to: src/WingetCreateCLI/Commands/UpdateCommand.cs:334 in 1000920. [](commit_id = 1000920, deletion_comment = False)

@palenshus palenshus dismissed their stale review July 16, 2021 02:24

revoking review

Copy link
Contributor

@palenshus palenshus left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🕐

@ghost ghost removed the Needs-Author-Feedback label Jul 16, 2021
@ryfu-msft
Copy link
Contributor Author

    private bool VerifyUpdatedInstallerHash(Manifests oldManifest, InstallerManifest newManifest)

changed to static


In reply to: 881127753


Refers to: src/WingetCreateCLI/Commands/UpdateCommand.cs:334 in 1000920. [](commit_id = 1000920, deletion_comment = False)

@azure-pipelines
Copy link

Command 'run

In' is not supported by Azure Pipelines.



Supported commands

  • help:
    • Get descriptions, examples and documentation about supported commands
    • Example: help "command_name"
  • list:
    • List all pipelines for this repository using a comment.
    • Example: "list"
  • run:
    • Run all pipelines or specific pipelines for this repository using a comment. Use this command by itself to trigger all related pipelines, or specify specific pipelines to run.
    • Example: "run" or "run pipeline_name, pipeline_name, pipeline_name"
  • where:
    • Report back the Azure DevOps orgs that are related to this repository and org
    • Example: "where"

See additional documentation.

@ryfu-msft ryfu-msft enabled auto-merge (squash) July 16, 2021 16:46
@ryfu-msft ryfu-msft requested a review from palenshus July 16, 2021 16:46
Copy link
Contributor

@palenshus palenshus left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

:shipit:

@ryfu-msft ryfu-msft merged commit 636b37e into main Jul 16, 2021
@ryfu-msft ryfu-msft deleted the HandleInstallerMismatch branch July 16, 2021 20:18
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
2 participants