Allow deps auditing to run on Linux via HOMEBREW_SIMULATE_MACOS_ON_LINUX by Bo98 · Pull Request #10767 · Homebrew/brew

Bo98 · 2021-03-03T00:47:12Z

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?

brew audit currently fails on macOS because audit_deps is not run on Linux and all auditing on homebrew-core is done on Linux.

This attempts to prevent that happening again by allowing the audit to run under HOMEBREW_FORCE_HOMEBREW_ON_LINUX.

This PR reverts 9336254 which removed the conditional. This PR also restores a867e78 for uses_for_macos handling, with a fix for it not working due to the :since handling. In addition, I've added handling of on_os blocks.

To ensure this works, I've added a step to the Linux CI.

See: Homebrew/homebrew-core#70826 (comment)

BrewTestBot · 2021-03-03T00:47:28Z

Review period will end on 2021-03-04 at 00:47:12 UTC.

Bo98 · 2021-03-03T04:51:57Z

If I'm not here: pr-publish/automerge Homebrew/homebrew-core#72357 to fix the audit that's currently failing.

MikeMcQuaid · 2021-03-03T13:42:18Z

brew audit currently fails on macOS because audit_deps is not run on Linux and all auditing on homebrew-core is done on Linux.

To be exact: the tap-wide auditing is done only on macOS but individual brew audit of modified formulae is still done on macOS.

Can you remove this auditing from the test everything (macOS) job if it's no longer reliable/needed?

.github/workflows/tests.yml

Bo98 · 2021-03-03T13:46:11Z

individual brew audit of modified formulae is still done on macOS.

Yes, indeed - I made that statement too broad.

audit_deps only reliably works on tap-wide auditing, since formulae not directly modified by the PR can be affected. The Qt6 PR is an example or this.

Can you remove this auditing from the test everything (macOS) job if it's no longer reliable/needed?

It is reliable, but the behaviour on Linux should now match macOS so I suppose I can remove it as unneeded.

Bo98 · 2021-03-03T13:52:39Z

the behaviour on Linux should now match macOS so I suppose I can remove it as unneeded.

Actually, Cask auditing is not included in Linux CI so maybe shouldn't remove this yet.

MikeMcQuaid · 2021-03-03T14:33:23Z

audit_deps only reliably works on tap-wide auditing, since formulae not directly modified by the PR can be affected. The Qt6 PR is an example or this.

Gotcha 👍🏻

Actually, Cask auditing is not included in Linux CI so maybe shouldn't remove this yet.

Could it be limited to just do cask tap auditing in this case?

Bo98 · 2021-03-03T15:44:22Z

I think HOMEBREW_FORCE_HOMEBREW_ON_LINUX is getting too broad. We're using it a bit to emulate macOS logic on Linux, but we're also using it on Linux as a part of migrating to homebrew-core.

We should probably separate this and introduce a HOMEBREW_SIMULATE_MACOS_ON_LINUX or something.

MikeMcQuaid · 2021-03-03T15:47:01Z

We should probably separate this and introduce a HOMEBREW_SIMULATE_MACOS_ON_LINUX or something.

Works for me.

I think HOMEBREW_FORCE_HOMEBREW_ON_LINUX is getting too broad. We're using it a bit to emulate macOS logic on Linux, but we're also using it on Linux as a part of migrating to homebrew-core.

The original intent was pretty much just to use homebrew-core on Linuxbrew.

BrewTestBot · 2021-03-04T03:10:38Z

Review period ended.

MikeMcQuaid

Looks good. I wonder if any other HOMEBREW_FORCE_HOMEBREW_ON_LINUX uses warrant replacing with the new variable?

.github/workflows/tests.yml

Bo98 · 2021-03-04T16:41:15Z

I think the other references are fine as far as I can tell.

MikeMcQuaid

Looking good! One question, non-blocking, feel free to merge without if desired.

MikeMcQuaid · 2021-03-05T11:52:47Z

Library/Homebrew/env_config.rb

Is there any situation where you'd want just HOMEBREW_SIMULATE_MACOS_ON_LINUX and not HOMEBREW_FORCE_HOMEBREW_ON_LINUX? If not, could we make HOMEBREW_SIMULATE_MACOS_ON_LINUX automatically set/imply HOMEBREW_FORCE_HOMEBREW_ON_LINUX?

Is there any situation where you'd want just HOMEBREW_SIMULATE_MACOS_ON_LINUX and not HOMEBREW_FORCE_HOMEBREW_ON_LINUX?

Probably not - or at least not for any of our use cases. In theory, HOMEBREW_SIMULATE_MACOS_ON_LINUX should work on linuxbrew-core and pass brew audit without issue - but that's not something we particularly care about.

make HOMEBREW_SIMULATE_MACOS_ON_LINUX automatically set/imply HOMEBREW_FORCE_HOMEBREW_ON_LINUX?

Yeah that makes sense.

Yeah that makes sense.

👍🏻 cool, let's do/document that then.

This reverts commit 9336254.

…r."" This reverts commit 9ad37dd.

…BREW_ON_LINUX

BrewTestBot added the waiting for feedback Merging is blocked until sufficient time has passed for review label Mar 3, 2021

Bo98 force-pushed the audit_deps-force_homebrew_on_linux branch from b7bb4dc to 156de0c Compare March 3, 2021 01:13

MikeMcQuaid reviewed Mar 3, 2021

View reviewed changes

.github/workflows/tests.yml Outdated Show resolved Hide resolved

Bo98 force-pushed the audit_deps-force_homebrew_on_linux branch from c2229f0 to aafd3aa Compare March 3, 2021 16:14

Bo98 changed the title ~~Allow deps auditing to run on Linux via HOMEBREW_FORCE_HOMEBREW_ON_LINUX~~ Allow deps auditing to run on Linux via HOMEBREW_SIMULATE_MACOS_ON_LINUX Mar 3, 2021

Bo98 force-pushed the audit_deps-force_homebrew_on_linux branch from aafd3aa to 03a56e5 Compare March 3, 2021 18:10

BrewTestBot removed the waiting for feedback Merging is blocked until sufficient time has passed for review label Mar 4, 2021

BrewTestBot approved these changes Mar 4, 2021

View reviewed changes

MikeMcQuaid approved these changes Mar 4, 2021

View reviewed changes

.github/workflows/tests.yml Outdated Show resolved Hide resolved

Bo98 force-pushed the audit_deps-force_homebrew_on_linux branch from 03a56e5 to 4091fdb Compare March 4, 2021 16:40

Bo98 mentioned this pull request Mar 4, 2021

workflows/tests: use HOMEBREW_SIMULATE_MACOS_ON_LINUX Homebrew/homebrew-core#72506

Merged

5 tasks

MikeMcQuaid approved these changes Mar 5, 2021

View reviewed changes

Bo98 added 5 commits March 5, 2021 12:21

Revert "dev-cmd/audit: don't do dependency checks on Linux."

b3e78f3

This reverts commit 9336254.

Revert "Revert "uses_from_macos: fix force_homebrew_on_linux behaviou…

7448a6b

…r."" This reverts commit 9ad37dd.

Add HOMEBREW_SIMULATE_MACOS_ON_LINUX

abb2b26

workflows/tests: test brew audit of homebrew-core on Linux

4879cf1

brew.sh: HOMEBREW_SIMULATE_MACOS_ON_LINUX implies HOMEBREW_FORCE_HOME…

5660a1b

…BREW_ON_LINUX

Bo98 force-pushed the audit_deps-force_homebrew_on_linux branch from c197986 to 5660a1b Compare March 5, 2021 12:22

Bo98 merged commit 344ab02 into Homebrew:master Mar 5, 2021

Bo98 deleted the audit_deps-force_homebrew_on_linux branch March 5, 2021 23:46

BrewTestBot added the outdated PR was locked due to age label Apr 5, 2021

Homebrew locked as resolved and limited conversation to collaborators Apr 5, 2021

Uh oh!

Conversation

Bo98 commented Mar 3, 2021 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

BrewTestBot commented Mar 3, 2021

Uh oh!

Bo98 commented Mar 3, 2021

Uh oh!

MikeMcQuaid commented Mar 3, 2021

Uh oh!

Uh oh!

Bo98 commented Mar 3, 2021

Uh oh!

Bo98 commented Mar 3, 2021 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

MikeMcQuaid commented Mar 3, 2021

Uh oh!

Bo98 commented Mar 3, 2021

Uh oh!

MikeMcQuaid commented Mar 3, 2021

Uh oh!

BrewTestBot commented Mar 4, 2021

Uh oh!

MikeMcQuaid left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Bo98 commented Mar 4, 2021

Uh oh!

MikeMcQuaid left a comment

Choose a reason for hiding this comment

Uh oh!

MikeMcQuaid Mar 5, 2021

Choose a reason for hiding this comment

Uh oh!

Bo98 Mar 5, 2021

Choose a reason for hiding this comment

Uh oh!

MikeMcQuaid Mar 5, 2021

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

Bo98 commented Mar 3, 2021 •

edited

Loading

Bo98 commented Mar 3, 2021 •

edited

Loading