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 head_only? to Formula, replace head? in livecheck #8680

Merged
merged 2 commits into from
Oct 5, 2020
Merged

Add head_only? to Formula, replace head? in livecheck #8680

merged 2 commits into from
Oct 5, 2020

Conversation

nandahkrishna
Copy link
Member

  • 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 tests with your changes locally?

The brew livecheck --installed command is supposed to run livecheck for all formulae that have been installed. As livecheck reports new versions of software from upstream (compared to the homebrew-core version and not the system-installed version), one would expect this command to work just like brew livecheck does but only limits the Formulae checked.

However,

➜ brew livecheck arduino-cli     
arduino-cli : 0.12.1 ==> 0.12.1
➜ brew livecheck --installed                   
adns : 1.6.0 ==> 1.6.0
anime-downloader : 4.6.4 ==> 4.6.4
aom : 2.0.0 ==> 2.0.0
==> Cloning https://github.com/arduino/arduino-cli.git
Updating /Users/nanda/Library/Caches/Homebrew/arduino-cli--git
From https://github.com/arduino/arduino-cli
   f1877ef..693a045  master     -> origin/master
==> Checking out branch master
Already on 'master'
Your branch is behind 'origin/master' by 1 commit, and can be fast-forwarded.
  (use "git pull" to update your local branch)
HEAD is now at 693a045 [skip changelog] Completely document resources not inherited from referenced platform (#940)
arduino-cli : b8cf32d ==> 693a045

This happens because I have a HEAD-version of arduino-cli installed, despite it not being a HEAD-only formula.

This PR fixes the --installed flag to check only installed formulae, but report their stable version instead (if it exists).

➜ brew livecheck --installed
adns : 1.6.0 ==> 1.6.0
anime-downloader : 4.6.4 ==> 4.6.4
aom : 2.0.0 ==> 2.0.0
arduino-cli : 0.12.1 ==> 0.12.1

For a HEAD-only formula, we continue to report HEAD version updates.

One problem still persists, for HEAD-only formulae, the output of fetch_latest_commit is not silenced despite the formula.head.downloader.shutup!. While I tried looking through Context and AbstractDownloadStrategy, I wasn't able to come up with a fix. It seems like setting @quiet = true isn't working. Any idea why?

Sorry, something went wrong.

@dawidd6
Copy link
Contributor

dawidd6 commented Sep 10, 2020

You can try adding @quiet = false below this line:

@nandahkrishna
Copy link
Member Author

nandahkrishna commented Sep 13, 2020

As suggested by Sam (and discussed previously), I've added head_only? to the Formula class, which partially fixes this issue. However, adding that alone didn't seem to fix the --installed check for me, as formula.version returned the head version which was installed. I then changed the value of current to formula.stable.version, and that seems to solve the issue. Let me know if that was different from what you tried out @samford.

I've also added a comment stating that all formulae will have normal checks carried out unless they are HEAD-only (worded almost exactly like Sam's comment in their review) to dev-cmd/livecheck.rb.

The only issue that remains to be solved is that of the downloader continuing to be verbose, I'll implement Dawid's suggestion and let you all know if it solves the problem.

Do let me know if anything has to be modified. Thanks!

@nandahkrishna nandahkrishna changed the title livecheck: fix --installed Add head_only? to Formula, replace head? in livecheck Sep 13, 2020
Copy link
Member

@samford samford left a comment

Choose a reason for hiding this comment

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

changed the value of current to formula.stable.version, and that seems to solve the issue

I tested this and can confirm that the reported version for a formula with stable and head that's installed using --head is the HEAD commit hash when running brew livecheck --installed. Using formula.stable.version instead of formula.version addresses this and makes sense to me 👍

@nandahkrishna
Copy link
Member Author

Sorry for the delay, was slightly occupied with some other work. With the latest push:

  • The comment was reworded and relocated (based on Sam's review).
  • It seems shutup! is only used by livecheck, or that's what it seems like to me from a grep -R. Also, quiet? only checks if the current Context has @quiet set to true, which seems to happen only when the --quiet flag is passed. Taking inspiration from mktemp.rb and Dawid's suggestion, I've defined a new quiet? specific to AbstractDownloadStrategy. Now, there is no output from fetch_last_commit during livecheck runs.

@nandahkrishna nandahkrishna merged commit a74565f into Homebrew:master Oct 5, 2020
@nandahkrishna nandahkrishna deleted the livecheck-head branch October 5, 2020 08:56
@BrewTestBot BrewTestBot added the outdated PR was locked due to age label Dec 3, 2020
@Homebrew Homebrew locked as resolved and limited conversation to collaborators Dec 3, 2020
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

5 participants