Fix the homebrew module failing because of warnings #8406

Please note that the tests fail in https://github.com/ansible-collections/community.general/blob/main/tests/integration/targets/homebrew/tasks/docker.yml#L27

It's probably best to simply remove the whole block https://github.com/ansible-collections/community.general/blob/main/tests/integration/targets/homebrew/tasks/docker.yml#L15-L27.

CC @kitizz

You're right. That test now doesn't make any sense. And probably the whole force_formula parameter is more or less not needed now because by default brew will install formulae.

I also added the changelog fragment. Forgot to commit it the first time...

changelogs/fragments/8406-fix-homebrew-cask-warning.yaml

felixfontein · 2024-05-22T21:04:20Z

And probably the whole force_formula parameter is more or less not needed now because by default brew will install formulae.

I think it's still good to have it, so you can ensure that it won't fall back to casks if a formula of that name does not exist. Also it's probably a good idea to change the default for that option at some point, so that this module will only install formulas, and homebrew_cask only casks.

plugins/modules/homebrew.py

Instead of checking if there is an error message, which can also be a warning, we now check the return code. This commit fixes ansible-collections#8229 ansible-collections#7044

kustodian · 2024-05-22T21:15:39Z

And probably the whole force_formula parameter is more or less not needed now because by default brew will install formulae.

I think it's still good to have it, so you can ensure that it won't fall back to casks if a formula of that name does not exist. Also it's probably a good idea to change the default for that option at some point, so that this module will only install formulas, and homebrew_cask only casks.

So the general direction is to use this module for installing formulae and the homebrew_cask for installing casks? I kind of liked it that I can use the same module to install everything. If you pass homebrew/cask/<pkg_name> it installs cask, otherwise it's a formula.

felixfontein · 2024-05-24T20:01:43Z

To be honest, I'm not sure. It sounded to me like more folks argumented for that than for using the same module for everything. But I'm not actively monitoring this module and the discussions about it (I don't even use it), so 🤷

felixfontein · 2024-05-24T20:02:53Z

In any case, I plan to merge this latest on Monday so it makes it into 9.0.1 for Ansible's 10.0.0rc1.

kustodian · 2024-05-25T09:43:26Z

Awesome. Thx for being very responsive 👌

…

On Fri, May 24, 2024, 22:03 Felix Fontein ***@***.***> wrote: In any case, I plan to merge this latest on Monday so it makes it into 9.0.1 for Ansible's 10.0.0rc1. — Reply to this email directly, view it on GitHub <#8406 (comment)>, or unsubscribe <https://github.com/notifications/unsubscribe-auth/AAO34LJSVK4VZ24EUEKCOUDZD6MIHAVCNFSM6AAAAABIEK7NTSVHI2DSMVQWIX3LMV43OSLTON2WKQ3PNVWWK3TUHMZDCMZQGI3TQOJYHE> . You are receiving this because you authored the thread.Message ID: ***@***.***>

patchback · 2024-05-26T19:07:12Z

Backport to stable-8: 💔 cherry-picking failed — conflicts found

❌ Failed to cleanly apply 43cb5a0 on top of patchback/backports/stable-8/43cb5a0d54856f1f09026e476a5b8ee03b44cf0b/pr-8406

Backporting merged PR #8406 into main

Ensure you have a local repo clone of your fork. Unless you cloned it
from the upstream, this would be your origin remote.
Make sure you have an upstream repo added as a remote too. In these
instructions you'll refer to it by the name upstream. If you don't
have it, here's how you can add it:
```
$ git remote add upstream https://github.com/ansible-collections/community.general.git
```

Ensure you have the latest copy of upstream and prepare a branch
that will hold the backported code:

$ git fetch upstream
$ git checkout -b patchback/backports/stable-8/43cb5a0d54856f1f09026e476a5b8ee03b44cf0b/pr-8406 upstream/stable-8

Now, cherry-pick PR Fix the homebrew module failing because of warnings #8406 contents into that branch:
```
$ git cherry-pick -x 43cb5a0d54856f1f09026e476a5b8ee03b44cf0b
```
If it'll yell at you with something like fatal: Commit 43cb5a0d54856f1f09026e476a5b8ee03b44cf0b is a merge but no -m option was given., add -m 1 as follows instead:
```
$ git cherry-pick -m1 -x 43cb5a0d54856f1f09026e476a5b8ee03b44cf0b
```
At this point, you'll probably encounter some merge conflicts. You must
resolve them in to preserve the patch from PR Fix the homebrew module failing because of warnings #8406 as close to the
original as possible.

Push this branch to your fork on GitHub:

$ git push origin patchback/backports/stable-8/43cb5a0d54856f1f09026e476a5b8ee03b44cf0b/pr-8406

Create a PR, ensure that the CI is green. If it's not — update it so that
the tests and any other checks pass. This is it!
Now relax and wait for the maintainers to process your pull request
when they have some cycles to do reviews. Don't worry — they'll tell you if
any improvements are necessary when the time comes!

🤖 @patchback
I'm built with octomachinery and
my source is open — https://github.com/sanitizers/patchback-github-app.

patchback · 2024-05-26T19:07:19Z

Backport to stable-9: 💚 backport PR created

✅ Backport PR branch: patchback/backports/stable-9/43cb5a0d54856f1f09026e476a5b8ee03b44cf0b/pr-8406

Backported as #8425

🤖 @patchback
I'm built with octomachinery and
my source is open — https://github.com/sanitizers/patchback-github-app.

Instead of checking if there is an error message, which can also be a warning, we now check the return code. This commit fixes #8229 #7044 Co-authored-by: Strahinja Kustudic <[email protected]> (cherry picked from commit 43cb5a0)

felixfontein · 2024-05-26T19:07:39Z

@kustodian thanks for fixing this!

…g because of warnings (#8425) Fix the homebrew module failing because of warnings (#8406) Instead of checking if there is an error message, which can also be a warning, we now check the return code. This commit fixes #8229 #7044 Co-authored-by: Strahinja Kustudic <[email protected]> (cherry picked from commit 43cb5a0) Co-authored-by: Strahinja Kustudic <[email protected]>

Israphel · 2024-05-27T14:21:51Z

will 9.x include the fix?

felixfontein · 2024-05-27T18:18:51Z

9.0.1 (to be released later today) will include it.

Israphel · 2024-05-27T18:29:03Z

9.0.1 (to be released later today) will include it.

9.6.1 ?

…ons#8406) Instead of checking if there is an error message, which can also be a warning, we now check the return code. This commit fixes ansible-collections#8229 ansible-collections#7044 Co-authored-by: Strahinja Kustudic <[email protected]> (cherry picked from commit 43cb5a0)

felixfontein · 2024-05-27T19:20:15Z

community.general 9.0.1 just got released.

You seem to refer to Ansible version numbers. Ansible 9.x.y includes community.general 8.x.y; the next Ansible 9 release is 9.7.0, which would include the next community.general 8.6.y bugfix release (very likely 8.6.2). Right now this bugfix hasn't been backported to the stable-8 branch (the automatic backporting didn't work: #8406 (comment)). I'll try to manually backport it so it will get included, the conflict likely comes from tests.

Israphel · 2024-05-27T19:27:29Z

community.general 9.0.1 just got released.

You seem to refer to Ansible version numbers. Ansible 9.x.y includes community.general 8.x.y; the next Ansible 9 release is 9.7.0, which would include the next community.general 8.6.y bugfix release (very likely 8.6.2). Right now this bugfix hasn't been backported to the stable-8 branch (the automatic backporting didn't work: #8406 (comment)). I'll try to manually backport it so it will get included, the conflict likely comes from tests.

I see, yes I was talking about ansible itself. So 9.7.0 is the candidate to include the fix, got it.

#8429) Fix the homebrew module failing because of warnings (#8406) Instead of checking if there is an error message, which can also be a warning, we now check the return code. This commit fixes #8229 #7044 Co-authored-by: Strahinja Kustudic <[email protected]> (cherry picked from commit 43cb5a0) Co-authored-by: Strahinja Kustudic <[email protected]>

…ons#8406) Instead of checking if there is an error message, which can also be a warning, we now check the return code. This commit fixes ansible-collections#8229 ansible-collections#7044 Co-authored-by: Strahinja Kustudic <[email protected]>

ansibullbot added bug This issue/PR relates to a bug module module new_contributor Help guide this first time contributor plugins plugin (any type) small_patch Hopefully easy to review labels May 22, 2024

kustodian force-pushed the fix-homebrew-cask-warning branch from 5e0a60c to bc4c8e6 Compare May 22, 2024 20:30

kustodian mentioned this pull request May 22, 2024

homebrew module: task fails on warning message #7044

Closed

1 task

felixfontein added check-before-release PR will be looked at again shortly before release and merged if possible. backport-8 Automatically create a backport for the stable-8 branch backport-9 Automatically create a backport for the stable-9 branch labels May 22, 2024

felixfontein reviewed May 22, 2024

View reviewed changes

plugins/modules/homebrew.py Outdated Show resolved Hide resolved

ansibullbot added has_issue needs_revision This PR fails CI tests or a maintainer has requested a review/revision of the PR labels May 22, 2024

kustodian force-pushed the fix-homebrew-cask-warning branch 3 times, most recently from 85d5d6a to d77ad2e Compare May 22, 2024 21:02

felixfontein reviewed May 22, 2024

View reviewed changes

changelogs/fragments/8406-fix-homebrew-cask-warning.yaml Outdated Show resolved Hide resolved

felixfontein reviewed May 22, 2024

View reviewed changes

plugins/modules/homebrew.py Outdated Show resolved Hide resolved

kustodian force-pushed the fix-homebrew-cask-warning branch from d77ad2e to 2196dbf Compare May 22, 2024 21:07

Fix the homebrew module failing because of warnings

0abb8c6

Instead of checking if there is an error message, which can also be a warning, we now check the return code. This commit fixes ansible-collections#8229 ansible-collections#7044

kustodian force-pushed the fix-homebrew-cask-warning branch from 2196dbf to 0abb8c6 Compare May 22, 2024 21:11

ansibullbot added integration tests/integration tests tests and removed needs_revision This PR fails CI tests or a maintainer has requested a review/revision of the PR small_patch Hopefully easy to review labels May 22, 2024

felixfontein removed the check-before-release PR will be looked at again shortly before release and merged if possible. label May 26, 2024

felixfontein merged commit 43cb5a0 into ansible-collections:main May 26, 2024
132 checks passed

patchback bot mentioned this pull request May 26, 2024

[PR #8406/43cb5a0d backport][stable-9] Fix the homebrew module failing because of warnings #8425

Merged

felixfontein mentioned this pull request May 27, 2024

[stable-8] Fix the homebrew module failing because of warnings (#8406) #8429

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Fix the homebrew module failing because of warnings #8406

Fix the homebrew module failing because of warnings #8406

kustodian commented May 22, 2024 •

edited

Loading

ansibullbot commented May 22, 2024

felixfontein left a comment

felixfontein commented May 22, 2024

kustodian commented May 22, 2024 •

edited

Loading

felixfontein commented May 22, 2024

kustodian commented May 22, 2024

felixfontein commented May 24, 2024

felixfontein commented May 24, 2024

kustodian commented May 25, 2024 via email

patchback bot commented May 26, 2024 •

edited

Loading

patchback bot commented May 26, 2024 •

edited

Loading

felixfontein commented May 26, 2024

Israphel commented May 27, 2024

felixfontein commented May 27, 2024

Israphel commented May 27, 2024

felixfontein commented May 27, 2024

Israphel commented May 27, 2024

Fix the homebrew module failing because of warnings #8406

Fix the homebrew module failing because of warnings #8406

Conversation

kustodian commented May 22, 2024 • edited Loading

SUMMARY

ISSUE TYPE

COMPONENT NAME

ansibullbot commented May 22, 2024

felixfontein left a comment

Choose a reason for hiding this comment

felixfontein commented May 22, 2024

kustodian commented May 22, 2024 • edited Loading

felixfontein commented May 22, 2024

kustodian commented May 22, 2024

felixfontein commented May 24, 2024

felixfontein commented May 24, 2024

kustodian commented May 25, 2024 via email

patchback bot commented May 26, 2024 • edited Loading

Backport to stable-8: 💔 cherry-picking failed — conflicts found

Backporting merged PR #8406 into main

patchback bot commented May 26, 2024 • edited Loading

Backport to stable-9: 💚 backport PR created

felixfontein commented May 26, 2024

Israphel commented May 27, 2024

felixfontein commented May 27, 2024

Israphel commented May 27, 2024

felixfontein commented May 27, 2024

Israphel commented May 27, 2024

kustodian commented May 22, 2024 •

edited

Loading

kustodian commented May 22, 2024 •

edited

Loading

patchback bot commented May 26, 2024 •

edited

Loading

patchback bot commented May 26, 2024 •

edited

Loading