Skip to content

Conversation

carlocab
Copy link
Member

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

Inspired by Homebrew/homebrew-core#235513.

It doesn't quite fix that issue, because libomp does not ship CMake
config files, but this would probably still be useful for keg-only
formulae that do ship CMake config files.

While we're here, let's replace the pkg-config references with the
now-canonical name pkgconf.

Inspired by Homebrew/homebrew-core#235513.

It doesn't quite fix that issue, because `libomp` does not ship CMake
config files, but this would probably still be useful for keg-only
formulae that do ship CMake config files.

While we're here, let's replace the `pkg-config` references with the
now-canonical name `pkgconf`.
Comment on lines -98 to +102
if which("pkg-config", ORIGINAL_PATHS) &&
if which("pkgconf", ORIGINAL_PATHS) &&
((formula.lib/"pkgconfig").directory? || (formula.share/"pkgconfig").directory?)
s << <<~EOS

For pkg-config to find #{formula.name} you may need to set:
For pkgconf to find #{formula.name} you may need to set:
Copy link
Member

Choose a reason for hiding this comment

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

Do we want to keep pkg-config here, and instead rely on pkgconf providing a pkg-config symlink, so that this continues to work for those with the old pkg-config? Don't feel strongly though!

Copy link
Member

Choose a reason for hiding this comment

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

Or I guess we could check for both

Copy link
Member

@MikeMcQuaid MikeMcQuaid left a comment

Choose a reason for hiding this comment

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

Seems fine, thanks. Would be nice to try and slim the number of lines of output as even before CMake it's quite a lot of output.

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.

3 participants