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

Backports from v6.x to v5.x #7888

Merged
merged 12 commits into from
Aug 26, 2023
Merged

Backports from v6.x to v5.x #7888

merged 12 commits into from
Aug 26, 2023

Conversation

bwoodsend
Copy link
Member

  • rthooks: win32com: fully isolate the genpy cache
  • rthooks: secure temp directories used by matplotlib and win32com rthooks
  • archive: splash: normalize path component separator in splash requirements
  • bootstrap: add a work-around for sys._stdlib_dir not being set
  • hookutils: gi: fix error handling in compile_glib_schema_files
  • Fix msys2 dll paths in GdkPixbuf loaders.cache gen (Fix msys2 dll paths in GdkPixbuf loaders.cache gen #7842)
  • Fix pkgutil.iter_modules for symbolically linked packages
  • Fix readthedocs building theme selection. [skip ci]

Running through CI/CD to flush out the Python 3.7 incompatibilities.

rokm and others added 9 commits August 25, 2023 23:49
Instead of extending the `win32.com.gen_py` sub-module search paths
(the `__path__` attribute) with the new location of the isolated
cache, override it completely. This prevents the global cache from
being accessed, which might result in errors when the global cache
contains some, but not all, required modules.
The run-time hooks that relocate the package's configuration/cache
directory into isolated temporary directory create this directory using
the `tempfile.mkdtemp` function. According to its documentation, the
function creates the temporary directory "in the most secure manner
possible", and the created directory should be "readable, writable, and
searchable only by the creating user ID".

However, this does not apply to Windows, where the 0o700 POSIX
permissions mask passed to the underyling `os.mkdir` call has no effect.
Consequently, the access to the created temporary directory is in fact
gated only by the access to the parent directory. So as long as `TEMP`
and `TMP` point to `%LOCALAPPDATA%\Temp`, the created temporary
directories are typically inaccessible to other users, who do not have
access to the user's home directory. On the other hand, if the temporary
directory base is relocated to a system-wide location (e.g., `c:\temp`),
the temporary directories created by the run-time hooks might become
accessible to other users as well. A malicious user with local access
might thus modify the contents of the temporary directory, interfering
with the application. If the application is running in privileged mode
and developer mode is enabled on the system, they might also attempt
a symlink attack due to lack of hardened mode for `shutil.rmtree`
(used for clean up) on Windows.

Therefore, we replace the use of `tempfile.mkdtemp` with custom function
that uses original `mkdtemp` on POSIX and provides a Windows-specific
implementation that secures the access to created directory via security
descriptor passed to the `CreateDirectoryW` call. This is a
`ctypes`-based port of the code that we already have in bootloader for
mitigating the same issue with temporary directory in onefile builds.

In order to share the implementation among the two run-time hooks that
require it, the code is provided by a new `_pyi_rth_utils` PyInstaller
"fake" package, which is bundled with the frozen application on demand
(i.e., if it is referenced in any of collected run-time hooks).
…ments

Explicitly normalize the names of splash requirements, and ensure
that on Windows, back-slash is always used as the path component
separator.

Fixes splash screen not being able to locate collected Tk resources
in onefile applications created in MSYS2 python environments; splash
requirements were using forward slash, and thus fail to match the
actual PKG entries, which use the back-slash.
Starting with python 3.11, stdlib modules used during python
bootstrap are frozen. Such modules have __file__ attribute set
only if sys._stdlib_dir is set when python's built-in FrozenImporter
is installed.

Unfortunately, python's config API does not allow us to explicitly
set the stdlib_dir value; the interpreter initialization seems to
always try to re-compute the value, and due to "non-standard" path
layout, it ends up being None.

Therefore, we set sys._stdlib_dir to sys._MEIPASS at the end
of `pyimod02_importers.install()`. However, this does not affect
the frozen stdlib modules that are already imported (which is most
of them). So we also need to retroactively fix their __file__
attribute, similarly to what python's built-in FrozenImporter was
supposed to do.
Fix error handling in compile_glib_schema_files; do not attempt
to access attributes of `p`, which is unavailable in the exception
handling block. To display stdout/stderr of the failed process,
use attributes of the `subprocess.CalledProcessError` instead.

Ignore the character encoding errors when decoding stdout/stderr
from the `glib-compile-schema` process. The warnings it emits
seem to use `U+201C` and `U+201D` quotation marks, which might
cause issues with active locale (for example, in msys2/mingw64
environment).
When a package is symbolically linked out of the _MEIPASS directory
(for example, in macOS .app bundles when package contains data files),
there is a path mismatch causing it to get skipped.

This resolves the real path of the package paths and the module paths
so that they can be compared correctly. It changes the comparison to
run in path space rather than module dotted space. I believe this is
equivalent because all modules in the PyiFrozenImporter should be in
the _MEIPASS directory.
It appears that the html_theme variable is now mandatory. Replace the
two different theme setting approaches and their surrounding *if on
readthedocs* logic with something that works everywhere.
sys.audit doesn't exist until Python 3.8.
lv-develer and others added 3 commits August 26, 2023 12:18
Commercial PyQt wheels install with the "_commercial" suffix as their
package name but still make the regular PyQt5/PyQt6 namespace packages
available.

This leads the layout detection logic to assume the old layout because
is_module_satisfies() enters a fallback code path where it looks for the
__version__ attribute inside the namespace package, which doesn't exist.

A possible solution is to look for package names with the "_commercial"
suffix when the standard lookup fails. The _use_new_layout utility
method was introduced to perform such checks and is then used in both
PyQt5 and PyQt6 code paths.
While packaging an application that uses pynvml on Linux, the library 
libnvidia-ml.so.1 may be bundled into the resulting application. This 
library is part of the Nvidia driver stack and this behavior creates 
an incompatibility if the packaging system and runtime system do not 
have the same driver version (or are compatible in some vague sense I 
don't understand).

For my application and I assume pretty much every application that 
might want to bundle pynvml this way, libnvidia-ml.so.1 should be 
excluded from the final distribution.
Have bootloader call `Py_GetPath` before `Py_SetPath` on all
platforms (instead of just on Windows) to work around
memory-initialization issues in python 3.8 and 3.9, which come to
light with `PYTHONMALLOC=debug` or `PYTHONDEVMODE=1` being set in
the environment.
@rokm
Copy link
Member

rokm commented Aug 26, 2023

Hmm, looks like python changed from 3.11.4 to 3.11.5 in alpine container between yesterday's and today's run... and looks like we have some breakage there (probably in general).

@bwoodsend
Copy link
Member Author

Looks like this change added an is_fork_ctx attribute but forgot to add handling for it in the pickle methods (__getstate__() and __setstate__()) so after that class has been pickled and unpickled, the is_fork_ctx gets lost?

@rokm
Copy link
Member

rokm commented Aug 26, 2023

Looks like this change added an is_fork_ctx attribute but forgot to add handling for it in the pickle methods (__getstate__() and __setstate__()) so after that class has been pickled and unpickled, the is_fork_ctx gets lost?

Indeed. Running pyinstaller/tests/functional/scripts/pyi_multiprocessing_nested_process.py unfrozen with spawn or forkserver hits the same error.

I guess there is not much point in us trying to work around it, because python will need to fix this either way. Shall we xfail/skip the nested multiprocessing test for 3.11.5?

@bwoodsend
Copy link
Member Author

I guess there is not much point in us trying to work around it, because python will need to fix this either way. Shall we xfail/skip the nested multiprocessing test for 3.11.5?

I guess we should xfail it in develop. I'm not sure there's much point in cherry picking it into v5. I'm hoping that we'll never need this branch again.

@rokm
Copy link
Member

rokm commented Aug 26, 2023

I guess there is not much point in us trying to work around it, because python will need to fix this either way. Shall we xfail/skip the nested multiprocessing test for 3.11.5?

I guess we should xfail it in develop. I'm not sure there's much point in cherry picking it into v5. I'm hoping that we'll never need this branch again.

I agree.

@bwoodsend bwoodsend marked this pull request as ready for review August 26, 2023 14:37
@bwoodsend
Copy link
Member Author

I've ran the alpine tests locally and there aren't any more surprises hiding behind that abort after 3 failures threshold.

@bwoodsend bwoodsend merged commit d8d3062 into pyinstaller:v5 Aug 26, 2023
17 of 19 checks passed
@bwoodsend bwoodsend deleted the v5 branch August 26, 2023 14:44
@bwoodsend
Copy link
Member Author

Are you submitting a fix for CPython/multiprocessing or shall I grab it?

@rokm
Copy link
Member

rokm commented Aug 26, 2023

Are you submitting a fix for CPython/multiprocessing or shall I grab it?

Go ahead if you want.

@github-actions github-actions bot locked as resolved and limited conversation to collaborators Oct 26, 2023
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

6 participants