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

rubocops/resource_requires_dependencies: allow lxml build resource #17032

Merged
merged 1 commit into from Apr 5, 2024

Conversation

cho-m
Copy link
Member

@cho-m cho-m commented Apr 5, 2024

Specifically, a lxml build resource can depend on build-only libxml2 and libxslt; however, these were not detected in previous audit since the dependency would be a hash.

Also change message if Linux-only formula.

These changes are both needed for systemd.

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

This isn't able to handle situation of detecting build-only vs runtime usage of resource. Most likely won't be easily able to do this without extending DSL to annotate this information

However, in the scenario where a build dependency should actually be runtime, our linkage audits should catch it (though won't be a hard failure in case of indirect dependency).

Specifically, a `lxml` build resource can depend on build-only `libxml2`
and `libxslt`; however, these were not detected in previous audit since
the dependency would be a hash.

Also change message if Linux-only formula.

These changes are both needed for `systemd`.

Signed-off-by: Michael Cho <michael@michaelcho.dev>
@p-linnane p-linnane requested a review from issyl0 April 5, 2024 00:42
@MikeMcQuaid
Copy link
Member

Thanks again @cho-m!

Will let @issyl0 review before merging.

Copy link
Member

@issyl0 issyl0 left a comment

Choose a reason for hiding this comment

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

Thank you!

@MikeMcQuaid MikeMcQuaid merged commit ad196e0 into Homebrew:master Apr 5, 2024
25 checks passed
@cho-m cho-m deleted the audit-allow-lxml-build-resource branch April 5, 2024 14:05
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

3 participants