Skip to content

Commit ef7b267

Browse files
committed
Fix 703
1 parent b6de8cb commit ef7b267

File tree

6 files changed

+267
-30
lines changed

6 files changed

+267
-30
lines changed

Cargo.lock

Lines changed: 1 addition & 0 deletions
Some generated files are not rendered by default. Learn more about customizing how changed files appear on GitHub.

py/tools/venv_shim/BUILD.bazel

Lines changed: 8 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1,19 +1,27 @@
11
load("//bazel/release:release.bzl", "release")
22
load("//bazel/rust:defs.bzl", "rust_binary")
3+
load("@rules_rust//rust:defs.bzl", "rust_test")
34
load("//bazel/rust:multi_platform_rust_binaries.bzl", "multi_platform_rust_binaries")
45

56
rust_binary(
67
name = "shim",
78
srcs = [
9+
"src/pyargs.rs",
810
"src/main.rs",
911
],
1012
deps = [
1113
"//py/tools/runfiles",
1214
"@crates//:miette",
1315
"@crates//:which",
16+
"@crates//:clap",
1417
],
1518
)
1619

20+
rust_test(
21+
name = "shim_tests",
22+
crate = ":shim",
23+
)
24+
1725
multi_platform_rust_binaries(
1826
name = "bins",
1927
target = ":shim",

py/tools/venv_shim/Cargo.toml

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -20,5 +20,6 @@ path = "src/main.rs"
2020

2121
[dependencies]
2222
miette = { workspace = true }
23+
clap.workspace = true
2324
runfiles = { path = "../runfiles" }
2425
which = "8.0.0"

py/tools/venv_shim/src/main.rs

Lines changed: 17 additions & 20 deletions
Original file line numberDiff line numberDiff line change
@@ -1,4 +1,6 @@
1-
use miette::{miette, Context, IntoDiagnostic};
1+
mod pyargs;
2+
3+
use miette::{miette, Context, IntoDiagnostic, Result};
24
use runfiles::Runfiles;
35
use which::which;
46
// Depended on out of rules_rust
@@ -339,13 +341,19 @@ fn main() -> miette::Result<()> {
339341
&actual_interpreter, &venv_interpreter, exec_args,
340342
);
341343

344+
let mut reexec_args = pyargs::reparse_args(&all_args.iter().map(|it| it.as_ref()).collect())?;
345+
// Lie about the value of argv0 to hoodwink the interpreter as to its
346+
// location on Linux-based platforms.
347+
reexec_args.remove(0);
348+
349+
#[cfg(feature = "debug")]
350+
eprintln!("Reexec args {:?}", reexec_args);
351+
342352
let mut cmd = Command::new(&actual_interpreter);
343353
let cmd = cmd
344-
// Pass along our args
345-
.args(exec_args)
346-
// Lie about the value of argv0 to hoodwink the interpreter as to its
347-
// location on Linux-based platforms.
348354
.arg0(&venv_interpreter)
355+
// Pass along our args
356+
.args(reexec_args)
349357
// Pseudo-`activate`
350358
.env("VIRTUAL_ENV", &venv_root);
351359

@@ -400,26 +408,15 @@ fn main() -> miette::Result<()> {
400408
eprintln!("Setting PYTHONHOME to {home:?} for {actual_interpreter:?}");
401409
cmd.env("PYTHONHOME", home);
402410

411+
// For the future, we could read, validate and reuse the env state.
403412
let mut hasher = DefaultHasher::new();
404413
venv_interpreter.to_str().unwrap().hash(&mut hasher);
414+
venv_root.to_str().unwrap().hash(&mut hasher);
405415
home.to_str().unwrap().hash(&mut hasher);
406-
407416
cmd.env("ASPECT_PY_VALIDITY", format!("{}", hasher.finish()));
408417

409-
// For the future, we could read, validate and reuse the env state.
410-
//
411-
// if let Ok(home) = env::var("PYTHONHOME") {
412-
// if let Ok(executable) = env::var("PYTHONEXECUTABLE") {
413-
// if let Ok(checksum) = env::var("ASPECT_PY_VALIDITY") {
414-
// let mut hasher = DefaultHasher::new();
415-
// executable.hash(&mut hasher);
416-
// home.hash(&mut hasher);
417-
// if checksum == format!("{}", hasher.finish()) {
418-
// return Ok(PathBuf::from(home).join("bin/python3"));
419-
// }
420-
// }
421-
// }
422-
// }
418+
#[cfg(feature = "debug")]
419+
eprintln!("Punting to {:?}", cmd);
423420

424421
// And punt
425422
let err = cmd.exec();

py/tools/venv_shim/src/pyargs.rs

Lines changed: 231 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,231 @@
1+
use clap::{arg, Arg, ArgAction, ArgMatches, Command};
2+
use miette::{IntoDiagnostic, Result};
3+
4+
fn build_parser() -> Command {
5+
Command::new("python_like_parser")
6+
.disable_version_flag(true)
7+
.disable_help_flag(true)
8+
.dont_delimit_trailing_values(true)
9+
.allow_hyphen_values(true)
10+
.arg(Arg::new("ignore_env").short('E').action(ArgAction::SetTrue))
11+
.arg(Arg::new("isolate").short('I').action(ArgAction::SetTrue))
12+
.arg(
13+
Arg::new("no_user_site")
14+
.short('s')
15+
.action(ArgAction::SetTrue),
16+
)
17+
.arg(
18+
Arg::new("no_import_site")
19+
.short('S')
20+
.action(ArgAction::SetTrue),
21+
)
22+
.arg(
23+
arg!(<args> ...)
24+
.trailing_var_arg(true)
25+
.required(false)
26+
.allow_hyphen_values(true),
27+
)
28+
}
29+
30+
pub struct ArgState {
31+
pub ignore_env: bool,
32+
pub isolate: bool,
33+
pub no_import_site: bool,
34+
pub no_user_site: bool,
35+
pub remaining_args: Vec<String>,
36+
}
37+
38+
fn extract_state(matches: &ArgMatches) -> ArgState {
39+
ArgState {
40+
// E and I are crucial for transformation
41+
ignore_env: *matches.get_one::<bool>("ignore_env").unwrap_or(&false),
42+
isolate: *matches.get_one::<bool>("isolate").unwrap_or(&false),
43+
44+
// s is crucial for transformation
45+
no_user_site: *matches.get_one::<bool>("no_user_site").unwrap_or(&false),
46+
47+
no_import_site: *matches.get_one::<bool>("no_import_site").unwrap_or(&false),
48+
49+
remaining_args: matches
50+
.get_many::<String>("args")
51+
.unwrap_or_default()
52+
.map(|it| it.to_string())
53+
.collect::<Vec<_>>(),
54+
}
55+
}
56+
57+
pub fn reparse_args(original_argv: &Vec<&str>) -> Result<Vec<String>> {
58+
let parser = build_parser();
59+
let matches = parser
60+
.try_get_matches_from(original_argv)
61+
.into_diagnostic()?;
62+
let parsed_args = extract_state(&matches);
63+
64+
let mut argv: Vec<String> = Vec::new();
65+
let push_flag = |argv: &mut Vec<String>, flag: char, is_set: bool| {
66+
if is_set {
67+
argv.push(format!("-{}", flag));
68+
}
69+
};
70+
71+
// Retain the original argv binary
72+
argv.push(original_argv[0].to_string());
73+
74+
// -I replacement logic: -I is never pushed, its effects (-E and -s) are handled separately.
75+
// -E removal: -E is never pushd
76+
// -s inclusion logic: we ALWAYS push -s
77+
push_flag(
78+
&mut argv,
79+
's',
80+
parsed_args.no_user_site | parsed_args.isolate,
81+
);
82+
83+
push_flag(&mut argv, 'S', parsed_args.no_import_site);
84+
85+
argv.extend(parsed_args.remaining_args.iter().cloned());
86+
87+
Ok(argv)
88+
}
89+
90+
#[cfg(test)]
91+
mod test {
92+
use super::*;
93+
94+
#[test]
95+
fn basic_args_preserved1() {
96+
let orig = vec!["python", "-B", "-s", "script.py", "arg1"];
97+
let reparsed = reparse_args(&orig);
98+
assert!(reparsed.is_ok(), "Args failed to parse {:?}", reparsed);
99+
let reparsed = reparsed.unwrap();
100+
assert!(
101+
orig == reparsed,
102+
"Args shouldn't have changed, got {:?}",
103+
reparsed
104+
);
105+
}
106+
107+
#[test]
108+
fn basic_args_preserved2() {
109+
let orig = vec!["python", "-s", "-c", "exit(0)", "arg1"];
110+
let reparsed = reparse_args(&orig);
111+
assert!(&reparsed.is_ok(), "Args failed to parse {:?}", reparsed);
112+
let reparsed = reparsed.unwrap();
113+
assert!(
114+
orig == reparsed,
115+
"Args shouldn't have changed, got {:?}",
116+
reparsed
117+
);
118+
}
119+
120+
#[test]
121+
fn basic_binary_preserved() {
122+
let orig = vec![
123+
"/some/arbitrary/path/python",
124+
"-B",
125+
"-s",
126+
"script.py",
127+
"arg1",
128+
];
129+
let reparsed = reparse_args(&orig);
130+
assert!(reparsed.is_ok(), "Args failed to parse {:?}", reparsed);
131+
let reparsed = reparsed.unwrap();
132+
assert!(
133+
orig == reparsed,
134+
"Args shouldn't have changed, got {:?}",
135+
reparsed
136+
);
137+
}
138+
139+
#[test]
140+
fn basic_s_remains() {
141+
// We expect to not modify the -s flag
142+
let orig = vec!["python", "-s", "-c", "exit(0)", "arg1"];
143+
let reparsed = reparse_args(&orig);
144+
assert!(reparsed.is_ok(), "Args failed to parse {:?}", reparsed);
145+
assert!(orig == reparsed.unwrap(), "Args shouldn't have changed");
146+
}
147+
148+
#[test]
149+
fn basic_e_gets_unset() {
150+
// We expect to REMOVE the -E flag
151+
let orig = vec!["python", "-E", "-s", "-c", "exit(0)", "arg1"];
152+
let expected = vec!["python", "-s", "-c", "exit(0)", "arg1"];
153+
let reparsed = reparse_args(&orig);
154+
assert!(reparsed.is_ok(), "Args failed to parse {:?}", reparsed);
155+
assert!(expected == reparsed.unwrap(), "-E wasn't unset");
156+
}
157+
158+
#[test]
159+
fn basic_i_becomes_s() {
160+
// We expect to CONVERT the -I flag to -E (which we ignore) and -s (which we keep)
161+
let orig = vec!["python", "-I", "-c", "exit(0)", "arg1"];
162+
let expected = vec!["python", "-s", "-c", "exit(0)", "arg1"];
163+
let reparsed = reparse_args(&orig);
164+
assert!(reparsed.is_ok(), "Args failed to parse {:?}", reparsed);
165+
assert!(expected == reparsed.unwrap(), "Didn't translate -I to -s");
166+
}
167+
168+
#[test]
169+
fn basic_m_preserved() {
170+
// We expect to ADD the -s flag
171+
let orig = vec!["python", "-m", "build", "--unknown", "arg1"];
172+
let expected = vec!["python", "-m", "build", "--unknown", "arg1"];
173+
let reparsed = reparse_args(&orig);
174+
assert!(reparsed.is_ok(), "Args failed to parse {:?}", reparsed);
175+
assert!(expected == reparsed.unwrap(), "Didn't add -s");
176+
}
177+
178+
#[test]
179+
fn basic_trailing_args_preserved() {
180+
let orig = vec![
181+
"python3",
182+
"uv/private/sdist_build/build_helper.py",
183+
"bazel-out/darwin_arm64-fastbuild/bin/external/+uv+sbuild__pypi__default__bravado_core/src",
184+
"bazel-out/darwin_arm64-fastbuild/bin/external/+uv+sbuild__pypi__default__bravado_core/build"
185+
];
186+
let expected = vec![
187+
"python3",
188+
"uv/private/sdist_build/build_helper.py",
189+
"bazel-out/darwin_arm64-fastbuild/bin/external/+uv+sbuild__pypi__default__bravado_core/src",
190+
"bazel-out/darwin_arm64-fastbuild/bin/external/+uv+sbuild__pypi__default__bravado_core/build"
191+
];
192+
let reparsed = reparse_args(&orig);
193+
assert!(reparsed.is_ok(), "Args failed to parse {:?}", reparsed);
194+
let reparsed = reparsed.unwrap();
195+
assert!(
196+
expected == reparsed,
197+
"Something happened to the args, got {:?}",
198+
reparsed
199+
);
200+
}
201+
202+
#[test]
203+
fn m_preserved_s_added_varargs_preserved() {
204+
let orig = vec![
205+
"python3",
206+
"-m",
207+
"build",
208+
"--no-isolation",
209+
"--out-dir",
210+
"bazel-out/darwin_arm64-fastbuild/bin/external/+uv+sbuild__pypi__default__bravado_core/build",
211+
"bazel-out/darwin_arm64-fastbuild/bin/external/+uv+sbuild__pypi__default__bravado_core/src",
212+
];
213+
let expected = vec![
214+
"python3",
215+
"-m",
216+
"build",
217+
"--no-isolation",
218+
"--out-dir",
219+
"bazel-out/darwin_arm64-fastbuild/bin/external/+uv+sbuild__pypi__default__bravado_core/build",
220+
"bazel-out/darwin_arm64-fastbuild/bin/external/+uv+sbuild__pypi__default__bravado_core/src",
221+
];
222+
let reparsed = reparse_args(&orig);
223+
assert!(reparsed.is_ok(), "Args failed to parse {:?}", reparsed);
224+
let reparsed = reparsed.unwrap();
225+
assert!(
226+
expected == reparsed,
227+
"Something happened to the args, got {:?}",
228+
reparsed
229+
);
230+
}
231+
}

uv/private/sdist_build/build_helper.py

Lines changed: 9 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -3,8 +3,8 @@
33
from argparse import ArgumentParser
44
import shutil
55
import sys
6-
from os import getenv, listdir, path
7-
from subprocess import call
6+
from os import getenv
7+
from build.__main__ import main as build
88

99
# Under Bazel, the source dir of a sdist to build is immutable. `build` and
1010
# other tools however are constitutionally incapable of not writing to the
@@ -15,6 +15,8 @@
1515
# - It punts to `build` targeting the tempdir
1616

1717
print(sys.executable, file=sys.stderr)
18+
for e in sys.path:
19+
print(" -", e, file=sys.stderr)
1820

1921
PARSER = ArgumentParser()
2022
PARSER.add_argument("srcdir")
@@ -27,14 +29,11 @@
2729
shutil.copystat = lambda x, y, **k: None
2830
shutil.copytree(opts.srcdir, t, dirs_exist_ok=True)
2931

30-
outdir = path.abspath(opts.outdir)
32+
# 1
3133

32-
call([
33-
sys.executable,
34-
"-m", "build",
34+
build([
3535
"--wheel",
3636
"--no-isolation",
37-
"--outdir", outdir,
38-
], cwd=t)
39-
40-
print(listdir(outdir), file=sys.stderr)
37+
"--outdir", opts.outdir,
38+
t,
39+
])

0 commit comments

Comments
 (0)