You'll have to amend this comment to align it with the new behaviour :)

I may also be worth mentioning that this target is added toINSTALLTARGETS by the configure script.

Continuing to have a separate Makefile rule to do the patch seems like a good idea at first, but unfortunately it doesn't play well with how thelibinstall rule works.libinstall does both the install of the lib files and then runscompileall.py to produce the various byte-compiled__pycache__/*.pyc files in the installedlib directory, including those forurllib/parse.py andtest/test_urlparse.py. As it stands, they will now get patched after thepycs have been generated, thus invalidating them. (As a side note, when those files are first imported after installation and if the executing process has write access to the installed location,importlib may try to recompile them but, for many reasons, we shouldn't count on that!)

I think we can simplify this by just moving the patch step directly into thelibinstall rule after the installs and before the compilealls while testing for APP_STORE_COMPLIANCE_PATCH and avoid adding the changes here and inconfigure to modifyINSTALLTARGETS. But see additional comments below.

🤦 I completely forgot about the PYC compilation. It's obvious when I think about it.

The install location needs to include DESTDIR:

--directory=""$(DESTDIR)$(LIBDEST)"

Also, should include the--batch option topatch, otherwisepatch may pause and ask for input if a file is missing.

While reviewing the Makefile changes, I was reminded of theconfigure--disable-test-modules option. Specifying that causes the Makefile to be generated to not install test directories and files, something which seems like an option that developers of stand-alone apps with an embedded Python might be likely to use. Unfortunately, that causes problems with the current approach of having a single unified default patch file for iOS and macOS builds (i.e.app-store-compliance.patch) since it includes patches for both a non-test and a test file. If a file to be patched is not present,patch fails (somewhat more gracefully if the --batch option is included):

No file to patch.  Skipping...1 out of 1 hunks ignored--saving rejects to 'test/test_urlparse.py.rej'Can't create '/var/folders/88/pzbbx0t17b58mtgk9hp9v4jh0000gn/T/patchrkhI75QBojY', output is in '/var/folders/88/pzbbx0t17b58mtgk9hp9v4jh0000gn/T/patchrkhI75QBojY': No such file or directorypatching file 'urllib/parse.py'make: *** [app-store-compliant] Error 1

So the net effect for this patch is kinda OK, since urlib/parse.py was successfully patched and thetest/test_urlparse.py.rej cannot be created since there is notest directory. Of course, the user can always specify a different (i.e. edited) patch file that doesn't include thetest file patch. As a somewhat ugly hack, we could ignorepatch errors and continue on.

It appears thatpatch --dry-run also fails if the file to be patched does not exist, which means thatmake install will fail for macOS builds if--disable-test-modules is included onconfigure and the default patch file is not overridden with--with-app-store-compliance=.... So even with other changes, it might be best to not try to runpatch --dry-run by default on macOS builds.

I didn't know about--disable-test-modules - good to know.

Ignoring patch errors seems like a bad idea to me - wewant the patch to be noisy if it can't be applied, so that we know the patch needs to be updated.

Maybe we could split the difference -always do a--dry-run on the app sources directory (so we can verify that the patch is up to date), but then onlyactually apply the patch if it is required, ignoring patch errors and missing files. Does that make sense?

I suppose that could work assuming we can trustpatch to not try to modify anything and to have the required options. It's a bit worrying that there are various flavors ofpatch out there including a POSIX standard. Also, the vast majority of builders of Python are not going to be using the patch.

Since the original PR hasn't been in a release yet, there's no need to add a blurb for this change.