From ddc72cdfc075716fd7d82bc108194d2758ad278b Mon Sep 17 00:00:00 2001 From: wim glenn Date: Fri, 8 Mar 2024 01:23:51 -0600 Subject: [PATCH 1/6] Added ability to select bytecode invalidation mode of generated .pyc files. Since Python 3.7, deterministic pycs are possible (see https://peps.python.org/pep-0552/) To select the bytecode invalidation mode explicitly by env var: PYC_INVALIDATION_MODE=UNCHECKED_HASH uv pip install --compile ... Valid values are TIMESTAMP (default), CHECKED_HASH, and UNCHECKED_HASH The latter options are useful for reproducible builds. --- crates/uv-installer/src/pip_compileall.py | 10 +++++++++- 1 file changed, 9 insertions(+), 1 deletion(-) diff --git a/crates/uv-installer/src/pip_compileall.py b/crates/uv-installer/src/pip_compileall.py index 454c6eb70478..1e259790b940 100644 --- a/crates/uv-installer/src/pip_compileall.py +++ b/crates/uv-installer/src/pip_compileall.py @@ -9,6 +9,8 @@ """ import compileall +import os +import py_compile import sys import warnings @@ -18,6 +20,12 @@ # Successful launch check print("Ready") + # https://docs.python.org/3/library/py_compile.html#py_compile.PycInvalidationMode + # TIMESTAMP, CHECKED_HASH, UNCHECKED_HASH + mode = os.environ.get("PYC_INVALIDATION_MODE") + if mode is not None: + mode = py_compile.PycInvalidationMode[mode] + # In rust, we provide one line per file to compile. for path in sys.stdin: # Remove trailing newlines. @@ -27,6 +35,6 @@ # Unlike pip, we set quiet=2, so we don't have to capture stdout. # We'd like to show those errors, but given that pip thinks that's totally fine, # we can't really change that. - success = compileall.compile_file(path, force=True, quiet=2) + success = compileall.compile_file(path, invalidation_mode=mode, force=True, quiet=2) # We're ready for the next file. print(path) From 93febfda6be08249375fc9f4bb92b10616665859 Mon Sep 17 00:00:00 2001 From: konstin Date: Fri, 8 Mar 2024 09:40:14 +0100 Subject: [PATCH 2/6] Add error handling and test --- crates/uv-installer/src/compile.rs | 18 ++++-- crates/uv-installer/src/pip_compileall.py | 21 +++++-- crates/uv/tests/common/mod.rs | 14 +++++ crates/uv/tests/pip_sync.rs | 67 +++++++++++++++-------- 4 files changed, 88 insertions(+), 32 deletions(-) diff --git a/crates/uv-installer/src/compile.rs b/crates/uv-installer/src/compile.rs index 4d0a82aacbcf..49c4ca4cc0d2 100644 --- a/crates/uv-installer/src/compile.rs +++ b/crates/uv-installer/src/compile.rs @@ -82,7 +82,7 @@ pub async fn compile_tree( let tempdir = tempdir_in(cache).map_err(CompileError::TempFile)?; let pip_compileall_py = tempdir.path().join("pip_compileall.py"); - // Start the workers. + debug!("Starting {} bytecode compilation workers", worker_count); let mut worker_handles = Vec::new(); for _ in 0..worker_count.get() { worker_handles.push(tokio::task::spawn(worker( @@ -92,6 +92,8 @@ pub async fn compile_tree( receiver.clone(), ))); } + // Make sure the channel gets closed when all workers exit. + drop(receiver); // Start the producer, sending all `.py` files to workers. let mut source_files = 0; @@ -191,9 +193,11 @@ async fn worker( device: "stderr", err, })?; - if !child_stderr_collected.is_empty() { + let result = if child_stderr_collected.is_empty() { + result + } else { let stderr = String::from_utf8_lossy(&child_stderr_collected); - return match result { + match result { Ok(()) => { debug!( "Bytecode compilation `python` at {} stderr:\n{}\n---", @@ -203,11 +207,13 @@ async fn worker( Ok(()) } Err(err) => Err(CompileError::ErrorWithStderr { - stderr: stderr.to_string(), + stderr: stderr.trim().to_string(), err: Box::new(err), }), - }; - } + } + }; + + debug!("Bytecode compilation worker exiting: {:?}", result); result } diff --git a/crates/uv-installer/src/pip_compileall.py b/crates/uv-installer/src/pip_compileall.py index 1e259790b940..47e0242f6399 100644 --- a/crates/uv-installer/src/pip_compileall.py +++ b/crates/uv-installer/src/pip_compileall.py @@ -22,9 +22,20 @@ # https://docs.python.org/3/library/py_compile.html#py_compile.PycInvalidationMode # TIMESTAMP, CHECKED_HASH, UNCHECKED_HASH - mode = os.environ.get("PYC_INVALIDATION_MODE") - if mode is not None: - mode = py_compile.PycInvalidationMode[mode] + invalidation_mode = os.environ.get("PYC_INVALIDATION_MODE") + if invalidation_mode is not None: + try: + invalidation_mode = py_compile.PycInvalidationMode[invalidation_mode] + except KeyError: + invalidation_modes = ", ".join( + '"' + x.name + '"' for x in py_compile.PycInvalidationMode + ) + print( + f'Invalid value for PYC_INVALIDATION_MODE "{invalidation_mode}", ' + f"valid are {invalidation_modes}: ", + file=sys.stderr, + ) + sys.exit(1) # In rust, we provide one line per file to compile. for path in sys.stdin: @@ -35,6 +46,8 @@ # Unlike pip, we set quiet=2, so we don't have to capture stdout. # We'd like to show those errors, but given that pip thinks that's totally fine, # we can't really change that. - success = compileall.compile_file(path, invalidation_mode=mode, force=True, quiet=2) + success = compileall.compile_file( + path, invalidation_mode=invalidation_mode, force=True, quiet=2 + ) # We're ready for the next file. print(path) diff --git a/crates/uv/tests/common/mod.rs b/crates/uv/tests/common/mod.rs index 45c8affefd22..9d1c6637f5fe 100644 --- a/crates/uv/tests/common/mod.rs +++ b/crates/uv/tests/common/mod.rs @@ -174,6 +174,20 @@ impl TestContext { pub fn python_kind(&self) -> &str { "python" } + + /// Returns the site-packages folder inside the venv. + pub fn site_packages(&self) -> PathBuf { + if cfg!(unix) { + self.venv + .join("lib") + .join(format!("{}{}", self.python_kind(), self.python_version)) + .join("site-packages") + } else if cfg!(windows) { + self.venv.join("Lib").join("site-packages") + } else { + unimplemented!("Only Windows and Unix are supported") + } + } } pub fn venv_to_interpreter(venv: &Path) -> PathBuf { diff --git a/crates/uv/tests/pip_sync.rs b/crates/uv/tests/pip_sync.rs index b10c6bf8a30b..05c137de8d3c 100644 --- a/crates/uv/tests/pip_sync.rs +++ b/crates/uv/tests/pip_sync.rs @@ -2,7 +2,7 @@ use fs_err as fs; use std::env::consts::EXE_SUFFIX; -use std::path::{Path, PathBuf}; +use std::path::Path; use std::process::Command; use anyhow::Result; @@ -73,25 +73,6 @@ fn uninstall_command(context: &TestContext) -> Command { command } -/// Returns the site-packages folder inside the venv. -fn site_packages(context: &TestContext) -> PathBuf { - if cfg!(unix) { - context - .venv - .join("lib") - .join(format!( - "{}{}", - context.python_kind(), - context.python_version - )) - .join("site-packages") - } else if cfg!(windows) { - context.venv.join("Lib").join("site-packages") - } else { - unimplemented!("Only Windows and Unix are supported") - } -} - #[test] fn missing_pip() { uv_snapshot!(Command::new(get_bin()).arg("sync"), @r###" @@ -186,7 +167,8 @@ fn install() -> Result<()> { ); // Counterpart for the `compile()` test. - assert!(!site_packages(&context) + assert!(!context + .site_packages() .join("markupsafe") .join("__pycache__") .join("__init__.cpython-312.pyc") @@ -2946,7 +2928,8 @@ fn compile() -> Result<()> { "### ); - assert!(site_packages(&context) + assert!(context + .site_packages() .join("markupsafe") .join("__pycache__") .join("__init__.cpython-312.pyc") @@ -2957,6 +2940,46 @@ fn compile() -> Result<()> { Ok(()) } +/// Test that the `PYC_INVALIDATION_MODE` option is recognized and that the error handling works. +#[test] +fn compile_invalid_pyc_invalidation_mode() -> Result<()> { + let context = TestContext::new("3.12"); + + let requirements_txt = context.temp_dir.child("requirements.txt"); + requirements_txt.touch()?; + requirements_txt.write_str("MarkupSafe==2.1.3")?; + + let mut filters = INSTA_FILTERS.to_vec(); + let site_packages = context.site_packages().display().to_string(); + filters.push((&site_packages, "[SITE-PACKAGES]")); + filters.push(( + r#"\[SITE-PACKAGES\]/.*.py", received: "#, + r#"[SITE-PACKAGES]/[FIRST-FILE]", received: "#, + )); + + uv_snapshot!(filters, command(&context) + .arg("requirements.txt") + .arg("--compile") + .arg("--strict") + .env("PYC_INVALIDATION_MODE", "bogus"), @r###" + success: false + exit_code: 2 + ----- stdout ----- + + ----- stderr ----- + Resolved 1 package in [TIME] + Downloaded 1 package in [TIME] + Installed 1 package in [TIME] + error: Failed to bytecode compile [SITE-PACKAGES] + Caused by: Python process stderr: + Invalid value for PYC_INVALIDATION_MODE "bogus", valid are "TIMESTAMP", "CHECKED_HASH", "UNCHECKED_HASH": + Caused by: Bytecode compilation failed, expected "[SITE-PACKAGES]/[FIRST-FILE]", received: "" + "### + ); + + Ok(()) +} + /// Raise an error when an editable's `Requires-Python` constraint is not met. #[test] fn requires_python_editable() -> Result<()> { From 2442ba8496b12cd49d86fcd8b900e6b3b114b04f Mon Sep 17 00:00:00 2001 From: konstin Date: Fri, 8 Mar 2024 10:00:31 +0100 Subject: [PATCH 3/6] Fix path escaping --- crates/uv/tests/pip_sync.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/crates/uv/tests/pip_sync.rs b/crates/uv/tests/pip_sync.rs index 05c137de8d3c..420af05bd412 100644 --- a/crates/uv/tests/pip_sync.rs +++ b/crates/uv/tests/pip_sync.rs @@ -2950,7 +2950,7 @@ fn compile_invalid_pyc_invalidation_mode() -> Result<()> { requirements_txt.write_str("MarkupSafe==2.1.3")?; let mut filters = INSTA_FILTERS.to_vec(); - let site_packages = context.site_packages().display().to_string(); + let site_packages = regex::escape(&context.site_packages().display().to_string()); filters.push((&site_packages, "[SITE-PACKAGES]")); filters.push(( r#"\[SITE-PACKAGES\]/.*.py", received: "#, From 84e9ba4e79fbab0dba5449cfb1232545dece229f Mon Sep 17 00:00:00 2001 From: konstin Date: Fri, 8 Mar 2024 10:40:02 +0100 Subject: [PATCH 4/6] Fix windows tests --- crates/uv-installer/src/compile.rs | 2 +- crates/uv/tests/pip_sync.rs | 18 +++++++++++------- 2 files changed, 12 insertions(+), 8 deletions(-) diff --git a/crates/uv-installer/src/compile.rs b/crates/uv-installer/src/compile.rs index 49c4ca4cc0d2..d4140f975919 100644 --- a/crates/uv-installer/src/compile.rs +++ b/crates/uv-installer/src/compile.rs @@ -32,7 +32,7 @@ pub enum CompileError { PythonSubcommand(#[source] io::Error), #[error("Failed to create temporary script file")] TempFile(#[source] io::Error), - #[error("Bytecode compilation failed, expected {0:?}, received: {1:?}")] + #[error(r#"Bytecode compilation failed, expected "{0}", received: "{1}""#)] WrongPath(String, String), #[error("Failed to write to Python {device}")] ChildStdio { diff --git a/crates/uv/tests/pip_sync.rs b/crates/uv/tests/pip_sync.rs index 420af05bd412..c0a175305a17 100644 --- a/crates/uv/tests/pip_sync.rs +++ b/crates/uv/tests/pip_sync.rs @@ -2949,13 +2949,17 @@ fn compile_invalid_pyc_invalidation_mode() -> Result<()> { requirements_txt.touch()?; requirements_txt.write_str("MarkupSafe==2.1.3")?; - let mut filters = INSTA_FILTERS.to_vec(); - let site_packages = regex::escape(&context.site_packages().display().to_string()); - filters.push((&site_packages, "[SITE-PACKAGES]")); - filters.push(( - r#"\[SITE-PACKAGES\]/.*.py", received: "#, - r#"[SITE-PACKAGES]/[FIRST-FILE]", received: "#, - )); + let site_packages = regex::escape(&context.site_packages().simplified_display().to_string()); + let filters: Vec<_> = [ + (site_packages.as_str(), "[SITE-PACKAGES]"), + ( + r#"\[SITE-PACKAGES\].*.py", received: "#, + r#"[SITE-PACKAGES]/[FIRST-FILE]", received: "# + ), + ] + .into_iter() + .chain(INSTA_FILTERS.to_vec()) + .collect(); uv_snapshot!(filters, command(&context) .arg("requirements.txt") From 71cac81584b9d8488e9b0ba939fbe3a38b05e0c8 Mon Sep 17 00:00:00 2001 From: konstin Date: Fri, 8 Mar 2024 12:15:42 +0100 Subject: [PATCH 5/6] cargo fmt --- crates/uv/tests/pip_sync.rs | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/crates/uv/tests/pip_sync.rs b/crates/uv/tests/pip_sync.rs index c0a175305a17..31248736f985 100644 --- a/crates/uv/tests/pip_sync.rs +++ b/crates/uv/tests/pip_sync.rs @@ -2954,12 +2954,12 @@ fn compile_invalid_pyc_invalidation_mode() -> Result<()> { (site_packages.as_str(), "[SITE-PACKAGES]"), ( r#"\[SITE-PACKAGES\].*.py", received: "#, - r#"[SITE-PACKAGES]/[FIRST-FILE]", received: "# + r#"[SITE-PACKAGES]/[FIRST-FILE]", received: "#, ), ] - .into_iter() - .chain(INSTA_FILTERS.to_vec()) - .collect(); + .into_iter() + .chain(INSTA_FILTERS.to_vec()) + .collect(); uv_snapshot!(filters, command(&context) .arg("requirements.txt") From c29b42dcbc31c77cf52502027e30d958e687bf11 Mon Sep 17 00:00:00 2001 From: konstin Date: Fri, 8 Mar 2024 16:03:58 +0100 Subject: [PATCH 6/6] Fix mac os, maybe? --- crates/uv/tests/pip_sync.rs | 9 ++++++++- 1 file changed, 8 insertions(+), 1 deletion(-) diff --git a/crates/uv/tests/pip_sync.rs b/crates/uv/tests/pip_sync.rs index 31248736f985..ae16f49d505e 100644 --- a/crates/uv/tests/pip_sync.rs +++ b/crates/uv/tests/pip_sync.rs @@ -2949,7 +2949,14 @@ fn compile_invalid_pyc_invalidation_mode() -> Result<()> { requirements_txt.touch()?; requirements_txt.write_str("MarkupSafe==2.1.3")?; - let site_packages = regex::escape(&context.site_packages().simplified_display().to_string()); + let site_packages = regex::escape( + &context + .site_packages() + .canonicalize() + .unwrap() + .simplified_display() + .to_string(), + ); let filters: Vec<_> = [ (site_packages.as_str(), "[SITE-PACKAGES]"), (