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

[Bug]: Installer URLs mismatch #215

Closed
1 task done
sitiom opened this issue Jun 15, 2023 · 13 comments · Fixed by #245
Closed
1 task done

[Bug]: Installer URLs mismatch #215

sitiom opened this issue Jun 15, 2023 · 13 comments · Fixed by #245
Assignees
Labels
bug Something isn't working help wanted Extra attention is needed

Comments

@sitiom
Copy link

sitiom commented Jun 15, 2023

Is there an existing issue for this?

  • I have searched the existing issues

What happened?

Tried to run this command:

Komac update --id Microsoft.OpenSSH.Beta --version 9.2.2.0 --urls https://github.com/PowerShell/Win32-OpenSSH/releases/download/v9.2.2.0p1-Beta/OpenSSH-ARM64-v9.2.2.0.msi,https://github.com/PowerShell/Win32-OpenSSH/releases/download/v9.2.2.0p1-Beta/OpenSSH-Win32-v9.2.2.0.msi,https://github.com/PowerShell/Win32-OpenSSH/releases/download/v9.2.2.0p1-Beta/OpenSSH-Win64-v9.2.2.0.msi

As you can see, the installer URLs are mismatched:
image

@sitiom sitiom added bug Something isn't working help wanted Extra attention is needed labels Jun 15, 2023
@russellbanks
Copy link
Owner

russellbanks commented Jun 15, 2023

I'll add Win64 and Win32 as options in the Regex.

@sitiom
Copy link
Author

sitiom commented Jun 15, 2023

It works on 1.6.0, though. So what changed?
image

@russellbanks
Copy link
Owner

It works on 1.6.0, though. So what changed? image

I need to improve the matching algorithm but I've been having trouble doing so that catches all these edge cases. One major flaw with it is that depending on the order of items that are given to the functions, they can be matched differently. I thought to standardise it better before I have a more permanent solution, I would order them all before the function gets them. I imagine this is one of those cases.

@sitiom
Copy link
Author

sitiom commented Jun 15, 2023

Got mismatched here, too. It worked in the previous version: https://github.com/rhysd/actionlint/actions/runs/5280674972/jobs/9553198795

@rhysd
Copy link

rhysd commented Jun 15, 2023

FWIW, these are links to the jobs:

@sitiom
Copy link
Author

sitiom commented Jun 19, 2023

Yeah, this is affecting every package using Winget Releaser. A quick look at https://github.com/microsoft/winget-pkgs/pulls?q=is%3Apr+is%3Aopen+sort%3Aupdated-desc+Winget+Releaser and you will see a bunch of mismatched URLs in the open PRs.

@russellbanks
Copy link
Owner

russellbanks commented Jun 23, 2023

It works on 1.6.0, though. So what changed?

Fixing #200 also meant it was more strict in that delimiters are required before and after the architecture in a URL. This means I have to now explicitly have Win32 and Win64 as architectures to look for.

russellbanks added a commit that referenced this issue Jun 23, 2023
@russellbanks
Copy link
Owner

I'll create 1.8.0 with a reverted change to 1.6.0 and some minor improvements and fixes to the retrieval of architectures from URLs. In 1.9.0, I'll focus on covering edge cases and making a more solid algorithm for matching installers when we don't have the full information about it (e.g. when the URL does not have an architecture in it and the binary says it's x86 when it's x64 - we need to still be able to match it through deduction by dealing with installers we have more information on first).

I'll close this issue once I'm happy with the state of it.

@sitiom
Copy link
Author

sitiom commented Jul 7, 2023

Komac update --id schollz.croc --version 9.6.5 --urls https://github.com/schollz/croc/releases/download/v9.6.5/croc_9.6.5_Windows-64bit.zip,https://github.com/schollz/croc/releases/download/v9.6.5/croc_9.6.5_Windows-32bit.zip,https://github.com/schollz/croc/releases/download/v9.6.5/croc_9.6.5_Windows-ARM64.zip,https://github.com/schollz/croc/releases/download/v9.6.5/croc_9.6.5_Windows-ARM.zip

Mismatch with x86 here:

Installers:
- Architecture: x86
  InstallerUrl: https://github.com/schollz/croc/releases/download/v9.6.5/croc_9.6.5_Windows-64bit.zip
  InstallerSha256: 17923DFC923792C7D6EADF8A52EC43F1009E34645A8AAA01C757F8920DD58A77
- Architecture: x64
  InstallerUrl: https://github.com/schollz/croc/releases/download/v9.6.5/croc_9.6.5_Windows-64bit.zip
InstallerSha256: 17923DFC923792C7D6EADF8A52EC43F1009E34645A8AAA01C757F8920DD58A77
- Architecture: arm
  InstallerUrl: https://github.com/schollz/croc/releases/download/v9.6.5/croc_9.6.5_Windows-ARM.zip
  InstallerSha256: B8945BD419A77A8CD5206615BE0C2AA77ABCFCFC28D926CFAE3023CB1AC07640
- Architecture: arm64
  InstallerUrl: https://github.com/schollz/croc/releases/download/v9.6.5/croc_9.6.5_Windows-ARM64.zip
  InstallerSha256: 99793A83985660B2C7BF275A17C779E9A6FC8FAC60A832D1775F638C9F57EEF4

@russellbanks
Copy link
Owner

Thanks @sitiom. I've added 64bit and 32bit into the regex in ede53df.

@russellbanks
Copy link
Owner

Please test the nightly release if you're able to and let me know if you come across any mismatches. The newer matching should be much better than before, and I've added tests for notable packages that cause edge cases in matching.

@sitiom
Copy link
Author

sitiom commented Aug 16, 2023

Installer mismatch in microsoft/winget-pkgs#116856:

Komac update --id YACReader.YACReader --version 9.13.1 --urls https://github.com/YACReader/yacreader/releases/download/9.13.1/YACReader-v9.13.1.2307166-winx64-7z-qt6.exe,https://github.com/YACReader/yacreader/releases/download/9.13.1/YACReader-v9.13.1.2307166-winx86-7z.exe
Installers:
- Architecture: x86
  InstallerUrl: https://github.com/YACReader/yacreader/releases/download/9.13.1/YACReader-v9.13.1.2307166-winx86-7z.exe
  InstallerSha256: 2D951763C7DC9447FD64B02C565A2445FD38EAB63913B7615557A2A8312B2C8E
- Architecture: x64
  InstallerUrl: https://github.com/YACReader/yacreader/releases/download/9.13.1/YACReader-v9.13.1.2307166-winx86-7z.exe
  InstallerSha256: 2D951763C7DC9447FD64B02C565A2445FD38EAB63913B7615557A2A8312B2C8E

@russellbanks
Copy link
Owner

russellbanks commented Aug 16, 2023

Thanks for reporting @sitiom! I'll add winx64 winx86 as architectures to look for.

russellbanks added a commit that referenced this issue Aug 16, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working help wanted Extra attention is needed
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants