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

add ios-cmake toolchain #1137

Merged
merged 17 commits into from
Dec 20, 2020
Merged

add ios-cmake toolchain #1137

merged 17 commits into from
Dec 20, 2020

Conversation

a4z
Copy link
Contributor

@a4z a4z commented Mar 18, 2020

ios-toolchain/3.1.2

  • I've read the guidelines for contributing.
  • I've followed the PEP8 style guides for Python code in the recipes.
  • I've used the latest Conan client version.
  • I've tried at least one configuration locally with the
    conan-center hook activated.

This toolchain file is mature, tested, and has plenty of users
It builds for all iOS platforms, and can (theoretical also for conan) build FAT binaries

This toolchain file is mature, well tested, and has plenty of users
It builds for all iOS platofrms, and can, theoritical also for conan
build FAT binaries
@conan-center-bot
Copy link
Collaborator

Some configurations of 'ios-cmake/3.1.2' failed in build 1 (98c0b0ea02c4574eba899ce20d60aa2f9c31fc7e):

  • Linux x86_64, Release, gcc 5, libstdc++
    • Hooks errors detected:
      • [HOOK - conan-center.py] pre_export(): ERROR: [TEST PACKAGE FOLDER (KB-H024)] There is no 'test_package' for this recipe (https://github.com/conan-io/conan-center-index/wiki/Error-Knowledge-Base#KB-H024)
      • [HOOK - conan-center.py] pre_export(): ERROR: [CONAN CENTER INDEX URL (KB-H027)] The attribute 'url' should point to: https://github.com/conan-io/conan-center-index (https://github.com/conan-io/conan-center-index/wiki/Error-Knowledge-Base#KB-H027)
    • Log download
    • Log error Download
    • Profile
  • Access to all the logs

@a4z a4z closed this Mar 18, 2020
@a4z a4z deleted the ios-cmake branch March 18, 2020 19:14
@a4z a4z restored the ios-cmake branch March 18, 2020 19:15
@a4z
Copy link
Contributor Author

a4z commented Mar 18, 2020

I get much more errors from the conan hooks , but do not know how to solve them,

@a4z a4z reopened this Mar 18, 2020
@conan-center-bot
Copy link
Collaborator

Some configurations of 'ios-cmake/3.1.2' failed in build 2 (98c0b0ea02c4574eba899ce20d60aa2f9c31fc7e):

  • Linux x86_64, Release, gcc 5, libstdc++
    • Hooks errors detected:
      • [HOOK - conan-center.py] pre_export(): ERROR: [TEST PACKAGE FOLDER (KB-H024)] There is no 'test_package' for this recipe (https://github.com/conan-io/conan-center-index/wiki/Error-Knowledge-Base#KB-H024)
      • [HOOK - conan-center.py] pre_export(): ERROR: [CONAN CENTER INDEX URL (KB-H027)] The attribute 'url' should point to: https://github.com/conan-io/conan-center-index (https://github.com/conan-io/conan-center-index/wiki/Error-Knowledge-Base#KB-H027)
    • Log download
    • Log error Download
    • Profile
  • Access to all the logs

@a4z
Copy link
Contributor Author

a4z commented Mar 18, 2020

The reported errors: The url points to this link, and I have no idea what I could put into the test folder

I get much more errors from the hooks , but do not know how to solve all of those,
Ideas welcome.
I really think, to support iOS for packages this toolchain should be part of the center index, so please help me to make it work. Thanks

Copy link
Contributor

@madebr madebr left a comment

Choose a reason for hiding this comment

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

self.settings.os and self.options.ios_target might clash.
Some additional logic in configure() should fix this.

recipes/ios-cmake/all/conanfile.py Show resolved Hide resolved
recipes/ios-cmake/all/conanfile.py Outdated Show resolved Hide resolved
recipes/ios-cmake/all/conanfile.py Outdated Show resolved Hide resolved
recipes/ios-cmake/all/conanfile.py Show resolved Hide resolved
recipes/ios-cmake/all/conanfile.py Outdated Show resolved Hide resolved
recipes/ios-cmake/all/conanfile.py Outdated Show resolved Hide resolved
recipes/ios-cmake/all/conanfile.py Show resolved Hide resolved
recipes/ios-cmake/all/conanfile.py Outdated Show resolved Hide resolved
recipes/ios-cmake/all/conanfile.py Show resolved Hide resolved
recipes/ios-cmake/all/conanfile.py Outdated Show resolved Hide resolved
@madebr
Copy link
Contributor

madebr commented Mar 18, 2020

The reported errors: The url points to this link, and I have no idea what I could put into the test folder

Just create a small test project that only builds when the os is an apple product.

I get much more errors from the hooks , but do not know how to solve all of those,
Ideas welcome.

I think my suggestions will fix some of these. I don't have a mac, so I could not test.

I really think, to support iOS for packages this toolchain should be part of the center index, so please help me to make it work. Thanks

I agree, it lowers the bar for ios developers to use cmake and conan.

@a4z
Copy link
Contributor Author

a4z commented Mar 19, 2020

self.settings.os and self.options.ios_target might clash.
Some additional logic in configure() should fix this.

yes, it is possible to put in combinations that make not sense, like tvOS with an arch that does not exist on TV. t
his will result in useless builds, or give a build error.
so users are expected to know what they do, or stick with defaults from the (todo) documentation
in some future, when everything else for this is done, and FAT binary builds can also be done, candies like such validation can be added

@conan-center-bot
Copy link
Collaborator

Some configurations of 'ios-cmake/3.1.2' failed in build 3 (a21daa64f3ef6b156c4a10174056d8f52aa47577):

  • Linux x86_64, Release, gcc 5, libstdc++
    • Hooks errors detected:
      • [HOOK - conan-center.py] pre_export(): ERROR: [TEST PACKAGE FOLDER (KB-H024)] There is no 'test_package' for this recipe (https://github.com/conan-io/conan-center-index/wiki/Error-Knowledge-Base#KB-H024)
      • [HOOK - conan-center.py] pre_export(): ERROR: [CONAN CENTER INDEX URL (KB-H027)] The attribute 'url' should point to: https://github.com/conan-io/conan-center-index (https://github.com/conan-io/conan-center-index/wiki/Error-Knowledge-Base#KB-H027)
    • Log download
    • Log error Download
    • Profile
  • Access to all the logs

Co-Authored-By: Anonymous Maarten <[email protected]>
@conan-center-bot
Copy link
Collaborator

Some configurations of 'ios-cmake/3.1.2' failed in build 4 (d02ba620f77da3e146d621821a476b6966e77362):

  • Linux x86_64, Release, gcc 5, libstdc++
    • Hooks errors detected:
      • [HOOK - conan-center.py] pre_export(): ERROR: [TEST PACKAGE FOLDER (KB-H024)] There is no 'test_package' for this recipe (https://github.com/conan-io/conan-center-index/wiki/Error-Knowledge-Base#KB-H024)
    • Log download
    • Log error Download
    • Profile
  • Access to all the logs

@SSE4
Copy link
Contributor

SSE4 commented Mar 19, 2020

I wonder if it's still needed, as CMake provides native iOS support?

@a4z
Copy link
Contributor Author

a4z commented Mar 19, 2020

I wonder if it's still needed, as CMake provides native iOS support?

yes, and that is respected by the tool chain file. if you go with a cmake > 3.14

but even if you want to go the native way, you would need an extra toolchain file, or a super complex wrapper adding lots of -D... to cmake
I tried it ;-)
for me using this toolchain, + some fixes from @theodelrieu for cmake to find packages, gave the best (working on all confs I have) results.
And I hope that in some near future the tweaks form @theodelrieu can go upstream in the ios-cmake project, because this would give the best results for all users

@conan-center-bot
Copy link
Collaborator

Some configurations of 'ios-cmake/3.1.2' failed in build 5 (656ff17b5f05683167a3514d9b97e073eded5adf):

  • Linux x86_64, Release, gcc 5, libstdc++
    • Hooks errors detected:
      • [HOOK - conan-center.py] pre_export(): ERROR: [TEST PACKAGE FOLDER (KB-H024)] There is no 'test_package' for this recipe (https://github.com/conan-io/conan-center-index/wiki/Error-Knowledge-Base#KB-H024)
    • Log download
    • Log error Download
    • Profile
  • Access to all the logs

@madebr madebr mentioned this pull request Mar 27, 2020
4 tasks
@prince-chrismc
Copy link
Contributor

The hooks are complaining about the test_pacakge/ not being provided, when you add recipes you need to make sure to provide it. the wiki it not very helpful but you should be able to reuse the cmake open that's in use https://github.com/conan-io/conan-center-index/tree/master/recipes/cmake/3.x.x/test_package

@conan-center-bot
Copy link
Collaborator

Some configurations of 'ios-cmake/3.1.2' failed in build 6 (a50f5341ad89efdbaca84dc34c00ccadf947d8a1):

  • Linux x86_64, Release, gcc 5, libstdc++
    • Hooks errors detected:
      • [HOOK - conan-center.py] post_package(): ERROR: [PACKAGE LICENSE (KB-H012)] No 'licenses' folder found in package: /tmp/c3ipr/pr_1137_6_0_0/.conan/data/ios-cmake/3.1.2/_/_/package/5ab84d6acfe1f23c4fae0ab88f26e3a396351ac9 (https://github.com/conan-io/conan-center-index/wiki/Error-Knowledge-Base#KB-H012)
      • [HOOK - conan-center.py] post_package(): ERROR: [MATCHING CONFIGURATION (KB-H014)] Package for Header Only does not contain artifacts with these extensions: ['h', 'h++', 'hh', 'hxx', 'hpp'] (https://github.com/conan-io/conan-center-index/wiki/Error-Knowledge-Base#KB-H014)
      • [HOOK - conan-center.py] post_package(): ERROR: [MATCHING CONFIGURATION (KB-H014)] Packaged artifacts does not match the settings used: os=Linux, compiler=None (https://github.com/conan-io/conan-center-index/wiki/Error-Knowledge-Base#KB-H014)
    • Log download
    • Log error Download
    • Profile
  • Access to all the logs

@a4z
Copy link
Contributor Author

a4z commented Jun 1, 2020

thanks for the tip with the test package, added one

now my next hook locally failing is

[HOOK - conan-center.py] post_package(): ERROR: [MATCHING CONFIGURATION (KB-H014)] Package for Header Only does not contain artifacts with these extensions: ['h', 'h++', 'hh', 'hxx', 'hpp'] (https://github.com/conan-io/conan-center-index/wiki/Error-Knowledge-Base#KB-H014) 
[HOOK - conan-center.py] post_package(): ERROR: [MATCHING CONFIGURATION (KB-H014)] Packaged artifacts does not match the settings used: os=iOS, compiler=None (https://github.com/conan-io/conan-center-index/wiki/Error-Knowledge-Base#KB-H014) 

how to address these ?

I know it is not exactly a header only package, but it is for whatever iOS config, so what is it?

@SSE4 SSE4 requested a review from danimtb August 23, 2020 19:20
@conan-center-bot
Copy link
Collaborator

All green in build 16 (2ec7215db2aace05fc8cb598e061fd333e210015)! 😊

@danimtb danimtb requested a review from jgsogo August 24, 2020 11:24
@conan-center-bot
Copy link
Collaborator

An unexpected error happened and has been reported. Help is on its way! 🏇

@conan-center-bot
Copy link
Collaborator

All green in build 18 (e7c8b1c016cf0b61a4de299e6e2ce72152ff4695)! 😊

@a4z
Copy link
Contributor Author

a4z commented Sep 4, 2020

@uilianries , can you please check, I think I either changed, or commented on all change request, and I do not know what is open. I think the actual point was solved in the comments, so there should not be any change request open

Comment on lines +84 to +85
# satisfy KB-H014 (header_only recipes require headers)
tools.save(os.path.join(self.package_folder, "include", "dummy_header.h"), "\n")
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
# satisfy KB-H014 (header_only recipes require headers)
tools.save(os.path.join(self.package_folder, "include", "dummy_header.h"), "\n")

You can remove it, this recipe is an exception, so we can add a rule in hooks to skip your package there.

target_version = self.settings.os.version
else:
#hackingtosh ? hu
raise ConanInvalidConfiguration("Building for iOS on a non Mac platform? Please tell me how!")
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
raise ConanInvalidConfiguration("Building for iOS on a non Mac platform? Please tell me how!")
raise ConanInvalidConfiguration("Building for iOS on a non Mac platform? Open an issue.")

Nice comment, just try to point to the correct place.

@a4z
Copy link
Contributor Author

a4z commented Sep 8, 2020

@uilianries , do you think I should stop caring about the old way of cross compiling?
with os_build, os_arch,
I do not test this path anyway anymore since the new way is much better
and this would allow some other simplifications, like pinn os and arch to Macos x86_64 , remove an if branch, ....

@uilianries
Copy link
Member

@uilianries , do you think I should stop caring about the old way of cross compiling?

@a4z Yes, the build context is the new way. As you can see, is much easier solve old limitations.

@a4z
Copy link
Contributor Author

a4z commented Sep 9, 2020

@uilianries , do you think I should stop caring about the old way of cross compiling?

@a4z Yes, the build context is the new way. As you can see, is much easier solve old limitations.

awesome, because this means I can also open a PR for the android ndk :-)

I will make an additional iteration over the recipe and remove the backward 1 profile os_build/os_arch branch , should be possible for me to do this week

@uilianries
Copy link
Member

Great!

@stale
Copy link

stale bot commented Dec 1, 2020

This pull request has been automatically marked as stale because it has not had recent activity. It will be closed if no further activity occurs. Thank you for your contributions.

@stale stale bot added the stale label Dec 1, 2020
@a4z
Copy link
Contributor Author

a4z commented Dec 1, 2020

I am against that !

@stale stale bot removed the stale label Dec 1, 2020
Copy link
Member

@uilianries uilianries left a comment

Choose a reason for hiding this comment

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

LGTM

Copy link
Contributor

@prince-chrismc prince-chrismc left a comment

Choose a reason for hiding this comment

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

prince-chrismc/conan-center-index-pending-review#1

First success story!

🚀

There are some editorials for a follow up PR 👍

@conan-center-bot conan-center-bot merged commit 6490c8c into conan-io:master Dec 20, 2020
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.

6 participants