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

brew bump-formula-pr and brew bump-cask-pr fail #15342

Closed
3 tasks done
xrisk opened this issue May 1, 2023 · 9 comments
Closed
3 tasks done

brew bump-formula-pr and brew bump-cask-pr fail #15342

xrisk opened this issue May 1, 2023 · 9 comments
Assignees
Labels
bug Reproducible Homebrew/brew bug in progress Maintainers are working on this install from api Relates to API installs outdated PR was locked due to age

Comments

@xrisk
Copy link

xrisk commented May 1, 2023

brew doctor output

Your system is ready to brew.

Verification

  • My "brew doctor output" above says Your system is ready to brew. and am still able to reproduce my issue.
  • I ran brew update twice and am still able to reproduce my issue.
  • This issue's title and/or description do not reference a single formula e.g. brew install wget. If they do, open an issue at https://github.com/Homebrew/homebrew-core/issues/new/choose instead.

brew config output

HOMEBREW_VERSION: 4.0.15-126-g01e0c20
ORIGIN: https://github.com/Homebrew/brew
HEAD: 01e0c20d01c6aab28d844280d130f0e745e2ddd9
Last commit: 8 hours ago
Core tap JSON: 01 May 02:21 UTC
HOMEBREW_PREFIX: /opt/homebrew
HOMEBREW_CASK_OPTS: []
HOMEBREW_DISPLAY: /private/tmp/com.apple.launchd.LZcoXQR5bk/org.xquartz:0
HOMEBREW_EDITOR: nvim
HOMEBREW_GITHUB_API_TOKEN: set
HOMEBREW_MAKE_JOBS: 8
Homebrew Ruby: 2.6.10 => /System/Library/Frameworks/Ruby.framework/Versions/2.6/usr/bin/ruby
CPU: octa-core 64-bit arm_firestorm_icestorm
Clang: 14.0.3 build 1403
Git: 2.40.1 => /opt/homebrew/bin/git
Curl: 7.87.0 => /usr/bin/curl
macOS: 13.3.1-arm64
CLT: 14.3.0.0.1.1679647830
Xcode: N/A
Rosetta 2: false

What were you trying to do (and why)?

Update the WhatsApp cask.

What happened (include all command output)?

~
08:01:13 ❯ brew bump whatsapp
==> whatsapp
Current cask version   :  2.2317.10
Latest livecheck version: unable to get versions
Latest Repology version:  2.2318.10
Open pull requests:       none
Closed pull requests:     none
~
08:01:22 ❯ brew bump-cask-pr --version 2.2318.10 whatsapp
Error: This cask's tap is not a Git repository!

What did you expect to happen?

I expected brew to create a PR.

Step-by-step reproduction instructions (by running brew commands)

08:01:27 ❯ brew bump-cask-pr -d --version 2.2318.10 whatsapp
Error: This cask's tap is not a Git repository!
Error: Kernel.exit
/opt/homebrew/Library/Homebrew/extend/kernel.rb:83:in `exit'
/opt/homebrew/Library/Homebrew/extend/kernel.rb:83:in `odie'
/opt/homebrew/Library/Homebrew/dev-cmd/bump-cask-pr.rb:76:in `bump_cask_pr'
/opt/homebrew/Library/Homebrew/brew.rb:94:in `<main>'

or

2. 08:03:27 ❯ brew bump-formula-pr -d p7zip
/opt/homebrew/Library/Homebrew/brew.rb (Formulary::FormulaLoader): loading /opt/homebrew/opt/p7zip/.brew/p7zip.rb
Error: This formula is not in a tap!
Error: Kernel.exit
/opt/homebrew/Library/Homebrew/extend/kernel.rb:83:in `exit'
/opt/homebrew/Library/Homebrew/extend/kernel.rb:83:in `odie'
/opt/homebrew/Library/Homebrew/dev-cmd/bump-formula-pr.rb:112:in `bump_formula_pr'
/opt/homebrew/Library/Homebrew/brew.rb:94:in `<main>'
@xrisk xrisk added the bug Reproducible Homebrew/brew bug label May 1, 2023
@carlocab carlocab added the install from api Relates to API installs label May 1, 2023
@apainintheneck
Copy link
Contributor

To bump formulae or casks you need to have a local tap which is essentially a git repo with all of the package files. These then can get changed and the resulting changes pushed to the remote repos using normal git operations. These dev commands do that work for you for trivial version upgrades.

The problem here is that it seems you're using the API to manage core casks and formulae. This is fine for all of the normal install, upgrade, reinstall and uninstall actions but means that you don't have local taps installed at all. Instead, you're getting the information in a bundle of JSON from the API.

Long story short, it's saying here that you can't create a pull request using changes from your local tap because you don't have one. If I remember correctly you'll need to tap homebrew/core and homebrew/cask to be able to do that along with setting HOMEBREW_NO_INSTALL_FROM_API so that it knows to load packages from the local taps instead of the API.

I wonder if this is documented anywhere already and if it'd be helpful to maybe add some better messaging here for this specific edge case.

@MikeMcQuaid MikeMcQuaid added the help wanted We want help addressing this label May 1, 2023
@MikeMcQuaid
Copy link
Member

I wonder if this is documented anywhere already and if it'd be helpful to maybe add some better messaging here for this specific edge case.

Agreed. This command (and others like it) should error if core/cask aren't tapped in cases like this.

@apainintheneck
Copy link
Contributor

Actually, I think I misdiagnosed this. We already force HOMEBREW_NO_INSTALL_FROM_API with these commands. What I don't understand is how you're able to load a formula or cask from a local tap and then later on it says that it's not a git repo. I'm confused about where these are getting loaded from though.

It looks like the formula is not actually being loaded from a tap. It's getting loaded from the opt directory instead.

  1. 08:03:27 ❯ brew bump-formula-pr -d p7zip
    /opt/homebrew/Library/Homebrew/brew.rb (Formulary::FormulaLoader): loading /opt/homebrew/opt/p7zip/.brew/p7zip.rb

It should look something like this.

/usr/local/Homebrew/Library/Homebrew/brew.rb (Formulary::FormulaLoader): loading /usr/local/Homebrew/Library/Taps/homebrew/homebrew-core/Formula/p7zip.rb

Could you maybe try running this command in the console?

echo '
cask = :whatsapp.c; nil
cask.sourcefile_path
cask.tap.path
cask.tap.git?

formula = :p7zip.f
formula.tap.path
formula.tap.git?
' | HOMEBREW_NO_INSTALL_FROM_API=1 brew irb

@xrisk
Copy link
Author

xrisk commented May 15, 2023

@apainintheneck

It's getting loaded from the opt directory instead

isn’t this a consequence of the fact that my HOMEBREW_PREFIX is /opt/homebrew, which is the default on Apple Silicon I think?

I tried running your commands, the cask example gives


07:36:00 ❯ echo '
           cask = :whatsapp.c; nil
           cask.sourcefile_path
           cask.tap.path
           cask.tap.git?

           ' | HOMEBREW_NO_INSTALL_FROM_API=1 brew irb
==> Interactive Homebrew Shell
Example commands available with: `brew irb --examples`

WARNING: This version of ruby is included in macOS for compatibility with legacy software.
In future versions of macOS the ruby runtime will not be available by
default, and may require you to install an additional package.

Switch to inspect mode.

cask = :whatsapp.c; nil
nil
cask.sourcefile_path
nil
cask.tap.path
#<Pathname:/opt/homebrew/Library/Taps/homebrew/homebrew-cask>
cask.tap.git?
false

And the formula gives:

07:37:45 ❯ echo '
           formula = :p7zip.f
           formula.tap.path
           formula.tap.git?
           ' | HOMEBREW_NO_INSTALL_FROM_API=1 brew irb
==> Interactive Homebrew Shell
Example commands available with: `brew irb --examples`

WARNING: This version of ruby is included in macOS for compatibility with legacy software.
In future versions of macOS the ruby runtime will not be available by
default, and may require you to install an additional package.

Switch to inspect mode.

formula = :p7zip.f
==> Tapping homebrew/core
Cloning into '/opt/homebrew/Library/Taps/homebrew/homebrew-core'…

(it starts cloning the core repo, I didn’t let it finish)

@apainintheneck
Copy link
Contributor

Sorry, that the command I gave you ended up trying to clone the core formula repo. I didn't expect that to happen.


It's unfortunately not that simple. There seem to be three situations that can happen with the bump-*-pr commands.

Before I start here are my taps:

~ $ brew tap
altinity/clickhouse
homebrew/cask-versions
mongodb/brew
  1. The formula/cask is in a local tap.
~ $ brew bump-formula-pr --dry-run --debug clickhouse
/opt/homebrew/Library/Homebrew/brew.rb (Formulary::FormulaLoader): loading /opt/homebrew/Library/Taps/altinity/homebrew-clickhouse/Formula/clickhouse@22.7.rb
...

It gets loaded from **/Library/Taps/**/{package-name}.rb as expected.

  1. The core tap is not installed locally because we are using the API but we can load the formula/cask because we've already installed it.
~ $ brew ls pandoc
/opt/homebrew/Cellar/pandoc/3.1.2/bin/pandoc
/opt/homebrew/Cellar/pandoc/3.1.2/etc/bash_completion.d/pandoc
/opt/homebrew/Cellar/pandoc/3.1.2/share/man/man1/pandoc.1
~ $ brew bump-formula-pr --dry-run --debug pandoc
/opt/homebrew/Library/Homebrew/brew.rb (Formulary::FormulaLoader): loading /opt/homebrew/opt/pandoc/.brew/pandoc.rb
Error: This formula is not in a tap!
...

It loads the cached version of the formula/cask file as best as I can tell. This is cached during installation. It's not in a tap though so the error is appropriate but not that helpful.

  1. The core tap is not installed locally because we are using the API and we haven't installed the formula/cask.
~ [1]$ brew ls luau
Error: No such keg: /opt/homebrew/Cellar/luau
~ [1]$ brew bump-formula-pr --dry-run --debug luau
Error: No available formula with the name "luau".
...

We aren't able to load the formula/cask at all so it dies in the parser.


It's pretty clear that the final two should have more informative errors. Neither mention the need to download a local version of a core tap.

Does it even make sense to load the cached file in the case where we're forcing HOMEBREW_NO_INSTALL_FROM_API and the formula/cask was originally loaded from the API? I assume this logic is here to allow you to interact with packages from taps that no longer exist but it doesn't really fit here.

I wonder if we could somehow handle this inside Homebrew::CLI::NamedArgs and then provide some generic messaging for API users that don't have local versions of the core taps.

@MikeMcQuaid
Copy link
Member

It's pretty clear that the final two should have more informative errors. Neither mention the need to download a local version of a core tap.

Agreed 👍🏻

Does it even make sense to load the cached file in the case where we're forcing HOMEBREW_NO_INSTALL_FROM_API and the formula/cask was originally loaded from the API? I assume this logic is here to allow you to interact with packages from taps that no longer exist but it doesn't really fit here.

I don't think so.

I wonder if we could somehow handle this inside Homebrew::CLI::NamedArgs and then provide some generic messaging for API users that don't have local versions of the core taps.

Sounds ideal and Formulary and raise/odie if not.

@Bo98
Copy link
Member

Bo98 commented Jun 8, 2023

It's pretty clear that the final two should have more informative errors. Neither mention the need to download a local version of a core tap.

Agreed 👍🏻

I wonder if we could somehow handle this inside Homebrew::CLI::NamedArgs and then provide some generic messaging for API users that don't have local versions of the core taps.

Sounds ideal and Formulary and raise/odie if not.

I've worked on this a bit and I've got something locally that can say that a full clone is required. This linked a bit with #15049, where basically I'm moving the HOMEBREW_NO_INSTALL_FROM_API command list from a global shell setting to being scoped around formula loads (mainly CLI parser - e.g. named_args :formula, without_api: true), which works much better in mixed tap scenarios.

I'm not entirely sure on the messaging yet though. We can tell users to brew tap Homebrew/core but there is the "stale tap" problem.

Currently, if you have a local tap, brew update will not update it and it's up to you to git pull manually. So we have the issue where running brew bump-formula-pr, which abstracts git commands away, is paired with a system that expects you to be maintaining it yourself using git pull.

Not sure what the best general solution is. The bump commands are in the auto-update list so special casing is possible there, though that assumes no other command has been run recently in the auto-update interval (1 hour). We could have brew update update taps (if installed) for devcmdrun users.

The question does expand a bit beyond the bump commands. For example, how should brew edit formula behave if run on a 6 month old tap. It probably should at least be giving a warning.

@MikeMcQuaid
Copy link
Member

Currently, if you have a local tap, brew update will not update it and it's up to you to git pull manually. So we have the issue where running brew bump-formula-pr, which abstracts git commands away, is paired with a system that expects you to be maintaining it yourself using git pull.

Some sort of brew update --actuallyupdatealltapsplz (not that name hopefully obviously) would be good to be able to both tell people to run in similar messages and for triggering as part of the brew bump-* auto-update.

It probably should at least be giving a warning.

For something like brew edit: I like the idea of a warning. I'd expect anyone who would use it to be able to cd and run git pull.

@MikeMcQuaid MikeMcQuaid added in progress Maintainers are working on this and removed help wanted We want help addressing this labels Jun 19, 2023
@MikeMcQuaid
Copy link
Member

Fixed by #15563

@github-actions github-actions bot added the outdated PR was locked due to age label Jul 23, 2023
@github-actions github-actions bot locked as resolved and limited conversation to collaborators Jul 23, 2023
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
bug Reproducible Homebrew/brew bug in progress Maintainers are working on this install from api Relates to API installs outdated PR was locked due to age
Projects
None yet
Development

No branches or pull requests

5 participants