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

[build-script] Prioritize SDK paths in CMake find_ functions #32436

Conversation

kastiglione
Copy link
Contributor

@kastiglione kastiglione commented Jun 17, 2020

Fixes a build failure that happens on macOS when both the CommandLineTools and homebrew pkg-config are installed.

Homebrew's pkg-config is setup to hardcode CommandLineTools paths, which causes many issues. As a result, when find_package() consults pkg-config, targets can have paths containing CommandLineTools, which can result in non-obvious build failures. Two example packages are libedit and libxml2.

This fix assumes that, as a default, xcrun --sdk macosx --show-sdk-path should be preferred to pkg-config.

The CMake documentation shows the search order that the find_ functions use (see find_path). This change makes use of CMAKE_PREFIX_PATH as an environment variable to prioritize the SDK sysroot over pkg-config. This is the 3rd search pass, just above the HINTS search pass (which is what's used by packages such as FindLibEdit.cmake).

If needed, to explicitly build with a custom package, the documentation shows that higher priority search passes are available, such as setting -DCMAKE_PREFIX_PATH=....

Setting the environment variable is also composable with a user supplied CMAKE_PREFIX_PATH env variable.

Resolves SR-12726.

@JDevlieghere
Copy link
Contributor

@swift-ci please test

@AndrewSB
Copy link
Contributor

could we merge this please? just spent half an hour debugging the problem that this fixes

@JDevlieghere
Copy link
Contributor

@kastiglione any objections to merging this?

@kastiglione
Copy link
Contributor Author

No objections, thanks for reviewing and merging.

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