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

cask/audit: filter bad artifacts in rosetta audit #17575

Merged
merged 2 commits into from
Jun 30, 2024

Conversation

krehel
Copy link
Member

@krehel krehel commented Jun 27, 2024

  • 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?

Follow up to #17573 to address the following -

  1. Remove binary artifacts if they are part of an app bundle. This was the same change I made in signing detection to avoid checking shell scripts or other plain files. Visual Studio Code, for example, fails without this change as a shell script artifact is picked up for assessment.

  2. Filter out files like .dylib and folders that shouldn't be in the <app>/Contents/MacOS/folder, but are there anyway. (See wondershare-uniconverter 15.5.13 homebrew-cask#177949 for a failure scenario)

@krehel krehel changed the title cask/audit: reject bad artifacts in rosetta audit cask/audit: filter bad artifacts in rosetta audit Jun 27, 2024
Library/Homebrew/cask/audit.rb Outdated Show resolved Hide resolved
Library/Homebrew/cask/audit.rb Show resolved Hide resolved
@MikeMcQuaid
Copy link
Member

Thanks @krehel!

  1. This was the same change I made in signing detection to avoid checking shell scripts or other plain files

This makes me wonder if there's the potential for this to be extracted into a helper method. Are we using it anywhere else beyond those 2 places?

@krehel
Copy link
Member Author

krehel commented Jun 28, 2024

@MikeMcQuaid just those two places, but there is some refinement here I would like to dig into after this is merged like your mention of extraction to iterate an improved solution.

This is meant to prevent PRs from stacking up on spurious failures.

@SMillerDev
Copy link
Member

Yeah, I was also considering splitting into a generic artifact audit but I didn't quite come up with a format yet.

@MikeMcQuaid MikeMcQuaid merged commit 3948359 into Homebrew:master Jun 30, 2024
25 checks passed
@MikeMcQuaid
Copy link
Member

Thanks again @krehel!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants