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

[depends][darwin] cleanup, fix variable order & build / host mix up #13816

Merged
merged 3 commits into from
Apr 26, 2018

Conversation

Rechi
Copy link
Member

@Rechi Rechi commented Apr 25, 2018

Description

  • remove handling for unsupported Xcode versions
  • reorder use_sdk_path (assign it before using it)
  • -mmacosx-version-min should be in native cflags for all build on osx, no matter what the target platform is
  • same is true for ac_cv_* settings in native config.site

Types of change

  • Bug fix (non-breaking change which fixes an issue)
  • Improvement (non-breaking change which improves existing functionality)
  • New feature (non-breaking change which adds functionality)
  • Breaking change (fix or feature that would cause existing functionality to change)

Checklist:

  • My code follows the Code guidelines of this project
  • My change requires a change to the documentation, either Doxygen or wiki
  • I have updated the documentation accordingly
  • I have read the CONTRIBUTING document
  • I have added tests to cover my change
  • All new and existing tests passed

@Rechi Rechi added Type: Fix non-breaking change which fixes an issue Type: Cleanup non-breaking change which removes non-working or unmaintained functionality Component: Depends v18 Leia labels Apr 25, 2018
@Rechi Rechi added this to the Leia 18.0-alpha2 milestone Apr 25, 2018
@Rechi Rechi requested a review from Memphiz April 25, 2018 16:45
Copy link
Member

@Memphiz Memphiz left a comment

Choose a reason for hiding this comment

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

Looks good in general. Jenkins doesn’t seem to like something about it though (only on phone - can’t check the compile output).
This definitely changes some Behavior for libffi - but I guess those libffi hacks are not needed anymore - same for GMP
Approved - once Jenkins builds it...

@@ -19,10 +19,10 @@ ifeq ($(OS),linux)
endif

ifeq ($(OS),ios)
CONFIGURE_FLAGS=CC_FOR_BUILD=llvm-gcc CPP_FOR_BUILD="llvm-gcc -E" --disable-assembly
CONFIGURE_FLAGS=CC_FOR_BUILD="$(CC_FOR_BUILD)" CPP_FOR_BUILD="$(CC_FOR_BUILD) -E" --disable-assembly

This comment was marked as spam.

This comment was marked as spam.

This comment was marked as spam.

This comment was marked as spam.

This comment was marked as spam.

@Rechi
Copy link
Member Author

Rechi commented Apr 26, 2018

@Memphiz jenkins succeeded now, maybe you want to look at the changes again.

@Memphiz
Copy link
Member

Memphiz commented Apr 26, 2018

Mhh OSX-64 failed?

@Rechi
Copy link
Member Author

Rechi commented Apr 26, 2018

Test suite only (TestEvent.GroupTimedWait), but will do another jenkins run.
Apart from that good to get merged?

Copy link
Member

@Memphiz Memphiz left a comment

Choose a reason for hiding this comment

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

Appart from that - fine by me - thx :)

@hudokkow
Copy link
Member

Nice work! iphoneos11.3_arm64 build was giving me problems before this with default sdk.

Tested this PR on 10.13.4 (x64, x32 and iOS arm64 clean builds). Apart from the usual shenanigans of having to run make a few times to build everything successfully, all seems to work as intended.

Going to do a round of Kodi builds to test the rest. 🤞

Thanks!

@Rechi Rechi merged commit 2f5f0c0 into xbmc:master Apr 26, 2018
@Rechi Rechi deleted the fix/darwin branch April 26, 2018 20:00
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Component: Depends Type: Cleanup non-breaking change which removes non-working or unmaintained functionality Type: Fix non-breaking change which fixes an issue v18 Leia
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants