Skip to content

Conversation

@kastiglione
Copy link
Contributor

@kastiglione kastiglione commented Sep 14, 2020

Disable pkg-config on Darwin to prevent compilation issues.

This is an NFC follow up to #32436 and #32437.

The benefits are:

  1. Prevents the pkg-config issues directly, instead of a roundabout way
  2. Centralize two related pieces of build isolation config to a single place

For 1, the original issue was that CMake find_ functions would search pkg-config before the SDK. Homebrew pkg-config hardcodes CommandLineTools, even when Xcode is installed, and this caused problems. See SR-12726.

Disabling pkg-config gives better isolation, and it removes the use of CMAKE_PREFIX_PATH which I see as a bigger hammer than disabling pkg-config.

Before any of these changes, if pkg-config is installed, the effective search order was:

  1. Try CMAKE_PREFIX_PATH (empty)
  2. Try HINTS paths (CommandLineTools via pkg-config)
  3. Try SDK paths

After #32436, the relevant search order became:

  1. Try CMAKE_PREFIX_PATH (SDK)
  2. Try HINTS paths (CommandLineTools via pkg-config)
  3. Try SDK paths

Now with this change, the relevant search order is:

  1. Try CMAKE_PREFIX_PATH (empty)
  2. Try HINTS paths (empty)
  3. Try SDK paths

# For additional isolation, disable pkg-config. Homebrew's pkg-config
# prioritizes CommandLineTools paths, resulting in compile errors.
args.extra_cmake_options += [
'-DCMAKE_IGNORE_PATH=/usr/lib;/usr/local/lib;/lib',
Copy link
Contributor

Choose a reason for hiding this comment

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

Is there a cmake option we can use that just does this without us needing to hack around it?

@compnerd

Copy link
Contributor Author

@kastiglione kastiglione Sep 14, 2020

Choose a reason for hiding this comment

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

I spoke with @gottesmm, he's referring to the line below:

-DPKG_CONFIG_EXECUTABLE=/usr/bin/true

Copy link
Contributor Author

@kastiglione kastiglione Sep 15, 2020

Choose a reason for hiding this comment

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

This was discussed out of band, and the conclusion was that disabling pkg-config via PKG_CONFIG_EXECUTABLE=/usr/bin/false is the most direct & concise way to do this. A larger scoped solution is to use checked-in CMake cache files to explicitly declare SDK dependencies. This could approach could happen in the future.

Copy link
Contributor

@gottesmm gottesmm left a comment

Choose a reason for hiding this comment

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

LGTM

@kastiglione
Copy link
Contributor Author

@swift-ci please smoke test and merge

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.

4 participants