Add processing of .pth files from "app_packages".

[timrid]

As described here, the app_packages should be added via site.addsitedir() so that .pth files are executed correctly.

I tested it using a manually created .pth file that executed a .py file with a print. The output of the print was visible in the console with briefcase run windows visualstudio.

I also added pywin32 as a dependency in my application and tested it with the following code:

import win32ui
win32ui.MessageBox("I am working now", "Hello pywin32")

Should fix beeware/briefcase#2195, beeware/briefcase#835, beeware/briefcase#669 and beeware/briefcase#381 for "windows visualstudio".

PR Checklist:

• All new features have been tested
• All new features have been documented
• I have read the CONTRIBUTING.md file
• I will abide by the code of conduct

[freakboy3742]

Thanks for the PR; however, as noted on beeware/briefcase-windows-app-template#63, this isn't the best approach for a fix.

If you're using the default python interpreter, sitecustomize is really the only option you have for controlling the behavior of the interpreter. However, in our case, we're maintaining our own binary, so we don't need to rely on extension points - we can make the modification directly, and ensure that the stub app is started with the site modules that we want.

So - the better approach here is to modify the source of the stub binary to perform the site modifications - importing the site module, getting a handle to the addsitedir method, then invoking that method with a string argument. The patch to app_packages is currently being computed so it can be added to the module_search_paths (i.e., PYTHONPATH/sys.path); that would no longer be required, and the call to site.addsitedir() would be performed after the interpreter is initialized (i.e., inside the try{} block, but before the runpy invocation). The runpy code can be used as a template for how to import a module and invoke a method.

[timrid]

Alright, good point. I modified it as suggested. But now the sys.path order is different. app_packages is now after app and not before. I am not sure if this is good or bad...

Before:

sys.path:
  - C:\[...]\briefcase-helloworld\build\helloworld\windows\visualstudio\x64\Release\python313.zip
  - C:\[...]\briefcase-helloworld\build\helloworld\windows\visualstudio\x64\Release
  - C:\[...]\briefcase-helloworld\build\helloworld\windows\visualstudio\x64\Release\app_packages
  - C:\[...]\briefcase-helloworld\build\helloworld\windows\visualstudio\x64\Release\app 

After:

sys.path:
  - C:\[...]\briefcase-helloworld\build\helloworld\windows\visualstudio\x64\Release\python313.zip
  - C:\[...]\briefcase-helloworld\build\helloworld\windows\visualstudio\x64\Release
  - C:\[...]\briefcase-helloworld\build\helloworld\windows\visualstudio\x64\Release\app
  - C:\[...]\briefcase-helloworld\build\helloworld\windows\visualstudio\x64\Release\app_packages

[freakboy3742]

As a heads up - this is going to be a slightly weird PR to land, because in addition to this change (which, with one modification flagged inline, looks great), we need to:

1. Tag the repo to rebuild the stub binary used by the app template
2. Update the app template to reference that new stub binary
3. Add a change to Briefcase that is only a change note.

1 has to be done by someone on the core team; I can handle (2) as well.

I don't know your appetite for making the analogous change to the iOS, macOS-Xcode, Linux-system and Linux-Flatpak templates; but if you're up for it, we should probably make those changes as well before landing (3). There's also a fix needed for Android, but AFAIK it's not a "copy and paste + minor modifications" of this PR.

[freakboy3742]

Alright, good point. I modified it as suggested. But now the sys.path order is different. app_packages is now after app and not before. I am not sure if this is good or bad...

Good catch! That's definitely a technical backwards incompatibility, but I don't think I'm too concerned about it. The only way it would manifest is if an app is relying on an package installed in app_packages being visible in preference to a application sources included in app - and for the vast majority of users, the only thing in app would be the actual app code.

It might be worth mentioning this in a Briefcase release note as a backwards incompatibility - but I don't think that should stop us from making the change, given that the impact will be minimal, and we get correct .pth file handling as a benefit.

[timrid]

So the changes for Windows-app I will leave to you. But I would like to try to make the analogous for the other platforms. Until the other platforms are ready, I would leave this PR marked as a draft, in case there are still things to be found on the other platforms that are also relevant here.

I think at least iOS and Linux-system are a little bit more than just copy and paste, because of my findings in beeware/briefcase#2173 (comment)  For iOS the handling of sys.stdout will be special and for Linux-system the usage of config.site_import = 0;.

I will try it out, but it can take some time...

I was also wondering where is the best place to add unit tests for .pth handling? Do the tests then also belong in briefcase as soon as you make the change notes there?

[freakboy3742]

So the changes for Windows-app I will leave to you. But I would like to try to make the analogous for the other platforms.

Awesome - I look forward to seeing those patches.

Until the other platforms are ready, I would leave this PR marked as a draft, in case there are still things to be found on the other platforms that are also relevant here.

That makes sense; I'll wait until they're all resolved before merging.

I think at least iOS and Linux-system are a little bit more than just copy and paste, because of my findings in beeware/briefcase#2173 (comment)  For iOS the handling of sys.stdout will be special and for Linux-system the usage of config.site_import = 0;.

I recognise that the iOS issue needs special handling for debugging, but I'm fairly certain that work is independent of site-package/.pth handling. I'd suggest fixing the .pth issue for iOS first, and then we can tackle stdout as a separate issue. The explicit std-nslog handling is going to be removed in the very near future as a result of some changes that have landed upstream in CPython (python/cpython#131172), so any fix for debugging purposes is going to need updated handling anyway.

I will try it out, but it can take some time...

Completely understood. Take whatever time you need; we're here to provide assistance if you need it.

I was also wondering where is the best place to add unit tests for .pth handling? Do the tests then also belong in briefcase as soon as you make the change notes there?

That's a really good question.

My suggestion would be to add a .pth handling test to Python-support-testbed. That project is how we verify that the templates work - it's a simple shakedown project that runs pieces of the Python standard library, plus some third party libraries that have binary components.

If we add to that repository an installable Python project (so, a subproject with it's own pyproject.toml) whose only purpose is to add a .pth file to site-packages when it is installed. That .pth file would trigger some logic that sets an attribute or does something else predictable; and we'd then add a test to the support testbed's test suite that validates that the .pth file has been executed.

If we take that approach, then we'll end up with a .pth test that will work on every platform, and will be part of the compliance check for every new template update.

There's a small "chicken-and-egg" problem though - If you add a .pth test to the Support testbed, that test won't pass until the templates fix .pth handling... we'd need to land the template changes first to fix .pth handling, then add the .pth test - otherwise, the templates won't include the fixes needed to make the .pth test work. I'm happy to tackle that with manual testing, though.

[freakboy3742]

Awesome - works perfectly in my testing. Merging is still on hold until we're ready for everything to land, but as far as this PR is concerned, I think we're good.