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

bump: add --start-with option to retrieve a subset of results #12121

Merged
merged 2 commits into from Oct 20, 2021

Conversation

EricFromCanada
Copy link
Member

@EricFromCanada EricFromCanada commented Sep 25, 2021

  • Have you followed the guidelines in our Contributing document?
  • Have you checked to ensure there aren't other open Pull Requests for the same change?
  • Have you added an explanation of what your changes do and why you'd like us to include them?
  • Have you written new tests for your changes? Here's an example.
  • Have you successfully run brew style with your changes locally?
  • Have you successfully run brew typecheck with your changes locally?
  • Have you successfully run brew tests with your changes locally?

Re-enables the Repology methods disabled in #11624 since they appear to be working again. Also implements the following for brew bump:

  • --start-with which, when passed a letter or word, fetches results that follow that term (inclusive)
  • reordered results so the livecheck version is in between the formula version and the Repology version, since it's more likely to reflect the ideal version for the formula
  • fix for --limit showing one more result than requested
  • fixes for API returning results in non-alphabetical order
  • output "cask version" for casks
  • headings and extra line break if listing both formulae and casks
  • display "[name] is up to date!" line in green
  • automatically install curl to ensure TLS 1.3 support

Sample output:

$ brew bump --start-with=z
==> Querying outdated formulae from Repology
2 outdated formulae found

==> Querying outdated casks from Repology
2 outdated casks found

==> Formulae
==> zeek
Current formula version:  4.1.0
Latest livecheck version: 4.1.1
Latest Repology version:  4.1.1
Open pull requests:       zeek 4.1.1 (https://github.com/Homebrew/homebrew-core/pull/85849)

==> zorba
Current formula version:  3.1
Latest livecheck version: 3.1
Latest Repology version:  4.0
Open pull requests:       none

==> Casks
==> zandronum
Current cask version:     3.0
Latest livecheck version: unable to get versions
Latest Repology version:  3.0.1
Open pull requests:       none

==> zeplin
Current cask version:     3.24,1276
Latest livecheck version: 3.24,1276
Latest Repology version:  6.8.0
Open pull requests:       none

@BrewTestBot
Copy link
Member

Review period will end on 2021-09-28 at 00:00:00 UTC.

@BrewTestBot BrewTestBot added the waiting for feedback Merging is blocked until sufficient time has passed for review label Sep 27, 2021
@BrewTestBot BrewTestBot removed the waiting for feedback Merging is blocked until sufficient time has passed for review label Sep 27, 2021
@MikeMcQuaid
Copy link
Member

Code looks good!

What about something like --starts-with or something?

@EricFromCanada EricFromCanada marked this pull request as draft September 28, 2021 20:36
@EricFromCanada EricFromCanada changed the title bump: add --initial option to retrieve a subset of results bump: add --start-with option to retrieve a subset of results Sep 28, 2021
@BrewTestBot
Copy link
Member

Review period ended.

@EricFromCanada
Copy link
Member Author

EricFromCanada commented Sep 28, 2021

Both of the repology.org servers appear to again be TLS 1.3-only, which macOS's shipping curl doesn't support.

% /usr/bin/curl --tlsv1.2 --tls-max 1.2 --disable --cookie-jar /dev/null --globoff --show-error --user-agent Homebrew/3.2.13-78-gf7d8e4d-dirty\ \(Macintosh\;\ Intel\ Mac\ OS\ X\ 10.12.6\)\ curl/7.54.0 --header Accept-Language:\ en --retry 3 https://repology.org/api/v1/projects/\?inrepo=homebrew\&outdated=1
curl: (35) error:1400442E:SSL routines:CONNECT_CR_SRVR_HELLO:tlsv1 alert protocol version
% /usr/bin/curl --tlsv1.3 --tls-max 1.3 --disable --cookie-jar /dev/null --globoff --show-error --user-agent Homebrew/3.2.13-78-gf7d8e4d-dirty\ \(Macintosh\;\ Intel\ Mac\ OS\ X\ 10.12.6\)\ curl/7.54.0 --header Accept-Language:\ en --retry 3 https://repology.org/api/v1/projects/\?inrepo=homebrew\&outdated=1
curl: (4) LibreSSL was built without TLS 1.3 support

@MikeMcQuaid
Copy link
Member

@EricFromCanada oh no! I think reasonable to make brew bump require curl to be installed in those cases?

@EricFromCanada EricFromCanada marked this pull request as ready for review September 29, 2021 15:01
@EricFromCanada
Copy link
Member Author

It's now working after making use of use_homebrew_curl from #11773. Also added a warning if brewed curl is not installed.

Copy link
Member

@MikeMcQuaid MikeMcQuaid left a comment

Choose a reason for hiding this comment

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

Looks good once tests are fixed!

@@ -53,6 +55,8 @@ def bump

limit = args.limit.to_i if args.limit.present?

opoo "Homebrew-installed `curl` is required for Repology queries" unless Formula["curl"].any_version_installed?
Copy link
Member

Choose a reason for hiding this comment

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

Need to handle curl formula not existing.

@EricFromCanada
Copy link
Member Author

Another thought: it might be useful if, when run without specifying a list of packages, the command only checked for open pull requests if the current version does not match the livecheck version.

@EricFromCanada EricFromCanada force-pushed the restore-repology branch 2 times, most recently from 64189ea to 0299703 Compare September 29, 2021 19:00
@EricFromCanada
Copy link
Member Author

Currently stuck on how to get repology_spec to pass, either by a) installing TLS 1.3-capable curl during the test, or b) faking the query and its result somehow.

@MikeMcQuaid
Copy link
Member

installing TLS 1.3-capable curl during the test

I wonder if we can do something like install a curl before the test and then pass the path for it through in a way that will be used by all tests?

@EricFromCanada EricFromCanada force-pushed the restore-repology branch 6 times, most recently from 6dbec1e to a40f148 Compare October 17, 2021 21:34
@EricFromCanada
Copy link
Member Author

My current workaround is to add the new :needs_tls13 flag to the associated tests so they only run on systems whose built-in curl includes TLS 1.3 support, i.e. ubuntu-latest. For everyone else, using brew bump will have it install and use brewed curl.

Copy link
Member

@MikeMcQuaid MikeMcQuaid left a comment

Choose a reason for hiding this comment

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

👍🏻. Would be good to install curl as part of the GitHub Actions tests to avoid the skip warning.

Copy link
Member

@MikeMcQuaid MikeMcQuaid left a comment

Choose a reason for hiding this comment

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

Looks great, nice work as usual @EricFromCanada!

@MikeMcQuaid MikeMcQuaid merged commit b77b08c into Homebrew:master Oct 20, 2021
@EricFromCanada EricFromCanada deleted the restore-repology branch October 20, 2021 22:21
@EricFromCanada EricFromCanada mentioned this pull request Oct 22, 2021
7 tasks
@github-actions github-actions bot added the outdated PR was locked due to age label Nov 20, 2021
@github-actions github-actions bot locked as resolved and limited conversation to collaborators Nov 20, 2021
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
outdated PR was locked due to age
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants