brew icon indicating copy to clipboard operation
brew copied to clipboard

Augmented the `brew livecheck` command with a `-r` or `--resources` option to check resources for a given Formulae

Open mohammadzainabbas opened this issue 3 years ago • 9 comments

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

This PR aims to extend the brew livecheck command to work for resources as well.


I have added an option -r or --resources in brew livecheck command which do a livecheck for resources when given a particular Formulae.

For instance: inside Library/Taps/homebrew/homebrew-core/Formula/influxdb.rb, add the following livecheck block for pkg-config-wrapper resource:

...
resource "pkg-config-wrapper" do
    url "https://github.com/influxdata/pkg-config/archive/refs/tags/v0.2.11.tar.gz"
    sha256 "52b22c151163dfb051fd44e7d103fc4cde6ae8ff852ffc13adeef19d21c36682"

    livecheck do
      url "https://github.com/influxdata/pkg-config/tags"
      regex(%r{href="/influxdata/pkg-config/archive/refs/tags/v(.*).tar.gz"}i)
      strategy :page_match
    end
  end
...

and then run the following command:

brew livecheck influxdb --resources

or

brew livecheck influxdb -r

you will get the resources' information as well (as expected)

influxdb: 2.3.0 ==> 2.3.0
-- pkg-config-wrapper: 0.2.11 ==> 0.2.12
-- ui-assets: 2.1.2 ==> 2.1.2

Also, this PR includes the implementation for resource information in the default, --debug, and --json output for resources (will be needing valuable feedback for this)

brew livecheck influxdb --resources --debug

will give us the below output

Formula:          influxdb
Livecheckable?:   Yes

URL (stable):     https://github.com/influxdata/influxdb.git
Strategy:         Git
Regex:            /^v?((?!9\.9\.9)\d+(?:\.\d+)+)$/i

Matched Versions:
1.8.0, 0.0.1, 0.0.2, 0.0.3, 0.0.4, 0.0.5, 0.0.6, 0.0.7, 0.0.8, 0.0.9, 0.1.0, 0.10.0, 0.10.1, 0.10.2, 0.10.3, 0.11.0, 0.11.1, 0.12.0, 0.12.1, 0.12.2, 0.13.0, 0.2.0, 0.3.0, 0.3.1, 0.3.2, 0.4.0, 0.4.1, 0.4.2, 0.4.3, 0.4.4, 0.5.0, 0.5.1, 0.5.10, 0.5.11, 0.5.12, 0.5.2, 0.5.3, 0.5.4, 0.5.5, 0.5.6, 0.5.7, 0.5.8, 0.5.9, 0.6.0, 0.6.1, 0.6.2, 0.6.3, 0.6.4, 0.6.5, 0.7.0, 0.7.1, 0.7.2, 0.7.3, 0.8.0, 0.8.1, 0.8.2, 0.8.3, 0.8.4, 0.8.5, 0.8.6, 0.8.7, 0.8.8, 0.9.0, 0.9.1, 0.9.2, 0.9.2.1, 0.9.3, 0.9.4, 0.9.4.1, 0.9.4.2, 0.9.5, 0.9.5.1, 0.9.6, 0.9.6.1, 1.0.0, 1.0.1, 1.0.2, 1.1.0, 1.1.1, 1.1.2, 1.1.3, 1.1.4, 1.1.5, 1.10.0, 1.2.0, 1.2.1, 1.2.2, 1.2.3, 1.2.4, 1.3.0, 1.3.1, 1.3.2, 1.3.3, 1.3.4, 1.3.5, 1.3.6, 1.3.7, 1.3.8, 1.3.9, 1.4.0, 1.4.1, 1.4.2, 1.4.3, 1.5.0, 1.5.1, 1.5.2, 1.5.3, 1.5.4, 1.5.5, 1.6.0, 1.6.1, 1.6.2, 1.6.3, 1.6.4, 1.6.5, 1.6.6, 1.7.0, 1.7.1, 1.7.10, 1.7.11, 1.7.2, 1.7.3, 1.7.4, 1.7.5, 1.7.6, 1.7.7, 1.7.8, 1.7.9, 1.8.1, 1.8.10, 1.8.2, 1.8.3, 1.8.4, 1.8.5, 1.8.6, 1.8.7, 1.8.8, 1.8.9, 1.9.0, 1.9.1, 1.9.2, 1.9.3, 1.9.4, 1.9.5, 1.9.6, 1.9.7, 1.9.8, 2.0.0, 2.0.1, 2.0.2, 2.0.3, 2.0.4, 2.0.5, 2.0.6, 2.0.7, 2.0.8, 2.0.9, 2.1.0, 2.1.1, 2.2.0, 2.3.0


Resource:          pkg-config-wrapper
Livecheckable?:    Yes

URL:              https://github.com/influxdata/pkg-config/tags
Strategy:         PageMatch
Regex:            /href="\/influxdata\/pkg-config\/archive\/refs\/tags\/v(.*).tar.gz"/i

Matched Versions:
0.2.12, 0.2.11, 0.2.10, 0.2.9, 0.2.8, 0.2.7, 0.2.6, 0.2.5, 0.2.4, 0.2.3


Resource:          ui-assets
Livecheckable?:    No

URL:              https://github.com/influxdata/ui/releases/download/OSS-2.1.2/build.tar.gz
URL (processed):  https://github.com/influxdata/ui.git
Strategy:         Git

Matched Versions:
r, 2.1.0, 2.1.1, 2.1.2, 9.9.9.test, 2.0.5, 2.0.7, 2.0.8, 2.0.9

influxdb: 2.3.0 ==> 2.3.0
  pkg-config-wrapper: 0.2.11 ==> 0.2.12
  ui-assets: 2.1.2 ==> 9.9.9.test

and

brew livecheck influxdb --resources --json

will output the following

[
  {
    "formula": "influxdb",
    "version": {
      "current": "2.3.0",
      "latest": "2.3.0",
      "outdated": false,
      "newer_than_upstream": false
    },
    "resources": [
      {
        "resource": "pkg-config-wrapper",
        "version": {
          "current": "0.2.11",
          "latest": "0.2.12",
          "newer_than_upstream": false,
          "outdated": true
        }
      },
      {
        "resource": "ui-assets",
        "version": {
          "current": "2.1.2",
          "latest": "9.9.9.test",
          "newer_than_upstream": false,
          "outdated": true
        }
      }
    ]
  }
]

Kindly, do let me know if there's something missing or any further improvement that we can do here.

mohammadzainabbas avatar Jul 27 '22 17:07 mohammadzainabbas

I have refactored the code for resources in livecheck command. Now, it works for resources without livecheck block (as it should). I have also updated code to get meta information as well (when --json --verbose flags are given for resources). i.e:

brew livecheck influxdb --resources --json --verbose

will now print the following output:

[
  {
    "formula": "influxdb",
    "version": {
      "current": "2.3.0",
      "latest": "2.3.0",
      "outdated": false,
      "newer_than_upstream": false
    },
    "meta": {
      "livecheckable": true,
      "url": {
        "symbol": "stable",
        "original": "https://github.com/influxdata/influxdb.git"
      },
      "strategy": "Git",
      "strategies": [
        "Git",
        "PageMatch"
      ],
      "regex": "/^v?((?!9\\.9\\.9)\\d+(?:\\.\\d+)+)$/i"
    },
    "resources": [
      {
        "resource": "pkg-config-wrapper",
        "version": {
          "current": "0.2.11",
          "latest": "0.2.12",
          "newer_than_upstream": false,
          "outdated": true
        },
        "meta": {
          "url": {
            "original": "https://github.com/influxdata/pkg-config/tags"
          },
          "livecheckable": "Yes",
          "livecheck": {
            "url": {
              "original": "https://github.com/influxdata/pkg-config/tags"
            },
            "strategy": "PageMatch",
            "strategies": [
              "PageMatch"
            ],
            "regex": "/href=\"\\/influxdata\\/pkg-config\\/archive\\/refs\\/tags\\/v(.*).tar.gz\"/i"
          }
        }
      },
      {
        "resource": "ui-assets",
        "version": {
          "current": "2.1.2",
          "latest": "9.9.9.test",
          "newer_than_upstream": false,
          "outdated": true
        },
        "meta": {
          "url": {
            "original": "https://github.com/influxdata/ui/releases/download/OSS-2.1.2/build.tar.gz",
            "processed": "https://github.com/influxdata/ui.git"
          },
          "livecheckable": "No"
        }
      }
    ]
  }
]

Also, I have updated bash, zsh and fish completions for brew livecheck command to incorporate --resources flag.

Now, I am looking into the tests which needs to be updated due to this update in brew livecheck command.


I had fixed all the issues with brew style except the block length issue for resource_version.

Inspecting 1098 files
......................................................................................................................................................................................................................................................................................................................................C...................................................................................................................................................................................................................................................................................................................................................................................................................................................................................................................................................................................................................................................................................................................................................................................................

Offenses:

Library/Homebrew/livecheck/livecheck.rb:662:7: C: Block has too many lines. [121/106]
      urls.each_with_index do |original_url, i| ...
      ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^

1098 files inspected, 1 offense detected

So, I disabled the block length issue (just like it's being done in latest_version) via # rubocop:disable Metrics/BlockLength. Do let me know if we can fix this issue without introducing this flag.

mohammadzainabbas avatar Aug 07 '22 17:08 mohammadzainabbas

Now, I am looking into the tests which needs to be updated due to this update in brew livecheck command.

Update: I have updated the tests for brew livecheck (for the methods that I modified during this PR), and verified locally by running:

brew tests --only=livecheck/livecheck

and

brew tests --only=livecheck/livecheck_version

mohammadzainabbas avatar Aug 07 '22 19:08 mohammadzainabbas

As per my discussion with @nandahkrishna, I have removed the usage of use_homebrew_curl for resources (in resource_version method) because in Resource class, :url is of type String and we can't do something like resource.send(:url).using == :homebrew_curl (unlike for e.g: when defined, in Formula, :stable or :head are of type SoftwareSpec, which enable us to do something like formula.send(:stable).using == :homebrew_curl).

I think for the time being, livecheck for resources can work without use_homebrew_curl. If needed, we may need to extend Resource class in the future to accommodate this functionality.

Do let me know if you have a better suggestion for this.

mohammadzainabbas avatar Aug 11 '22 19:08 mohammadzainabbas

I'm taking some time to test this and go through the code in detail, so I'll hopefully post my comments by tonight (ET).

nandahkrishna avatar Aug 15 '22 19:08 nandahkrishna

I'm also pinging Sam to review this when they can, as they may be able to spot something that I've missed in my review.

I ran out of time tonight but I'll try to take another pass through this tomorrow evening (EDT).

Edit: I unexpectedly had to go to the eye doctor today (Wednesday) as I woke up this morning with some persistent pain/discomfort and I wasn't able to get to this like I planned. I'm hoping that my eye will recover enough for me to look through this in the next day or two but that remains to be seen. If I'm not able to review this within the next few days, don't feel the need to hold this up on my behalf (i.e., I trust Nanda's judgment).

samford avatar Aug 17 '22 03:08 samford

This pull request has been automatically marked as stale because it has not had recent activity. It will be closed if no further activity occurs.

github-actions[bot] avatar Sep 13 '22 18:09 github-actions[bot]

After verifying and testing locally the changes suggested by @nandahkrishna and @samford, I have updated this PR. Now, we account for skip livecheck blocks for resources.

For instance: inside Library/Taps/homebrew/homebrew-core/Formula/influxdb.rb, add the following livecheck block for pkg-config-wrapper resource:

...
  resource "pkg-config-wrapper" do
    url "https://github.com/influxdata/pkg-config/archive/refs/tags/v0.2.11.tar.gz"
    sha256 "52b22c151163dfb051fd44e7d103fc4cde6ae8ff852ffc13adeef19d21c36682"

    livecheck do
      skip "No longer actively developed"
    end
  end
...

and then run the following command:

brew livecheck influxdb -r

you will get the see the SkipConditions.print_skip_information will work as expected:

influxdb: 2.4.0 ==> 2.4.0
  pkg-config-wrapper: skipped - No longer actively developed
  ui-assets: 2.4.0 ==> 2022-09-01

and after running:

brew livecheck influxdb -rj

you will get the json output in the following manner:

[
  {
    "formula": "influxdb",
    "version": {
      "current": "2.4.0",
      "latest": "2.4.0",
      "outdated": false,
      "newer_than_upstream": false
    },
    "resources": [
      {
        "resource": "pkg-config-wrapper",
        "status": "skipped",
        "messages": [
          "No longer actively developed"
        ]
      },
      {
        "resource": "ui-assets",
        "version": {
          "current": "2.4.0",
          "latest": "2022-09-01",
          "newer_than_upstream": false,
          "outdated": true
        }
      }
    ]
  }
]

Also as suggested, if the formula somehow results in error when the -r resource flag is given, it will still show the output for resources:

Error: influxdb: Unable to get versions
  pkg-config-wrapper: 0.2.11 ==> 0.2.12
  ui-assets: 2.4.0 ==> 2022-09-01

and the json output is:

[
  {
    "formula": "influxdb",
    "status": "error",
    "messages": [
      "Unable to get versions"
    ],
    "meta": {
      "livecheckable": true
    },
    "resources": [
      {
        "resource": "pkg-config-wrapper",
        "version": {
          "current": "0.2.11",
          "latest": "0.2.12",
          "newer_than_upstream": false,
          "outdated": true
        },
        "meta": {
          "url": {
            "original": "https://github.com/influxdata/pkg-config/tags"
          },
          "livecheckable": true,
          "livecheck": {
            "url": {
              "original": "https://github.com/influxdata/pkg-config/tags"
            },
            "strategy": "PageMatch",
            "strategies": [
              "PageMatch"
            ],
            "regex": "/href=\"\\/influxdata\\/pkg-config\\/archive\\/refs\\/tags\\/v(.*).tar.gz\"/i"
          }
        }
      },
      {
        "resource": "ui-assets",
        "version": {
          "current": "2.4.0",
          "latest": "2022-09-01",
          "newer_than_upstream": false,
          "outdated": true
        },
        "meta": {
          "url": {
            "original": "https://github.com/influxdata/ui/releases/download/OSS-Master/build.tar.gz",
            "processed": "https://github.com/influxdata/ui.git"
          },
          "livecheckable": false
        }
      }
    ]
  }
]

Lastly, if the resource latest version is not found (if the latest version is blank for resource):

influxdb: 2.4.0 ==> 2.4.0
  pkg-config-wrapper: 0.2.11 ==> 0.2.12
  ui-assets: Unable to get versions

and the json output is:

[
  {
    "formula": "influxdb",
    "version": {
      "current": "2.4.0",
      "latest": "2.4.0",
      "outdated": false,
      "newer_than_upstream": false
    },
    "resources": [
      {
        "resource": "pkg-config-wrapper",
        "version": {
          "current": "0.2.11",
          "latest": "0.2.12",
          "newer_than_upstream": false,
          "outdated": true
        }
      },
      {
        "resource": "ui-assets",
        "status": "error",
        "messages": [
          "Unable to get versions"
        ]
      }
    ]
  }
]

There is only one issue that I am currently running into, and it's about Cyclomatic complexity for run_checks method. I am not sure how I can reduce the complexity any further since I have tried to minimise the checks and conditions wherever possible.

Offenses:

Library/Homebrew/livecheck/livecheck.rb:173:5: C: Cyclomatic complexity for run_checks is too high. [83/80]
    def run_checks( ...
    ^^^^^^^^^^^^^^^

1110 files inspected, 1 offense detected

Kindly, do let me know if there is something missing or any further improvement that we can do here.

mohammadzainabbas avatar Sep 14 '22 04:09 mohammadzainabbas

I tested this PR on a couple of formulae like go and I'm not sure we're quite there yet. Here's the output:

Upon debugging, this happened when we don't have any strategy for a Formula's resource. For e.g: For go's resource gobootstrap (I ran the following):

brew lc go -rd

you will see that

...
Resource:         gobootstrap
Livecheckable?:   No

URL:              https://storage.googleapis.com/golang/go1.16.darwin-amd64.tar.gz
Strategy:         None
...

this tells that we don't have any strategy for gobootstrap resource.

However, I have made the changes and now you should get an expected output:

brew lc go -r

will result in

go: 1.19.1 ==> 1.19.1
  gobootstrap: Unable to get versions

and

brew lc go -rj

will yield

[
  {
    "formula": "go",
    "version": {
      "current": "1.19.1",
      "latest": "1.19.1",
      "outdated": false,
      "newer_than_upstream": false
    },
    "resources": [
      {
        "resource": "gobootstrap",
        "status": "error",
        "messages": [
          "Unable to get versions"
        ]
      }
    ]
  }
]

I was able to reproduce this with influxdb by adding a livecheck block with an invalid/unreachable URL, or a regex that doesn't match any version text (e.g. /(alpha-beta-gamma-delta)/i).

This was another issue, but now it's being handled as well.

mohammadzainabbas avatar Sep 18 '22 15:09 mohammadzainabbas

The feature works as intended, but there is one small problem: an error in the resource check (e.g. Unable to get versions) does not affect the status code of the brew livecheck command.

@nandahkrishna Are you suggesting to change status code for brew livecheck command via Homebrew.failed = true whenever we have an error for any resource (for any formula) ?

mohammadzainabbas avatar Sep 20 '22 20:09 mohammadzainabbas

The failing tests here are a known issue (discussed on Slack) and unrelated to the changes made in this PR. We can merge once the issue is fixed.

nandahkrishna avatar Sep 27 '22 08:09 nandahkrishna

As discussed, this feature is still far from complete and the code definitely needs to be refactored, but we can take care of this after GSoC ends (I'm very eager to contribute to this!).

Thank you @samford and @nandahkrishna for your valuable feedbacks. I would very much like to take part in code refactoring as well (after GSoC) and keep contributing to Homebrew code base (esp. livecheck module).

mohammadzainabbas avatar Sep 27 '22 10:09 mohammadzainabbas

The issue seems fixed, so I'll be merging this.

nandahkrishna avatar Oct 02 '22 17:10 nandahkrishna