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

gh-113628: Fix test_site test with long stdlib paths #113640

Merged
merged 4 commits into from
Jan 3, 2024

Conversation

itamaro
Copy link
Contributor

@itamaro itamaro commented Jan 2, 2024

As suggested by @terryjreedy on the issue, adapt the number of libpath lines in pth lines based on the length of libpath to avoid exceeding a pth file that exceeds the 32KB limit.

I (arbitrarily) added a skip test if we end up with 2 or fewer repetitions (which is highly unlikely, considering this would mean a stdlib path of over 10k bytes)

@carljm carljm merged commit 5dc79e3 into python:main Jan 3, 2024
34 checks passed
@miss-islington-app
Copy link

Thanks @itamaro for the PR, and @carljm for merging it 🌮🎉.. I'm working now to backport this PR to: 3.11, 3.12.
🐍🍒⛏🤖

miss-islington pushed a commit to miss-islington/cpython that referenced this pull request Jan 3, 2024
…113640)

(cherry picked from commit 5dc79e3)

Co-authored-by: Itamar Oren <itamarost@gmail.com>
@bedevere-app
Copy link

bedevere-app bot commented Jan 3, 2024

GH-113671 is a backport of this pull request to the 3.12 branch.

@bedevere-app bedevere-app bot removed the needs backport to 3.12 bug and security fixes label Jan 3, 2024
miss-islington pushed a commit to miss-islington/cpython that referenced this pull request Jan 3, 2024
…113640)

(cherry picked from commit 5dc79e3)

Co-authored-by: Itamar Oren <itamarost@gmail.com>
@bedevere-app
Copy link

bedevere-app bot commented Jan 3, 2024

GH-113672 is a backport of this pull request to the 3.11 branch.

@bedevere-app bedevere-app bot removed the needs backport to 3.11 only security fixes label Jan 3, 2024
carljm pushed a commit that referenced this pull request Jan 3, 2024
… (#113671)

gh-113628: Fix test_site test with long stdlib paths (GH-113640)
(cherry picked from commit 5dc79e3)

Co-authored-by: Itamar Oren <itamarost@gmail.com>
carljm pushed a commit that referenced this pull request Jan 3, 2024
… (#113672)

gh-113628: Fix test_site test with long stdlib paths (GH-113640)
(cherry picked from commit 5dc79e3)

Co-authored-by: Itamar Oren <itamarost@gmail.com>
facebook-github-bot pushed a commit to facebookincubator/cinder that referenced this pull request Jan 3, 2024
Summary:
upstream issue: python/cpython#113628
upstream PR: python/cpython#113640
upstream commit: python/cpython@5dc79e3

 ---

The `getpath` module [tries to read certain `_pth` files during initialization](https://github.com/python/cpython/blob/b4b2cc101216ae1017898dfbe43c90da2fd0a308/Modules/getpath.py#L462)).

This is tested in `test_site` with generated `_pth` files that [include the stdlib path 200 times](https://github.com/python/cpython/blob/b4b2cc101216ae1017898dfbe43c90da2fd0a308/Lib/test/test_site.py#L669).

The `getpath` module [disallows reading files over 32KB during initialization](https://github.com/python/cpython/blob/b4b2cc101216ae1017898dfbe43c90da2fd0a308/Modules/getpath.c#L375).

If the test suite runs from a very long base path, 200 repetitions of the stdlib path in the `_pth` file would be enough to exceed the 32KB limit.

To demonstrate, artificially increase the number of repetitions to some high number that would exceed 32KB, e.g. this patch:

```
 diff --git a/Lib/test/test_site.py b/Lib/test/test_site.py
 --- a/Lib/test/test_site.py
+++ b/Lib/test/test_site.py
@@ -631,5 +631,5 @@
         pth_lines = [
             'fake-path-name',
-            *[libpath for _ in range(200)],
+            *[libpath for _ in range(5000)],
             '',
             '# comment',
```

and observe the test failing with the following error:

```
python3.12 -m test test_site -v -m '*_pthFileTests.test_underpth_nosite_file'
== CPython 3.12.1+meta (3.12:2305ca5, Dec 07 2023, 21:46:47) [Clang 15.0.7 (mononoke://mononoke.internal.tfbnw.net/fbsource 3e29b2044f484840f
== Linux-5.12.0-0_fbk16_hardened_7661_geb00762ce6d2-x86_64-with-glibc2.34 little-endian
== Python build: release ThinLTO dtrace
== cwd: /tmp/test_python_worker_2504793æ
== CPU count: 72
== encodings: locale=UTF-8 FS=utf-8
== resources: all test resources are disabled, use -u option to unskip tests

Using random seed: 3001644498
0:00:00 load avg: 6.52 Run 1 test sequentially
0:00:00 load avg: 6.52 [1/1] test_site
test_underpth_nosite_file (test.test_site._pthFileTests.test_underpth_nosite_file) ... Exception ignored error evaluating path:
Traceback (most recent call last):
  File "<frozen getpath>", line 463, in <module>
MemoryError: cannot read file larger than 32KB during initialization
Fatal Python error: error evaluating path
Python runtime state: core initialized

Current thread 0x00007fb01f2d0740 (most recent call first):
  <no Python frame>
ERROR

======================================================================
ERROR: test_underpth_nosite_file (test.test_site._pthFileTests.test_underpth_nosite_file)
----------------------------------------------------------------------
Traceback (most recent call last):
  File "/data/users/itamaro/fbsource/buck-out/v2/gen/fbsource/da203790281a65b9/third-party/python/3.12/__install-base__/out/install/lib/python3.12/test/test_site.py", line 647, in test_underpth_nosite_file
    output = subprocess.check_output([exe_file, '-c',
             ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
  File "/data/users/itamaro/fbsource/buck-out/v2/gen/fbsource/da203790281a65b9/third-party/python/3.12/__install-base__/out/install/lib/python3.12/subprocess.py", line 466, in check_output
    return run(*popenargs, stdout=PIPE, timeout=timeout, check=True,
           ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
  File "/data/users/itamaro/fbsource/buck-out/v2/gen/fbsource/da203790281a65b9/third-party/python/3.12/__install-base__/out/install/lib/python3.12/subprocess.py", line 571, in run
    raise CalledProcessError(retcode, process.args,
subprocess.CalledProcessError: Command '['/tmp/tmpsq9c4frs/python3.12', '-c', 'import sys; print("\\n".join(sys.path) if sys.flags.no_site else "")']' returned non-zero exit status 1.

----------------------------------------------------------------------
Ran 1 test in 0.019s

FAILED (errors=1)
test test_site failed
test_site failed (1 error)

== Tests result: FAILURE ==

1 test failed:
    test_site

Total duration: 118 ms
Total tests: run=1 (filtered)
Total test files: run=1/1 (filtered) failed=1
Result: FAILURE
```

See python/cpython#113628 for an upstream issue report.

Reviewed By: carljm

Differential Revision: D52475050

fbshipit-source-id: 3d8d19bc9058d8c78eb65fd4477e0c7e848c7e81
kulikjak pushed a commit to kulikjak/cpython that referenced this pull request Jan 22, 2024
aisk pushed a commit to aisk/cpython that referenced this pull request Feb 11, 2024
@itamaro itamaro deleted the gh-113628-pth-long-path branch April 9, 2024 15:30
Glyphack pushed a commit to Glyphack/cpython that referenced this pull request Sep 2, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
skip news tests Tests in the Lib/test dir
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants