-
Notifications
You must be signed in to change notification settings - Fork 19
The Windows filesystem and process surface: PEP 529 paths, OSError.winerror, the noop os calls, and subprocess #1081
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
Changes from all commits
be972f0
ed6ccdc
f4d555e
7f779ed
a8a2e18
6a6b87a
baea4ba
4868669
2a86b7f
7115b92
a911164
c8ff3fd
d7ced19
e5242e6
df95fd1
0135625
ede7a44
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -7,7 +7,8 @@ | |
| `acceptable_as_base_class = False` (:487); `:172-180` does the same for | ||
| `W_ScandirIterator.typedef`. `:463-465 descr_reduce_ex` refuses to pickle an | ||
| entry, spelling the type with `%T` (`error.py:592-593` -> the qualified typedef | ||
| name `'posix.DirEntry'`, `:469`). | ||
| name `'posix.DirEntry'`, `:469`). The module holding these types is the one | ||
| `os` is implemented by, which Windows spells `nt`. | ||
|
|
||
| Pickle protocols 0 and 1 are deliberately NOT asserted: they never reach | ||
| `reduce_newobj`, they land in `copyreg._reduce_ex`, and that leg already | ||
|
|
@@ -17,46 +18,50 @@ | |
| import copy | ||
| import os | ||
| import pickle | ||
| import sys | ||
|
|
||
| HERE = os.path.dirname(os.path.abspath(__file__)) | ||
| MODULE = "nt" if sys.platform == "win32" else "posix" | ||
|
|
||
| assert os.DirEntry.__module__ == "posix", os.DirEntry.__module__ | ||
| assert os.DirEntry.__module__ == MODULE, os.DirEntry.__module__ | ||
| assert os.DirEntry.__name__ == "DirEntry", os.DirEntry.__name__ | ||
| assert os.DirEntry.__qualname__ == "DirEntry", os.DirEntry.__qualname__ | ||
| assert repr(os.DirEntry) == "<class 'posix.DirEntry'>", repr(os.DirEntry) | ||
| assert repr(os.DirEntry) == "<class '%s.DirEntry'>" % MODULE, repr(os.DirEntry) | ||
|
|
||
| try: | ||
| os.DirEntry() | ||
| except TypeError as exc: | ||
| assert str(exc) == "cannot create 'posix.DirEntry' instances", str(exc) | ||
| assert str(exc) == "cannot create '%s.DirEntry' instances" % MODULE, str(exc) | ||
| else: | ||
| raise AssertionError("os.DirEntry() must not construct an entry") | ||
|
|
||
| try: | ||
| class _Sub(os.DirEntry): | ||
| pass | ||
| except TypeError as exc: | ||
| assert str(exc) == "type 'posix.DirEntry' is not an acceptable base type", str(exc) | ||
| expected = "type '%s.DirEntry' is not an acceptable base type" % MODULE | ||
| assert str(exc) == expected, str(exc) | ||
| else: | ||
| raise AssertionError("os.DirEntry must not be an acceptable base type") | ||
|
|
||
| it = os.scandir(HERE) | ||
| scandir_iterator = type(it) | ||
| assert scandir_iterator.__module__ == "posix", scandir_iterator.__module__ | ||
| assert scandir_iterator.__module__ == MODULE, scandir_iterator.__module__ | ||
| assert scandir_iterator.__name__ == "ScandirIterator", scandir_iterator.__name__ | ||
|
|
||
| try: | ||
| scandir_iterator() | ||
| except TypeError as exc: | ||
| assert str(exc) == "cannot create 'posix.ScandirIterator' instances", str(exc) | ||
| assert str(exc) == "cannot create '%s.ScandirIterator' instances" % MODULE, str(exc) | ||
| else: | ||
| raise AssertionError("ScandirIterator() must not construct an iterator") | ||
|
|
||
| try: | ||
| class _SubIter(scandir_iterator): | ||
| pass | ||
| except TypeError as exc: | ||
| assert str(exc) == "type 'posix.ScandirIterator' is not an acceptable base type", str(exc) | ||
| expected = "type '%s.ScandirIterator' is not an acceptable base type" % MODULE | ||
| assert str(exc) == expected, str(exc) | ||
|
Comment on lines
+21
to
+64
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win 🧩 Analysis chain🏁 Script executed: #!/bin/bash
set -euo pipefail
f=pyre/pyre-interpreter/src/module/posix/interp_posix.rs
rg -n 'make_builtin_type\("(posix|nt)\.' "$f"
# Check whether any layer rewrites a builtin type's module name for Windows.
rg -n -C4 'cfg!\(windows\)|target_os = "windows"' pyre/pyre-interpreter/src/typedef.rs | head -50Repository: youknowone/pyre Length of output: 761 🏁 Script executed: #!/bin/bash
set -euo pipefail
# Inspect the relevant typedef constructors and nearby Windows/platform-aware examples.
f=pyre/pyre-interpreter/src/module/posix/interp_posix.rs
sed -n '2920,3085p' "$f" | cat -n
# Check the test behavior expectations and module detection.
g=pyre/extra_tests/parity_tests/dir_entry_uninstantiable_python314.py
sed -n '1,90p' "$g" | cat -n
# Find all references to os.DirEntry / ScandirIterator in extra_tests and module definitions.
rg -n "DirEntry|ScandirIterator|make_builtin_type\\(\"posix\\.|make_builtin_type\\(\"nt\\." pyre/extra_tests pyre/pyre-interpreter/src | head -200Repository: youknowone/pyre Length of output: 19799 🏁 Script executed: #!/bin/bash
set -euo pipefail
# Find any layer that chooses nt vs posix module prefixes for os types.
rg -n -C6 'os_module|os_scandir|scandir|nt|make_builtin_type\("nt|make_builtin_type\("posix|target_os = "windows|cfg!\(windows\)' pyre/pyre-interpreter/src/module | head -300
# Find os module files and their scandir-related type definitions.
fd -e rs -x sh -c 'echo "--- $1"; rg -n -C5 "scandir|DirEntry|ScandirIterator|os_module|module_ns_store|make_builtin_type" "$1"' sh pyre/pyre-interpreter/src/moduleRepository: youknowone/pyre Length of output: 50372 🏁 Script executed: #!/bin/bash
set -euo pipefail
# Read focused os module files and the typedef name handling implementation.
fd -e rs '^(os|nt|typedef)\.rs$' pyre/pyre-interpreter/src/module pyre
for f in $(fd -e rs '^(os|nt)\.rs$' pyre/pyre-interpreter/src/module); do
echo "--- $f"
wc -l "$f"
rg -n -C5 'DirEntry|ScandirIterator|scandir|os_module|nt|module_ns_store|register_module|target_os = "windows|cfg!\(windows\)' "$f" || true
done
echo "--- typedef.rs relevant lines"
rg -n -C6 'fn make_builtin_type|__module__|w_type_set_name|type_get_name|PySys_SetConfig|module_path' pyre/pyre-interpreter/src/typedef.rsRepository: youknowone/pyre Length of output: 50373 🏁 Script executed: #!/bin/bash
set -euo pipefail
# List exact os/nt-related files.
git ls-files pyre/pyre-interpreter/src/module | grep -E '(^|/)(os|nt)\.rs$|/module/(os|nt)/' || true
# Search focused file list for relevant registry code.
while IFS= read -r f; do
echo "--- $f"
wc -l "$f"
rg -n -C6 'DirEntry|ScandirIterator|scandir|os_module|nt|register_module|module_ns_store|make_builtin_type|target_os = "windows|cfg!\(windows"' "$f" || true
done < <(git ls-files pyre/pyre-interpreter/src/module | grep -E '(^|/)(os|nt)\.rs$|/module/(os|nt)/')
# Read typedef name handling around the module derivation.
f=pyre/pyre-interpreter/src/typedef.rs
sed -n '2268,2293p' "$f" | cat -n
sed -n '10888,10931p' "$f" | cat -nRepository: youknowone/pyre Length of output: 206 🏁 Script executed: #!/bin/bash
set -euo pipefail
echo "--- module files"
git ls-files pyre/pyre-interpreter/src/module | grep -E '(^|/)(os|nt)\.rs$|module/(os|nt)/' || true
echo "--- focused search for scandir/os types"
for f in pyre/pyre-interpreter/src/module/os.rs pyre/pyre-interpreter/src/module/nt.rs $(git ls-files pyre/pyre-interpreter/src/module | grep -E '(^|/)(os|nt)\.rs$'); do
if [ -f "$f" ]; then
echo "--- $f"
wc -l "$f"
rg -n -C6 'DirEntry|ScandirIterator|scandir|os_module|nt|register_module|module_ns_store|make_builtin_type|target_os = "windows|cfg!\(windows"' "$f" || true
fi
done
echo "--- os crate references"
rg -n -C4 'os_module|os_scandir|scandir_fn|DirEntry|ScandirIterator|pyre::module::os|module::os' pyre/pyre-interpreter/src pyre | head -250Repository: youknowone/pyre Length of output: 26249 🏁 Script executed: #!/bin/bash
set -euo pipefail
echo "--- build config files mentioning posix/windows/os"
git ls-files | grep -E '(^|/)(Cargo.toml|Cargo.lock|pyproject.toml|oxc_build|build\.rs|Makefile|\.gclient|platform|windows)\.rs$' | head -200 || true
rg -n -C4 'target_arch.*wasm|windows|nt|os\scmodule|os_module|positive|posix|Platform|sys.platform|python314|platforms=' Cargo.toml configure.py pyproject.toml . | head -300 || true
echo "--- search for times_result_seq_type platform handling (mentioned in original comment)"
rg -n -C8 'times_result_seq_type|make_time|ScandirIterator|DirEntry|cfg!\(windows\)|target_os = "windows"' pyre/pyre-interpreter/src pyre/extra_tests/parity_tests | head -300
echo "--- read-only verifier: model name/module derivation and platform tests"
python3 - <<'PY'
from pathlib import Path
import re
f = Path("pyre/pyre-interpreter/src/module/posix/interp_posix.rs")
t = Path("pyre/pyre-interpreter/src/typedef.rs")
test = Path("pyre/extra_tests/parity_tests/dir_entry_uninstantiable_python314.py")
for sym in ["DirEntry", "ScandirIterator"]:
src = f.read_text()
for n, line in enumerate(src.splitlines(), 1):
if f'make_builtin_type("{sym}"' in line:
print(f"{f}:{n}: {line.strip()}")
print("typedef module derivation:")
text = t.read_text()
for n, line in enumerate(text.splitlines(), 1):
if "rsplit_once" in line and "{" in line:
start = n - 8
end = n + 10
print(f"typedef.rs:{start}-{end}")
for i in range(max(1,start), end+1):
print(f" {i}: {text.splitlines()[i-1]}")
print("test module expectation:")
lines = test.read_text()
for n, line in enumerate(lines.splitlines(), 1):
if "MODULE = " in line or "=='%s." in line:
print(f"{test}:{n}: {line.strip()}")
print("scan for any Windows module-name rewrite around these symbols:")
root = Path("pyre/pyre-interpreter/src")
matches=[]
for p in root.rglob("*.rs"):
s=p.read_text(errors="ignore")
if "DirEntry" in s or "ScandirIterator" in s or "times_result_seq_type" in s:
for n,line in enumerate(s.splitlines(),1):
if any(sg in line for sg in ["DirEntry", "ScandirIterator", "times_result_seq_type", 'make_builtin_type("nt', 'make_builtin_type("posix']):
matches.append((str(p),n,line.strip()))
for p,n,line in matches:
print(f"{p}:{n}: {line}")
PYRepository: youknowone/pyre Length of output: 50373 Make the scandir-related typedef names platform-aware.
🧰 Tools🪛 Ruff (0.16.1)[warning] 29-29: Use format specifiers instead of percent format Replace with format specifiers (UP031) [warning] 34-34: Found assertion on exception (PT017) [warning] 34-34: Found assertion on exception (PT017) [warning] 34-34: Use format specifiers instead of percent format Replace with format specifiers (UP031) [warning] 36-36: Avoid specifying long messages outside the exception class (TRY003) [warning] 42-42: Use format specifiers instead of percent format Replace with format specifiers (UP031) [warning] 43-43: Found assertion on exception (PT017) [warning] 43-43: Found assertion on exception (PT017) [warning] 45-45: Avoid specifying long messages outside the exception class (TRY003) [warning] 55-55: Found assertion on exception (PT017) [warning] 55-55: Found assertion on exception (PT017) [warning] 55-55: Use format specifiers instead of percent format Replace with format specifiers (UP031) [warning] 57-57: Avoid specifying long messages outside the exception class (TRY003) [warning] 63-63: Use format specifiers instead of percent format Replace with format specifiers (UP031) [warning] 64-64: Found assertion on exception (PT017) [warning] 64-64: Found assertion on exception (PT017) 🤖 Prompt for AI Agents |
||
| else: | ||
| raise AssertionError("ScandirIterator must not be an acceptable base type") | ||
|
|
||
|
|
@@ -72,15 +77,15 @@ class _SubIter(scandir_iterator): | |
| try: | ||
| pickle.dumps(entry, protocol) | ||
| except TypeError as exc: | ||
| assert str(exc) == "cannot pickle 'posix.DirEntry' object", (protocol, str(exc)) | ||
| assert str(exc) == "cannot pickle '%s.DirEntry' object" % MODULE, (protocol, str(exc)) | ||
| else: | ||
| raise AssertionError("a DirEntry must refuse to be pickled") | ||
|
|
||
| for clone in (copy.copy, copy.deepcopy): | ||
| try: | ||
| clone(entry) | ||
| except TypeError as exc: | ||
| assert str(exc) == "cannot pickle 'posix.DirEntry' object", (clone, str(exc)) | ||
| assert str(exc) == "cannot pickle '%s.DirEntry' object" % MODULE, (clone, str(exc)) | ||
| else: | ||
| raise AssertionError("a DirEntry must refuse to be copied") | ||
|
|
||
|
|
||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
🎯 Functional Correctness | 🟠 Major | ⚡ Quick win
Require the CPython implementation before accepting the oracle.
Both resolvers accept any interpreter whose
sys.version_infois(3, 14). A PyPy, GraalPy, or other implementation with that version can pass and become the parity baseline. This invalidates the comparison that these scripts report as CPython parity.pyre/check.py#L41-L79: Includesys.implementation.namein the probe output. Reject candidates unless it equals"cpython"and the version equals(3, 14).pyre/extra_tests/parity_tests/run.py#L126-L179: Apply the same implementation check before_cpython()returns the executable path.As per PR objectives, this layer must require a matching CPython 3.14 interpreter.
🧰 Tools
🪛 ast-grep (0.45.0)
[error] 42-44: Command coming from incoming request
Context: subprocess.run(
[command, "-c", probe], capture_output=True, text=True, timeout=30,
)
Note: [CWE-78] Improper Neutralization of Special Elements used in an OS Command ('OS Command Injection').
(subprocess-from-request)
🪛 Ruff (0.16.1)
[error] 43-43:
subprocesscall: check for execution of untrusted input(S603)
[warning] 43-43:
subprocess.runwithout explicitcheckargumentAdd explicit
check=False(PLW1510)
[warning] 60-60: Missing return type annotation for private function
_resolve_python3(ANN202)
[warning] 79-79: Use format specifiers instead of percent format
(UP031)
📍 Affects 2 files
pyre/check.py#L41-L79(this comment)pyre/extra_tests/parity_tests/run.py#L126-L179🤖 Prompt for AI Agents