Skip to content

Conversation

johnzhou721
Copy link

@johnzhou721 johnzhou721 commented Apr 24, 2025

Refs #7 and #5, with #7 being the messed-up version of this patch. This is still draft. Part of beeware/Python-Apple-support#117

@johnzhou721
Copy link
Author

johnzhou721 commented Apr 24, 2025

OK so command I'm testing locally w/ now (the cflags and ldflags don't worry they are my own directories w/ unpacked deps packages

export PATH=./MacCatalyst/Resources/bin:/usr/bin:/bin:/usr/sbin:/sbin:/Library/Apple/usr/bin
./configure --host=arm64-apple-ios14.0-macabi --build=arm64-apple-darwin --enable-framework --with-build-python=/Library/Frameworks/Python.framework/Versions/3.14/bin/python3.14 CFLAGS="-I/Users/johnzhou/cpython-macabi-deps/include" LDFLAGS="-L/Users/johnzhou/cpython-macabi-deps/lib" LIBLZMA_LIBS="-llzma" BZIP2_LIBS="-lbz2" LIBMPDEC_LIBS="-lmpdec" LIBFFI_LIBS="-lffi" --with-openssl=/Users/johnzhou/cpython-macabi-deps/ && make -j16 && make install

This is just for my own benefit.

@johnzhou721
Copy link
Author

A way more involved set of changes is required; Mac Catalyst is versioned.

@johnzhou721
Copy link
Author

Need logic to get the MACOS version for this -- currently we have ios deployment target:

<key>LSMinimumSystemVersion</key>
	<string>10.0</string>

@johnzhou721
Copy link
Author

OK just destroyed the iOS testbed -- Mac Catalyst has a sort of different process to do stuff (not using simulators, diff ways to codesign etc) and I'm not that familiar and will investigate later.

@johnzhou721
Copy link
Author

More stuff also needs to be modified w.r.t. versioned structure (but embedded only) for Mac Catalyst.

@johnzhou721
Copy link
Author

Just pushed 2 nonworking commits on the versioned structure for mac catalyst but I have absolutely no clue right now... so I need to dig in a bit

Also apologize for the large amounts of small comments.

@freakboy3742 freakboy3742 force-pushed the 3.14-patched branch 2 times, most recently from 50c666f to 0dbdbcd Compare April 25, 2025 03:31
@freakboy3742
Copy link
Owner

This branch has now diverged pretty far from the current state of 3.14-patched; I've got all these changes collapsed into a single commit if you'd like me to force-push an update to this branch.

In my experimentation, the build completes, but testing isn't a simple process. The testbed shouldn't be needed, as there's no need to start a simulator... but python.exe doesn't work either, because of RPATH-based framework linking issues.

@johnzhou721
Copy link
Author

johnzhou721 commented Apr 25, 2025 via email

@johnzhou721
Copy link
Author

OK just finished force-pushing -- added some new changes.

@johnzhou721
Copy link
Author

Right now we have

ln -fs "../../../Python" "iOS/Frameworks/arm64-iphoneos-macabi/Python.framework/Versions/3.14/lib/python3.14/config-3.14-arm64-iphoneos-macabi/libpython3.14.a"
ln: iOS/Frameworks/arm64-iphoneos-macabi/Python.framework/Versions/3.14/lib/python3.14/config-3.14-arm64-iphoneos-macabi/libpython3.14.a: No such file or directory
make: *** [frameworkinstallmaclib] Error 1

@johnzhou721
Copy link
Author

OK that is fixed and now this builds!

rpath needs to be fixed definitely -- but I think we should start the review process.

Remarks:

  1. The plist generated as of 2025 now uses the macOS version instead of the iOS version in the minimum deployment target for MacCatalyst, we need to find a mapping of these values and hardcode them -- https://stackoverflow.com/questions/63581114/mac-catalyst-version may be helpful but I'll save the polishing until later -- we need the embedding done first.
  2. When we run Python.exe now in the terminal, what is failing is NOT the rpath (although it is problematic, see later) but some stuff during startup using _ios_support and somehow getting a none somewhere -- also ios platform isn't really accomodated for terminal stuff I guess so this might require changes, but just for testing purposes -- but I'd rather have an ``embedded''/testbed test because then we test how it actually works in an application and whether any of the embedding process is broken.
  3. rpath issue. Right now it referenced the path of the framework but within the build directory, which is problematic. Will investigate later.
  4. Testing -- To [provide a testbed] or not to [provide a testbed], that is the question. My inclination is yes -- see two items abover; however we shall figure out how to put stuff in the bundle etc.

Overall I'm ending my day now but will make this ready for review so you might leave some additional remarks, even though it'll probably take at least 1-2 weeks on my end to get this merged, because I have school mon-fri and (just like you, sorry for not respecting that and the repeated pings) spend time w/ family (aka. mom dad and sibling(s) and those only) on sat and sun, so I might only have 1h at worst per day.

@johnzhou721 johnzhou721 marked this pull request as ready for review April 26, 2025 03:04
@johnzhou721
Copy link
Author

Oh I'm really sorry but forgot to add

5. Platform identitification -- how can we know we're on Catalyst? Any stuff to take care of regarding iOS vs Catalyst?

Also that said I'll be waking up very early tomorrow so like if you have urgent questions you can ask 6:20 pm in Perth, Australia (it's 5:20am here, but I have some stuff at 5:30am which I won't disclose).

@freakboy3742
Copy link
Owner

OK that is fixed and now this builds!

rpath needs to be fixed definitely -- but I think we should start the review process.

So - it's very difficult to review something that doesn't actually do anything yet. I can pick on cosmetic stuff (the indentation in the configure.ac is the biggest thing that stands out); but unless I can start an interpreter or run the test suite... it doesn't work yet. There's nothing to meaningfully review.

This PR doesn't contain a testbed project; and as I said yesterday, when I run python.exe from any location, or with any DYLD_LIBRARY_PATH set, I get RPATH errors... so if you're getting further than that, you'll need to reveal your testing strategy.

@johnzhou721
Copy link
Author

@freakboy3742 Thanks for organizing everything in the latest PR but afraid would have to revert some changes in the configure.ac -- anyway is a separate testbed acceptable? Or would you prefer it to be one? The latter would require some stuff and the glue scripts to be re-written.

@johnzhou721
Copy link
Author

Will bundle a testbed soon but in the meantime a humongous amount of tests are failing on my end.

@freakboy3742
Copy link
Owner

@freakboy3742 Thanks for organizing everything in the latest PR but afraid would have to revert some changes in the configure.ac -- anyway is a separate testbed acceptable? Or would you prefer it to be one? The latter would require some stuff and the glue scripts to be re-written.

I'm not clear why you've reverted the configure.ac changes though. Why does MacCatalyst require a versioned framework? A versioned framework is required on macOS because it's installed in /Library; that won't be the case here.

As for the testbed; I've been considering whether the iOS, tvOS, watchOS, visionOS and MacCatalyst directories need to be merged into a single Apple folder so that they can share a common testbed project and script, with multiple output targets. We don't need to solve that problem right now, but it's something that probably needs to be done sooner rather than later. For right now, I'm OK with a duplicated testbed project.

@freakboy3742
Copy link
Owner

Also - as there have been changes to the visionOS patch, this PR now needs to be updated; when you do, you need to use a rebase, not a merge commit. The "patch tree" approach to managing the 3.14-patched branch works isn't compatible with merge commits.

@johnzhou721
Copy link
Author

I'm not clear why you've reverted the configure.ac changes though. Why does MacCatalyst require a versioned framework? A versioned framework is required on macOS because it's installed in /Library; that won't be the case here.

macOS distribution requires versioned framework AFAIK; see, for example, the Frameworks folder of LyX, a WYSIWYM LaTeX editor, and things like QtCore.framework is versioned:

Screenshot 2025-04-27 at 10 27 28 AM

@johnzhou721
Copy link
Author

@freakboy3742 Done

@johnzhou721
Copy link
Author

johnzhou721 commented Apr 27, 2025

There's no way AFAIK to obtain the macOS version from the Mac Catalyst verison, the former of which is actually requried in our plist, so I provided the correct value for the default version and added --with-catalyst-macos-version to pass in the macOS version.

@johnzhou721
Copy link
Author

I just killed the MacCatalyst testbed and merged with iOS testbed. Let me know what you think -- b/c this is usually what they do when they build actual apps.

Copy link
Owner

@freakboy3742 freakboy3742 left a comment

Choose a reason for hiding this comment

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

I'm not clear why you've reverted the configure.ac changes though. Why does MacCatalyst require a versioned framework? A versioned framework is required on macOS because it's installed in /Library; that won't be the case here.

macOS distribution requires versioned framework AFAIK; see, for example, the Frameworks folder of LyX, a WYSIWYM LaTeX editor, and things like QtCore.framework is versioned:

I'm not arguing that you can't use a versioned framework - I'm asking if we must use a versioned framework.

I just killed the MacCatalyst testbed and merged with iOS testbed. Let me know what you think -- b/c this is usually what they do when they build actual apps.

Some notes inline; keep in mind that one of the primary goals here is that this is a patch on top of cpython, so changes must be kept to a minimium. Even "minor reorganisations" are a problem because they won't merge/rebase cleanly.

There's no way AFAIK to obtain the macOS version from the Mac Catalyst verison, the former of which is actually requried in our plist, so I provided the correct value for the default version and added --with-catalyst-macos-version to pass in the macOS version.

Why is the macOS version needed? macOS version matters when it's a gateway for features... but if you're on Catalyst, the macOS version isn't the gateway being used - it's the iOS version.

@johnzhou721
Copy link
Author

johnzhou721 commented May 16, 2025

Sorry for the late response. Life has been crazy

The multiprocessing thing may be because that stuff assume the current stuff is a Python interpreter so it forks the iOSTestbed program instead of the python program which does nothing... and the forked process isn't debugged or tested. I'm not sure about this though, this is purely speculative.

Edits

EDIT: I might be right. Might be an embedded mode problem.

#include <Python/Python.h>

int main(int argc, char *argv[]) {
    Py_Initialize();

    PyRun_SimpleString(
        "import multiprocessing\n"
        "def worker():\n"
        "    print('Worker running')\n"
        "\n"
        "if __name__ == '__main__':\n"
        "    p = multiprocessing.Process(target=worker)\n"
        "    p.start()\n"
        "    p.join()\n"
    );

    Py_Finalize();
    return 0;
}

will hang and when I tried interrupting, there's an infinite scroll of errors, and when I force it to close, it showed that 1110 ish processes has been created. Yikes!

Since we're currently using embedded mode only, it'd make sense to skip this so we don't open this can of worms.

@freakboy3742 Any problems with my reproducer? Compiling using Python 3.14 framework.

I'll mark multiprocessing as done for now.

EDIT 2 -- I am right, see https://docs.python.org/3/library/multiprocessing.html#multiprocessing.set_executable, by default sys.executable is used, we can't provide a Python interpreter, and sys.executable is actually the embedder. So this is resolved -- no multiprocessing in embed mode.

EDIT 3 -- But if you're allowed to invoke other binaries in Mac Catalyst, we should definitely sort out the thing about rpath and install it into the framework, sign the binary tool, and have the program use that binary. Should this be done? It's sort of complicated.

@johnzhou721
Copy link
Author

My test_os modification might be suspect to https://en.wikipedia.org/wiki/Time-of-check_to_time-of-use but I think it's sufficient here since we're not invoking any more system things that might be doing those stuff.

IMO we don't need to worry about multiprocessing right now since the macOS stuff doesn't have a bundled interpreter anyways.

@johnzhou721
Copy link
Author

mimetypes reproducer: test_httplib test_mimetypes

@johnzhou721
Copy link
Author

test.test_httplib.HTTPSTest.test_local_bad_hostname test.test_mimetypes.CommandLineTest.test_invocation_error reproduces!

@johnzhou721
Copy link
Author

Nevermind... python#132266 seems to be solve this and is in Beta 1, so I'll cherry-pick f5a7037 into here to get a clean testbed run, but this will be unnessacary once we update the patch to Beta 1.

@johnzhou721
Copy link
Author

johnzhou721 commented May 17, 2025

Will rerequest review after the tests pass and I make a few final cleanups.

When I started this I thought it was just going to be some cherry-picking and addressing comments and merging, but this went from 6 to 29 pages for the patch length...

EDIT

Nevermind... another fail... I hate order dependent tests

@johnzhou721
Copy link
Author

Turns out it was caused by me, myself. https://en.wikipedia.org/wiki/Memory_leak#Pseudocode is pretty similar, not sure if this is it, will retest

@johnzhou721
Copy link
Author

Yes!!! For the first time in forever, I got a clean testbed run.

Requesting a review in GitHub again, but the existing things I have not resolved are still pending reply.

Thank you for your extremely nontrivial assistance on this patch.

@johnzhou721 johnzhou721 requested a review from freakboy3742 May 18, 2025 01:17
@freakboy3742
Copy link
Owner

FYI - I likely won't get a chance to look at this for a couple of days; PyCon US sprints for the next couple of days will keep me fairly well occupied.

@johnzhou721
Copy link
Author

johnzhou721 commented Jun 3, 2025

@freakboy3742 Don't believe the force push is correct... will need to drop the tvOS, watchOS + visionOS commits.

EDIT or is this intentional to combine into 1 commit?

EDIT nevermind it's me who need to rebase... misread branch name, sorry

@johnzhou721
Copy link
Author

Done. Really sorry for accientally misreading the branch name in the last comment.

@johnzhou721
Copy link
Author

Aplogies for missing the latest force push.

commit a228887
Author: John Zhou <[email protected]>
Date:   Sat May 17 19:53:30 2025 -0500

    https://en.wikipedia.org/wiki/Memory_leak#Pseudocode

commit 78eb704
Author: John Zhou <[email protected]>
Date:   Thu May 15 22:12:29 2025 -0500

    Revert "try to build multiprocessing"

    This reverts commit 4e92456.

commit 687916e
Author: John Zhou <[email protected]>
Date:   Thu May 15 22:07:48 2025 -0500

    try to build multiprocessing

commit 02be1ef
Author: John Zhou <[email protected]>
Date:   Thu May 15 18:31:47 2025 -0500

    misc fixes

commit 1ff1574
Author: John Zhou <[email protected]>
Date:   Thu May 15 17:14:16 2025 -0500

    why do i forget to handle exceptions???

commit ff71f93
Author: John Zhou <[email protected]>
Date:   Thu May 15 16:54:10 2025 -0500

    grumble

commit 8d3f831
Author: John Zhou <[email protected]>
Date:   Wed May 14 20:23:37 2025 -0500

    grumble

commit 5d73cfc
Author: John Zhou <[email protected]>
Date:   Wed May 14 20:17:51 2025 -0500

    fix test_os

commit 084a83c
Author: John Zhou <[email protected]>
Date:   Sat May 10 21:50:13 2025 -0500

    disable sandbox

commit c8227e4
Author: John Zhou <[email protected]>
Date:   Thu May 8 22:07:42 2025 -0500

    platform fix

commit 00c08c3
Author: John Zhou <[email protected]>
Date:   Thu May 8 21:28:54 2025 -0500

    fix platform on tvOS et. al.

commit 63433a6
Author: John Zhou <[email protected]>
Date:   Mon May 5 20:57:35 2025 -0500

    add a missing file

commit 9930836
Author: John Zhou <[email protected]>
Date:   Sat May 10 07:42:54 2025 -0500

    use python for build

commit 1ee48cd
Author: John Zhou <[email protected]>
Date:   Sat May 3 19:23:52 2025 -0500

    install binaries

commit 19e8d64
Author: John Zhou <[email protected]>
Date:   Sat May 3 14:39:59 2025 -0500

    merge test commands

commit 639bf33
Author: John Zhou <[email protected]>
Date:   Sat May 3 09:02:19 2025 -0500

    elif

commit f1b9686
Author: John Zhou <[email protected]>
Date:   Sat May 3 09:00:09 2025 -0500

    another fix

commit 7253cdd
Author: John Zhou <[email protected]>
Date:   Sat May 3 08:53:54 2025 -0500

    fix test

commit ab9480e
Author: John Zhou <[email protected]>
Date:   Fri May 2 20:54:59 2025 -0500

    grumble

commit 5ec2564
Author: John Zhou <[email protected]>
Date:   Fri May 2 20:50:18 2025 -0500

    nvm that did not work

commit 59e62e6
Author: John Zhou <[email protected]>
Date:   Fri May 2 20:34:26 2025 -0500

    more fixes, also parallel

commit da68fb6
Author: John Zhou <[email protected]>
Date:   Fri May 2 18:49:29 2025 -0500

    remove irrel comment

commit d662098
Author: John <[email protected]>
Date:   Fri May 2 18:44:00 2025 -0500

    Update test_util.py

commit 06e8510
Author: John <[email protected]>
Date:   Fri May 2 18:43:24 2025 -0500

    Update test_loader.py

commit daf3275
Author: John <[email protected]>
Date:   Fri May 2 18:42:41 2025 -0500

    Update __init__.py

commit aaf27df
Author: John <[email protected]>
Date:   Fri May 2 18:41:56 2025 -0500

    Update test_misc.py

commit 1588275
Author: John <[email protected]>
Date:   Fri May 2 18:41:24 2025 -0500

    Update datetimetester.py

commit fc008b8
Author: John Zhou <[email protected]>
Date:   Fri May 2 18:44:37 2025 -0500

    stuff

commit da006cc
Author: John Zhou <[email protected]>
Date:   Fri May 2 18:40:58 2025 -0500

    add flag for fwork

commit ad7a0e1
Author: John Zhou <[email protected]>
Date:   Fri May 2 18:34:53 2025 -0500

    haaaands

commit a29e718
Author: John Zhou <[email protected]>
Date:   Fri May 2 18:26:38 2025 -0500

    yet another error

commit 8b41226
Author: John Zhou <[email protected]>
Date:   Fri May 2 18:08:23 2025 -0500

    outdated message

commit 6c07c00
Author: John Zhou <[email protected]>
Date:   Thu May 1 21:49:34 2025 -0500

    more fixups

commit 897e182
Author: John Zhou <[email protected]>
Date:   Thu May 1 20:58:06 2025 -0500

    another ref

commit 5edde3a
Author: John Zhou <[email protected]>
Date:   Tue Apr 29 20:12:19 2025 -0500

    whitespace

commit 3ece1d6
Author: John Zhou <[email protected]>
Date:   Tue Apr 29 18:11:42 2025 -0500

    I'm not sure if multiprocessing is available...

commit c47a04b
Author: John Zhou <[email protected]>
Date:   Tue Apr 29 17:39:16 2025 -0500

    enable testing stuff

commit fecd3b5
Author: John Zhou <[email protected]>
Date:   Tue Apr 29 17:23:02 2025 -0500

    support fork, fix another appleframeworkloader reference

commit 834b207
Author: John Zhou <[email protected]>
Date:   Tue Apr 29 16:52:59 2025 -0500

    detect mac catalyst in datetimetester

commit 03a40b1
Author: John Zhou <[email protected]>
Date:   Tue Apr 29 07:47:53 2025 -0500

    use sys

commit 849e276
Author: John Zhou <[email protected]>
Date:   Tue Apr 29 07:28:55 2025 -0500

    bootstrap external cleanup`

commit 65e3bf8
Author: John Zhou <[email protected]>
Date:   Tue Apr 29 07:20:31 2025 -0500

    (untested) add mac catalyst detection

commit 8e008c4
Author: John Zhou <[email protected]>
Date:   Mon Apr 28 21:37:37 2025 -0500

    minor adjustments to test script

commit 3d539a4
Author: John Zhou <[email protected]>
Date:   Mon Apr 28 21:30:45 2025 -0500

    disable lib valid + use ad hoc

commit d16af17
Author: John Zhou <[email protected]>
Date:   Sun Apr 27 21:49:08 2025 -0500

    change the regex for catalyst and add if

commit 823d7b6
Author: John Zhou <[email protected]>
Date:   Sun Apr 27 21:43:57 2025 -0500

    add missing entitlemnet

commit 5604df8
Author: John Zhou <[email protected]>
Date:   Sun Apr 27 21:32:21 2025 -0500

    whitespace

commit 5f767fd
Author: John Zhou <[email protected]>
Date:   Tue Jun 3 08:12:12 2025 -0500

    glue for testbed

commit e7b83ae
Author: John Zhou <[email protected]>
Date:   Sun Apr 27 18:11:17 2025 -0500

    fix plist

commit de0ab4d
Author: John Zhou <[email protected]>
Date:   Sun Apr 27 18:09:32 2025 -0500

    address review

commit 87482cd
Author: John <[email protected]>
Date:   Sun Apr 27 14:53:52 2025 -0500

    Delete MacCatalyst/Resources/pyconfig.h

commit 2e971be
Author: John Zhou <[email protected]>
Date:   Sun Apr 27 12:27:10 2025 -0500

    plist adjustments + kill maccatalyst testbed

    revert changes

commit 8d186e3
Author: John Zhou <[email protected]>
Date:   Sun Apr 27 11:40:04 2025 -0500

    esac

commit 1a20cd7
Author: John Zhou <[email protected]>
Date:   Sun Apr 27 11:38:09 2025 -0500

    fixes for plist

commit f00cadf
Author: John Zhou <[email protected]>
Date:   Sun Apr 27 10:47:38 2025 -0500

    git rebase cleanup

commit 956cfbd
Author: John Zhou <[email protected]>
Date:   Sat Apr 26 07:00:24 2025 -0500

    Remove ignored files from tracking

commit 3828ce4
Author: John Zhou <[email protected]>
Date:   Sat Apr 26 07:00:09 2025 -0500

    remove more ios refs

commit bd99ef5
Author: John Zhou <[email protected]>
Date:   Sat Apr 26 06:49:55 2025 -0500

    testbed

commit 2479a35
Author: John Zhou <[email protected]>
Date:   Sat Apr 26 06:11:51 2025 -0500

    here more code

commit b3adac4
Author: John Zhou <[email protected]>
Date:   Sat Apr 26 05:50:09 2025 -0500

    fix

commit ce636cd
Author: John Zhou <[email protected]>
Date:   Sat Apr 26 05:47:45 2025 -0500

    rpath fix w/ embedded

commit e0ec373
Author: John Zhou <[email protected]>
Date:   Sun Apr 27 10:46:32 2025 -0500

    Minor cleanups.

commit 4071f1d
Author: John Zhou <[email protected]>
Date:   Fri Apr 25 20:36:44 2025 -0500

    remove useless stuff.

commit 54856e3
Author: John Zhou <[email protected]>
Date:   Fri Apr 25 18:48:41 2025 -0500

    another fix

commit 97598c4
Author: John Zhou <[email protected]>
Date:   Fri Apr 25 18:43:42 2025 -0500

    another fix

commit f39ed64
Author: John Zhou <[email protected]>
Date:   Fri Apr 25 18:38:12 2025 -0500

    more changes to configure

commit 9de10c0
Author: John Zhou <[email protected]>
Date:   Fri Apr 25 18:30:44 2025 -0500

    Add supp for mac catalyst

    Co-Authored-By: Andrew Savva <[email protected]>
Copy link
Author

@johnzhou721 johnzhou721 left a comment

Choose a reason for hiding this comment

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

FInal pieces of cleanup.

@johnzhou721
Copy link
Author

FYI: I've cleaned up some of the things I flagged and also cleaned up a bunch of comments.

@johnzhou721
Copy link
Author

Hi @freakboy3742 , been digging through my list of old PRs and found this -- could another look be taken at this?

Current state of this PR should be passing the testbed; most changes have been straightforward except for the guarded file descriptor modification on line 2513 of test_os.py -- fstat fails for guarded FDs making Python think it's open to use, so for mac catalyst (not sure if relevant on other apple platforms?) the test keeps opening new FDs (since they're sequential with the exception of some of those system-guarded ones) on a file until they become non-sequential and uses the sequential range opened to test closerange.

Thanks!

@freakboy3742
Copy link
Owner

Apologies for the delay on reviewing this. I've been mostly deferring looking into this (and #11) because I knew that python#138176 was going to be on my todo list, and it's going to be a lot easier to land other Apple changes once that change is in place.

Actually getting to the point of writing that PR took a lot longer than I was hoping...

So - my current intention is to wait until that PR has landed, and then integrate these changes into that new directory (and build/testbed) structure.

@johnzhou721
Copy link
Author

@freakboy3742 No worries, more of a mistake on my part flooding in new PRs when I haven't completely old ones.

I won't have time to integrate testbed into new directory structure though, so if your schedule's also pretty tight we either omit the testbed part or close this PR. Both options are fine with me.

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.

2 participants