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

Fix / update packages that use GitHub for version checking #2007

Closed
36 tasks done
pauby opened this issue Oct 12, 2022 · 19 comments
Closed
36 tasks done

Fix / update packages that use GitHub for version checking #2007

pauby opened this issue Oct 12, 2022 · 19 comments

Comments

@pauby
Copy link
Member

pauby commented Oct 12, 2022

Current Behaviour

GitHub has changed how it accepts web scraping that many packages are using for determining available versions (and other things). As a consequence, updating of those packages are now breaking and becoming outdated.

Expected Behaviour

To fix this we need to now use the GitHub API, preferably in a PowerShell way with a module such as PowerShellForGitHub. And to achieve that, many packages need to be refactored. I've already done some of this work on these packages in my repository (I'm linking these as suggested ways I've done things that I hope helps, not the way we or it should be done):

NOTE: There is a bug in PowerShellForGitHub that only returns the first 30 release assets. It's been 'fixed' but not sure when it will be merged and released. This may affect the timing of the work for this, which is why I mention it.

Affected Packages

I would propose we use this issue to collect all the affected packages here, in a checklist, so we can work our way through them. If you don't have permission to edit this post, please add a comment and somebody can make sure it's added here:

  • aptana-studio
  • bulk-crap-uninstaller
  • composer
  • freecad
  • ghostscript
  • ghostscript.app
  • git ((git) outdated #1995)
  • git.install ((git) outdated #1995
  • git.portable ((git) outdated #1995)
  • git-lfx
  • gif-lfs.install
  • intunewinapputil
  • k9s
  • kubelogin ? (this uses the same URL, but the method is different enough that I suspect it should be okay)
  • kubernetes-kompose
  • minikube ((minikube) is outdated #1994)
  • minishift
  • mpc-hc-clsid2
  • msys2
  • notepadplusplus
  • notepadplusplus.install
  • notepadplusplus.commandline
  • openshift-cli
  • paint.net
  • poi
  • protoc
  • qtox
  • selenium-gecko-driver
  • simplewall
  • simplewall.install
  • simplewall.portable
  • transifex-client
  • tribler
  • waterfox
  • waterfox-classic
  • yo
@pauby pauby added the Bug label Oct 12, 2022
@pauby pauby mentioned this issue Oct 12, 2022
4 tasks
@JPRuskin
Copy link
Contributor

JPRuskin commented Oct 12, 2022

I've (lazily?) used IRM to scrape latest releases - may be simpler than using a module (though equally may become outdated in a dozen places rather than one if GitHub changes methods again).

Perhaps as a compromise, as we'd likely lock the version of any module we used for security purposes and lose the magic of it automatically fixing itself, we could have a Get-LatestGitHubRelease function in the repository we could use?

# From https://github.com/JPRuskin/ChocolateyPackages/blob/main/packages/cloudflared/Update.ps1
$LatestRelease = Invoke-RestMethod "https://api.github.com/repos/cloudflare/cloudflared/releases/latest"
$LatestVersion = $LatestRelease.tag_name.TrimStart('v')
# Also available: $LatestRelease.assets, body, etc.

I'm actually surprised to see that that method against syncthing returns 37 assets - I was expecting to have to handle paging, given the issue you mentioned - but looking at the fix for it, it makes sense now.

There are other packages that use this method already, e.g. KiTTY.

@AdmiringWorm has pointed out that we should use authentication, due to the potential for rate limiting with the list of releases we're interested at every run.

@majkinetor
Copy link
Contributor

To fix this we need to now use the GitHub API, preferably in a PowerShell way with a module such as PowerShellForGitHub. And to achieve that, many packages need to be refactored.

🙄

Here is a quick fix. Trivial to change, I fixed all of my 15 or so failing packages in an hour.

JPRuskin added a commit to JPRuskin/chocolatey-community-packages that referenced this issue Oct 14, 2022
Adds a script to address the updates to GitHub Release parsing.
Updates chocolatey-community#2007
JPRuskin added a commit to JPRuskin/chocolatey-community-packages that referenced this issue Oct 14, 2022
JPRuskin added a commit to JPRuskin/chocolatey-community-packages that referenced this issue Oct 15, 2022
Adds a script to address the updates to GitHub Release parsing.
Updates chocolatey-community#2007
JPRuskin added a commit to JPRuskin/chocolatey-community-packages that referenced this issue Oct 15, 2022
Adds a script to address the updates to GitHub Release parsing.
Updates chocolatey-community#2007
JPRuskin added a commit to JPRuskin/chocolatey-community-packages that referenced this issue Oct 15, 2022
JPRuskin added a commit to JPRuskin/chocolatey-community-packages that referenced this issue Oct 15, 2022
JPRuskin added a commit to JPRuskin/chocolatey-community-packages that referenced this issue Oct 15, 2022
@JPRuskin
Copy link
Contributor

It's worth noting that I've got working branches for all packages currently mentioned in the original post, just waiting to merge PR #2013 so they can depend on the function being in a shared location.

@dieechtenilente
Copy link

notepadplusplus* is also affected

@brogers5 brogers5 mentioned this issue Oct 17, 2022
4 tasks
@pauby pauby pinned this issue Oct 18, 2022
@RedBaron2
Copy link
Contributor

@pauby
What would be the process to have the PowerShellForGitHub module imported used for this repository?

I just tried using #2013 for the FreeCAD package, and it will not return the URL items for development.
Though using the module and minor changes all items can be returned.

@pauby
Copy link
Member Author

pauby commented Oct 19, 2022

It sounds like @JPRuskin may have a solution that doesn't require it in that PR if we can get a few changes made.

AdmiringWorm pushed a commit to JPRuskin/chocolatey-community-packages that referenced this issue Oct 19, 2022
Adds a script to address the updates to GitHub Release parsing.
Updates chocolatey-community#2007
JPRuskin added a commit to JPRuskin/chocolatey-community-packages that referenced this issue Oct 19, 2022
Adds a script to address the updates to GitHub Release parsing.
Updates chocolatey-community#2007
@pauby pauby mentioned this issue Oct 19, 2022
4 tasks
@JPRuskin
Copy link
Contributor

@RedBaron2 I've got updates to the FreeCAD branch which a Get-GitHubRelease function. Looks like I need to redo that, but that's all good.

RedBaron2 added a commit to RedBaron2/chocolatey-coreteampackages that referenced this issue Oct 19, 2022
Update to use the new script Get-GitHubRelease from chocolatey-community#2007
@chimmmpie
Copy link

I don't know it will help but u can also use the atom feed to detect the latest version.
For example: https://github.com/notepad-plus-plus/notepad-plus-plus/releases.atom

JPRuskin added a commit that referenced this issue Dec 16, 2022
Updates to use the GitHub REST API for releases.
This adds to #2007.
JPRuskin added a commit that referenced this issue Dec 16, 2022
Updates to use the GitHub REST API for releases.
This adds to #2007.
JPRuskin added a commit that referenced this issue Dec 16, 2022
Updates to use the GitHub REST API for releases.
This adds to #2007.
JPRuskin added a commit that referenced this issue Dec 16, 2022
Updates to use the GitHub REST API for releases.
This adds to #2007.
JPRuskin added a commit that referenced this issue Dec 16, 2022
Updates to use the GitHub REST API for releases.
This adds to #2007.
JPRuskin added a commit that referenced this issue Dec 16, 2022
Updates to use the GitHub REST API for releases.
This adds to #2007.
JPRuskin added a commit that referenced this issue Dec 16, 2022
Updates to use the GitHub REST API for releases.
This adds to #2007.
JPRuskin added a commit that referenced this issue Dec 16, 2022
Updates to use the GitHub REST API for releases.
This adds to #2007.
JPRuskin added a commit that referenced this issue Dec 16, 2022
Updates to use the GitHub REST API for releases.
This adds to #2007.
JPRuskin added a commit that referenced this issue Dec 16, 2022
Updates to use the GitHub REST API for releases.
This adds to #2007.
JPRuskin added a commit that referenced this issue Dec 16, 2022
Updates to use the GitHub REST API for releases.
This adds to #2007.
JPRuskin added a commit that referenced this issue Dec 16, 2022
Updates to use the GitHub REST API for releases.
This adds to #2007.
JPRuskin added a commit that referenced this issue Dec 16, 2022
Updates to use the GitHub REST API for releases.
This adds to #2007.
JPRuskin added a commit that referenced this issue Dec 16, 2022
Updates to use the GitHub REST API for releases.
This adds to #2007.
JPRuskin added a commit that referenced this issue Dec 16, 2022
Updates to use the GitHub REST API for releases.
This adds to #2007.
JPRuskin added a commit that referenced this issue Dec 16, 2022
Updates to use the GitHub REST API for releases.
This adds to #2007.
JPRuskin added a commit to JPRuskin/chocolatey-community-packages that referenced this issue Dec 16, 2022
Updates to use the GitHub REST API for releases.
This adds to chocolatey-community#2007.
JPRuskin added a commit that referenced this issue Dec 16, 2022
Updates to use the GitHub REST API for releases.
This adds to #2007.
JPRuskin added a commit that referenced this issue Dec 16, 2022
Updates to use the GitHub REST API for releases.
This adds to #2007.
JPRuskin added a commit that referenced this issue Dec 16, 2022
Updates to use the GitHub REST API for releases.
This adds to #2007.
@pauby
Copy link
Member Author

pauby commented Jan 20, 2023

As all the packages in the original description have been completed, I'm going to close this.

@pauby
Copy link
Member Author

pauby commented Jan 20, 2023

Thanks to everybody who updated those packages. @JPRuskin did a lot of the work so thank you for that. If I have missed others, I apologize. You are welcome to shout out some kudos!

@pauby pauby unpinned this issue Feb 2, 2023
@koitsu
Copy link
Contributor

koitsu commented Sep 7, 2023

Is there a reason this cmdlet happens to share the same name as the official Microsoft-managed one (ref: source) yet uses completely incompatible flags/params? How are contributors supposed to test locally using this? I was unable to find any documentation on these cmdlets -- I literally came across scripts/ by total chance whilst doing a grep for Get-GitHubRelease.

@JPRuskin
Copy link
Contributor

JPRuskin commented Sep 7, 2023

Is there a reason this cmdlet happens to share the same name as the official Microsoft-managed one (ref: source) yet uses completely incompatible flags/params?

Naming is hard, but this seems fairly straightforward - Get-GitHubRelease is the obvious name for a function that gets a GitHub release.

I'd assume because we both used the same method of naming, but we (chocolatey-community) either didn't want to take a dependency on a large module for such a simple thing or didn't consider it. "Happens to" is about right.

Testing locally works for me, even though I have PowerShellForGitHub installed. I'll note that I do need to manually import the script (which is then used first) when I'm lazily running update scripts line by line, otherwise I get interesting parameter errors - but my general method for testing a package (assuming it's working) is to run .\update_all.ps1 -name <ID>, which handles ensuring the scripts are available.

We should probably create a separate discussion around this, as I don't think it contributes to this closed issue - if I create one, would you be happy to discuss how you'd prefer to see these scripts documented? We do have open issues/PR (1754, 1940, 2155) for updating the contribution guidelines, I believe - though possibly they should be kept to the scope they currently have, and we can chat about this in a new discussion.

@koitsu
Copy link
Contributor

koitsu commented Sep 7, 2023

@JPRuskin "Interesting parameter errors" are exactly what I'm talking about. Let me be crystal clear here with syntax:

  1. chocolatey-packages/scripts/Get-GitHubRelease.ps1 -- Get-GetHubRelease ghuser reponame
  2. PowerShellForGitHub -- Get-GitHubRelease -OwnerName ghuser -RepositoryName reponame -Latest

If you try to use (1)s syntax with PowerShellForGitHub, you will get parameter errors. If you try to use (2)s syntax with chocolatey-packages/scripts/Get-GitHubRelease.ps1, you will get parameter errors. These are syntactically incompatible with each other, yet new contributors wishing to add to chocolatey-packages must write code that works with cmdlets dropped into chocolatey-packages/scripts to be compliant.

If you are manually importing chocolatey-packages/scripts/Get-GitHubRelease.ps1 which effectively lets PowerShell override the cmdlet function from PowerShellForGitHub, then yes, that is exactly why you don't encounter this problem.

My recommendations for action -- pick one:

  • In all of this repository's documentation, make it crystal clear that contributors need to comply with cmdlets that exist in scripts/ and not things they find on the Internet (even if from official sources),
  • (I do not know PowerShell well enough to know if this is possible): Improve chocolatey-packages/scripts/Get-GitHubRelease.ps1 to be both backwards-compatible (index-based parameters) as well as support the 3 parameters from Microsoft's version (-OwnerName, -RepositoryName, and -Latest (which if not specified should be the default anyway)).

Thank you!

@pauby
Copy link
Member Author

pauby commented Sep 8, 2023

@koitsu This issue is closed as the issue this was raised for has been resolved. Please raise a new issue for this.

@chocolatey-community chocolatey-community locked as resolved and limited conversation to collaborators Sep 8, 2023
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

No branches or pull requests

9 participants