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

Merge #18743: depends: Add --sysroot option to mac os native compile flags #4281

Merged
merged 1 commit into from
Jul 19, 2021

Conversation

UdjinM6
Copy link

@UdjinM6 UdjinM6 commented Jul 19, 2021

14647: build: Remove illegal spacing in darwin.mk from #4271 broke depends compilation on my machine. Backporting 18743 fixes the issue.

EDIT: 18743 is also a part of #4255 but it needs rebase anyway atm.

…ompile flags

1e94a2b depends: Add --sysroot option to mac os native compile flags (Russell Yanofsky)

Pull request description:

  Catalina SDK clang stopped automatically searching the SDK include paths when invoked without `--sysroot`:

  - bitcoin#16367 (comment)
  - Homebrew/homebrew-core#45061

  This hasn't been a problem for current native depends packages because are passing their own `--sysroot` values, and hasn't been a problem for current host packages because they use `darwin_` commands instead of `build_darwin_` commands.  But the current `build_darwin_CC` and `build_darwin_CXX` commands are still unnecessarily fragile, and incompatible with new native depends packages added in bitcoin#18677.

  Cory Fields (theuni) suggested in bitcoin#16367 (comment) switching compiler from SDK clang to native clang (from $PATH) to avoid this problem. This is easy and makes a certain amount of sense for building native packages, as opposed to host packages. But Michael (fanquake) pointed out in bitcoin#18677 (comment) that it would be inconsistent to switch to non-SDK compilers while still using other SDK tools like `ranlib` and `install_name_tool`. So simplest, minimal fix seems to be just adding the missing `--sysroot` option.

ACKs for top commit:
  ryanofsky:
    > ACK [1e94a2b](bitcoin@1e94a2b) - I think this change is ok, and I prefer it to the previous patch.
  fanquake:
    ACK 1e94a2b - I think this change is ok, and I prefer it to the previous patch. Thanks for the summary in the PR description. I played around with Xcode and the CLT; I think previously I didn't fully grok the slight differences between the two.

Tree-SHA512: 4d4bbb7f49acb76d934a872a15b4e14f36290b508cb9e728815f959767ec174bcfb6d2ca7dcd995cc550d86980d64d4247ea5ecfca2301f0953006e50744fdb4
@UdjinM6 UdjinM6 added this to the 18 milestone Jul 19, 2021
Copy link
Member

@PastaPastaPasta PastaPastaPasta left a comment

Choose a reason for hiding this comment

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

utACK

@UdjinM6 UdjinM6 merged commit 1f6adef into dashpay:develop Jul 19, 2021
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