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

[python3] cross compile python3 for android on linux #33078

Closed

Conversation

Arlen-LT
Copy link

@Arlen-LT Arlen-LT commented Aug 9, 2023

Fixed #30501

  • Changes comply with the maintainer guide
  • SHA512s are updated for each updated download
  • The "supports" clause reflects platforms that may be fixed by this new version
  • Any fixed CI baseline entries are removed from that file.
  • Any patches that are no longer applied are deleted from the port's directory.
  • The version database is fixed by rerunning ./vcpkg x-add-version --all and committing the result.
  • Only one version is added to each modified port's versions file.

Description

This PR primarily addresses the cross-compilation-for-android issue.

Changes Made

  • Set appropriate compilation options for cross-compilation in the python3/portfile.cmake.

Validation

  • This change will not affect existing builds for other platforms

@Arlen-LT
Copy link
Author

Arlen-LT commented Aug 9, 2023

@microsoft-github-policy-service agree

@Arlen-LT Arlen-LT marked this pull request as ready for review August 9, 2023 13:51
@Arlen-LT Arlen-LT changed the title cross compile python3 for android on linux [python3] cross compile python3 for android on linux Aug 9, 2023
Copy link
Contributor

@dg0yt dg0yt left a comment

Choose a reason for hiding this comment

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

FTR #31621 is not dead. The work was put on hold by the pending update to python 3.11.

@dg0yt
Copy link
Contributor

dg0yt commented Aug 9, 2023

FTR there are also users who build for Android on Windows or macOS.

@MonicaLiu0311
Copy link
Contributor

At present, the version information of your commit is inconsistent with your local one:

##[error]Found the following errors:
##[error]versions/p-/python3.json(2,): error : while parsing versions for python3 from version/p-/python3.json
Version 3.11.4#1 was not found in versions file for python3.

--overwrite-version
After the first modification, please run ./vcpkg x-add-version portName to generate the corresponding json; for subsequent modifications of the same version, please use ./vcpkg x-add-version portName --overwrite-version to update git-tree.

@MonicaLiu0311
Copy link
Contributor

Note: I will be converting your PR to draft status. When you respond, please revert to "ready for review".

@MonicaLiu0311 MonicaLiu0311 added the category:port-bug The issue is with a library, which is something the port should already support label Aug 10, 2023
@MonicaLiu0311 MonicaLiu0311 marked this pull request as draft August 10, 2023 02:31
@Arlen-LT Arlen-LT marked this pull request as ready for review August 10, 2023 06:16
Copy link
Contributor

@dg0yt dg0yt left a comment

Choose a reason for hiding this comment

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

As commented before, this PR doesn't properly deal with triplets and toolchains.
Getting it right will throw you on the changes which I already have in #31621. (And in particular in #31228 - vcpkg maintainers, are you listening?)

Given that the python changes are expensive in vcpkg CI, I would really recommend alignment of these activities.

What I still might need to integrate is the INSTSONAME and ctypes changes. If these changes are needed, they should also be upstreamed.

@Arlen-LT Arlen-LT marked this pull request as draft August 10, 2023 07:46
@Arlen-LT Arlen-LT requested a review from dg0yt August 10, 2023 09:59
@Arlen-LT
Copy link
Author

As commented before, this PR doesn't properly deal with triplets and toolchains. Getting it right will throw you on the changes which I already have in #31621. (And in particular in #31228 - vcpkg maintainers, are you listening?)

Given that the python changes are expensive in vcpkg CI, I would really recommend alignment of these activities.

What I still might need to integrate is the INSTSONAME and ctypes changes. If these changes are needed, they should also be upstreamed.

Which you mentioned are all resolved, please review again. I have seen #31621, but I don't think they are conflict. I'm attemping to upstream the patches.

@Arlen-LT Arlen-LT marked this pull request as ready for review August 10, 2023 10:28
@Arlen-LT
Copy link
Author

Arlen-LT commented Aug 11, 2023

@MonicaLiu0311 @dg0yt Please approve this pull request

@Arlen-LT
Copy link
Author

@LilyWangLL Could anybody merge this pr?

@dg0yt
Copy link
Contributor

dg0yt commented Aug 14, 2023

My position is unchanged:

  • There is no need to limit cross builds to linux host.
  • It is the wrong direction to handle compiler setup to specific portfiles when it should/could be fixed in vcpkg_configure_make.

But you will need wait for the vcpkg owners to decide.

MonicaLiu0311
MonicaLiu0311 previously approved these changes Aug 14, 2023
@MonicaLiu0311 MonicaLiu0311 added the info:reviewed Pull Request changes follow basic guidelines label Aug 14, 2023
@MonicaLiu0311
Copy link
Contributor

Mark info:reviewed for further review.

@Arlen-LT
Copy link
Author

My position is unchanged:

  • There is no need to limit cross builds to linux host.
  • It is the wrong direction to handle compiler setup to specific portfiles when the it should/could be fixed in vcpkg_configure_make.

But you will need wait for the vcpkg owners to decide.

I know your PR #31621 addresses the nature of cross-compile, however this PR is more inclined to aim at creating a python interpreter and binary artifact that meets runtime expectations on Android.
In addition, I found that if you only use the default clang without the architecture of the target android system and the Android API version, compiling Python_Module fails due to finding the header files on the host.
So, you can modify the cross-compiling settings for Mac or windows, but using Android NDK for building python3 is not just about cross-compiling problem. There is two fact:

  1. Building python3 for Android never succeeds with current port, even without the commit updating from 3.10.7. At least on linux, this now can be addressed by this PR.
  2. This PR won't affect any existing behavior that has been successfully built.

@dg0yt
Copy link
Contributor

dg0yt commented Aug 14, 2023

In addition, I found that if you only use the default clang without the architecture of the target android system and the Android API version, compiling Python_Module fails due to finding the header files on the host.

And consistently passing on the flags is the key contribution from #31621. It is not just a python3 or Android problem. Any workaround is technical debt.

And not supporting osx hosts immediately calls for the next PR. With a single line of change.

@MonicaLiu0311 MonicaLiu0311 removed the info:reviewed Pull Request changes follow basic guidelines label Aug 15, 2023
@MonicaLiu0311
Copy link
Contributor

Please resolve the conflict.

@MonicaLiu0311
Copy link
Contributor

Ping @Arlen-LT

@Arlen-LT Arlen-LT closed this Sep 4, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
category:port-bug The issue is with a library, which is something the port should already support
Projects
None yet
Development

Successfully merging this pull request may close these issues.

[python3] Build error (arm64-android)
3 participants