From cec4bd9a238979d4bb3dbf43378acfc9489299ee Mon Sep 17 00:00:00 2001 From: Steven Presti Date: Wed, 10 Jul 2024 17:07:54 -0400 Subject: [PATCH] pyc: add arg reset-m-time In ostree systems, the mtime gets confused and results in a re-compile of the .pyc files due to how the files are linked. Add support to identify and set mtime to zero to avoid this issue. Co-authored-by: Luke Yang --- src/handlers/ar.rs | 2 +- src/handlers/javadoc.rs | 4 +- src/handlers/pyc.rs | 42 ++++++++++--- src/options.rs | 9 ++- tests/cases/adapters.cpython-311~mtime.pyc | Bin 0 -> 228 bytes tests/cases/adapters.cpython-36~mtime.pyc | Bin 0 -> 136 bytes tests/test_handlers/mod.rs | 5 +- tests/test_handlers/test_ar.rs | 16 ++--- tests/test_handlers/test_javadoc.rs | 6 +- tests/test_handlers/test_pyc.rs | 67 +++++++++++++++++++-- 10 files changed, 122 insertions(+), 29 deletions(-) create mode 100644 tests/cases/adapters.cpython-311~mtime.pyc create mode 100644 tests/cases/adapters.cpython-36~mtime.pyc diff --git a/src/handlers/ar.rs b/src/handlers/ar.rs index cc0bfed..771b307 100644 --- a/src/handlers/ar.rs +++ b/src/handlers/ar.rs @@ -156,7 +156,7 @@ mod tests { #[test] fn filter_a() { - let cfg = Rc::new(options::Config::empty(0, true)); + let cfg = Rc::new(options::Config::empty(0, true, false)); let h = Ar::boxed(&cfg); assert!( h.filter(Path::new("/some/path/libfoobar.a")).unwrap()); diff --git a/src/handlers/javadoc.rs b/src/handlers/javadoc.rs index 31d9e7c..4ddee29 100644 --- a/src/handlers/javadoc.rs +++ b/src/handlers/javadoc.rs @@ -148,7 +148,7 @@ mod tests { #[test] fn test_filter_html() { - let cfg = Rc::new(options::Config::empty(1704106800, false)); + let cfg = Rc::new(options::Config::empty(1704106800, false, false)); let h = Javadoc::boxed(&cfg); assert!( h.filter(Path::new("/some/path/page.html")).unwrap()); @@ -162,7 +162,7 @@ mod tests { #[test] fn test_process_line() { - let config = Rc::new(options::Config::empty(1704106800, false)); + let config = Rc::new(options::Config::empty(1704106800, false, false)); let h = Javadoc { config }; let plu = |s| h.process_line(s).unwrap(); diff --git a/src/handlers/pyc.rs b/src/handlers/pyc.rs index 9fa77c2..19ee6c5 100644 --- a/src/handlers/pyc.rs +++ b/src/handlers/pyc.rs @@ -700,8 +700,28 @@ impl PycParser { assert_eq!(data == self.data, removed_count == 0); Ok((removed_count > 0, data)) } + + fn clear_mtime(&mut self) -> Result<(bool, Vec)> { + //The first version supporting PEP 552 is Python 3.7 + let mut cloned = self.data.clone(); + let mut has_been_modified = false; + if self.version >= (3, 7) { + let invalidation = [self.data[4], self.data[5], self.data[6], self.data[7]]; + if invalidation == [0, 0, 0, 0] { + cloned[8..12].fill(0); + has_been_modified = true; + } + } else { + cloned[4..8].fill(0); + has_been_modified = true; + } + + return Ok((has_been_modified, cloned)); + } + } + impl super::Processor for Pyc { fn name(&self) -> &str { "pyc" @@ -713,21 +733,29 @@ impl super::Processor for Pyc { fn process(&self, input_path: &Path) -> Result { let (mut io, input) = InputOutputHelper::open(input_path, self.config.check)?; - + let mut parser = PycParser::from_file(input_path, input)?; if parser.version < (3, 0) { return Ok(super::ProcessResult::Noop); // We don't want to touch python2 files } - + parser.read_object()?; let (have_mod, data) = parser.clear_unused_flag_refs()?; - if have_mod { - io.open_output()?; - io.output.as_mut().unwrap().write_all(&data)?; + parser.data = data; + + let mut have_mod2 = false; + let mut data2 = parser.data.clone(); + if self.config.reset_m_time { + (have_mod2, data2) = parser.clear_mtime()?; } - io.finalize(have_mod) + let has_been_modified = have_mod || have_mod2; + if has_been_modified { + io.open_output()?; + io.output.as_mut().unwrap().write_all(&data2)?; + } + io.finalize(has_been_modified) } } @@ -737,7 +765,7 @@ mod tests { #[test] fn filter_a() { - let cfg = Rc::new(options::Config::empty(0, false)); + let cfg = Rc::new(options::Config::empty(0, false, false)); let h = Pyc::boxed(&cfg); assert!( h.filter(Path::new("/some/path/foobar.pyc")).unwrap()); diff --git a/src/options.rs b/src/options.rs index 74474cb..2c05a02 100644 --- a/src/options.rs +++ b/src/options.rs @@ -58,6 +58,10 @@ struct Options { num_args = 0..=1, default_missing_value = "0")] pub jobs: Option, + + /// Remove mtime from pyc files + #[arg(short, long)] + pub reset_m_time: bool, } pub struct Config { @@ -71,6 +75,7 @@ pub struct Config { pub source_date_epoch: Option, pub handler_names: Vec<&'static str>, pub strict_handlers: bool, + pub reset_m_time: bool, } fn filter_by_name(name: &str, filter: &[&str]) -> bool { @@ -186,12 +191,13 @@ impl Config { source_date_epoch, handler_names, strict_handlers, + reset_m_time: options.reset_m_time, })) } #[allow(dead_code)] // FIXME: should this be marked as #[cfg(test)]? But then the tests don't compile. - pub const fn empty(source_date_epoch: i64, check: bool) -> Self { + pub const fn empty(source_date_epoch: i64, check: bool, reset_mtime: bool) -> Self { Self { inputs: vec![], brp: false, @@ -203,6 +209,7 @@ impl Config { source_date_epoch: Some(source_date_epoch), handler_names: vec![], strict_handlers: false, + reset_m_time: reset_mtime, } } } diff --git a/tests/cases/adapters.cpython-311~mtime.pyc b/tests/cases/adapters.cpython-311~mtime.pyc new file mode 100644 index 0000000000000000000000000000000000000000..d1f8bae962ab78bca28f38a9bc1807f8477541bc GIT binary patch literal 228 zcmZ3^%ge<81dcEJ(?o%EGKd2M%uq(L43IIMfr%lNA%$TXBLl-~Acg=&h7`tN22G|a z?u^u&oP34y{Gyx`KTXCc)`FtUyb?`@&meVGF8UeyxvBca1x2aFC7Jp_RoTS_iOH$@ zi76?%DXAr?MY)-InZ>#KB|!ON{j|)Al2V{jeXyZmGxZ88e{tC4=BJeAq}mmM><8gu gX&~`|nURt41_NsYYeViu29YZaA{St&2q?_}04hjG*`FrLz`*brh~a<<$Z`PUVjduo%857q(JpjS|Ni^C>2KczG$ M)s7Kl0}wL+0D|TrApigX literal 0 HcmV?d00001 diff --git a/tests/test_handlers/mod.rs b/tests/test_handlers/mod.rs index c02332b..855e8c3 100644 --- a/tests/test_handlers/mod.rs +++ b/tests/test_handlers/mod.rs @@ -22,10 +22,11 @@ fn prepare_dir(path: &str) -> Result<(Box, Box)> { fn make_handler( source_date_epoch: i64, check: bool, + reset_mtime: bool, func: handlers::HandlerBoxed, ) -> Result> { - let cfg = Rc::new(options::Config::empty(source_date_epoch, check)); + let cfg = Rc::new(options::Config::empty(source_date_epoch, check, reset_mtime)); let mut handler = func(&cfg); handler.initialize()?; Ok(handler) @@ -112,7 +113,7 @@ fn test_inode_map() { fn test_inode_map_2() { let (dir, _input) = prepare_dir("tests/cases/testrelro.a").unwrap(); - let cfg = Rc::new(options::Config::empty(111, false)); + let cfg = Rc::new(options::Config::empty(111, false, false)); let ar = handlers::ar::Ar::boxed(&cfg); let handlers = vec![ar]; diff --git a/tests/test_handlers/test_ar.rs b/tests/test_handlers/test_ar.rs index a06dc2f..49c11e3 100644 --- a/tests/test_handlers/test_ar.rs +++ b/tests/test_handlers/test_ar.rs @@ -14,7 +14,7 @@ use super::{prepare_dir, make_handler, test_corpus_file}; fn test_libempty() { let (_dir, input) = prepare_dir("tests/cases/libempty.a").unwrap(); - let ar = make_handler(111, false, ar::Ar::boxed).unwrap(); + let ar = make_handler(111, false, false, ar::Ar::boxed).unwrap(); assert!(ar.filter(&*input).unwrap()); @@ -32,7 +32,7 @@ fn test_libempty() { fn test_testrelro() { let (_dir, input) = prepare_dir("tests/cases/testrelro.a").unwrap(); - let cfg = Rc::new(options::Config::empty(111, false)); + let cfg = Rc::new(options::Config::empty(111, false, false)); let ar = ar::Ar::boxed(&cfg); assert!(ar.filter(&*input).unwrap()); @@ -52,7 +52,7 @@ fn test_testrelro() { fn test_testrelro_check() { let (_dir, input) = prepare_dir("tests/cases/testrelro.a").unwrap(); - let cfg = Rc::new(options::Config::empty(111, true)); + let cfg = Rc::new(options::Config::empty(111, true, false)); let ar = ar::Ar::boxed(&cfg); assert!(ar.filter(&*input).unwrap()); @@ -71,7 +71,7 @@ fn test_testrelro_check() { fn test_testrelro_hardlinked() { let (_dir, input) = prepare_dir("tests/cases/testrelro.a").unwrap(); - let ar = make_handler(111, false, ar::Ar::boxed).unwrap(); + let ar = make_handler(111, false, false, ar::Ar::boxed).unwrap(); assert!(ar.filter(&*input).unwrap()); @@ -91,7 +91,7 @@ fn test_testrelro_hardlinked() { fn test_testrelro_hardlinked_check() { let (_dir, input) = prepare_dir("tests/cases/testrelro.a").unwrap(); - let ar = make_handler(111, true, ar::Ar::boxed).unwrap(); + let ar = make_handler(111, true, false, ar::Ar::boxed).unwrap(); assert!(ar.filter(&*input).unwrap()); @@ -111,7 +111,7 @@ fn test_testrelro_hardlinked_check() { fn test_testrelro_c() { let (_dir, input) = prepare_dir("tests/cases/testrelro.c").unwrap(); - let ar = make_handler(111, false, ar::Ar::boxed).unwrap(); + let ar = make_handler(111, false, false, ar::Ar::boxed).unwrap(); assert!(!ar.filter(&*input).unwrap()); @@ -129,7 +129,7 @@ fn test_testrelro_c() { fn test_testrelro_fixed() { let (_dir, input) = prepare_dir("tests/cases/testrelro.fixed.a").unwrap(); - let ar = make_handler(111, false, ar::Ar::boxed).unwrap(); + let ar = make_handler(111, false, false, ar::Ar::boxed).unwrap(); assert!(ar.filter(&*input).unwrap()); @@ -151,6 +151,6 @@ fn test_libhsbase_compat_batteries() { let filename = "tests/cases/libHSbase-compat-batteries-0.12.3-EvvecFThiaEAGWq5U5Tpi9.a"; - let ar = make_handler(1717842014, false, ar::Ar::boxed).unwrap(); + let ar = make_handler(1717842014, false, false, ar::Ar::boxed).unwrap(); test_corpus_file(ar, filename); } diff --git a/tests/test_handlers/test_javadoc.rs b/tests/test_handlers/test_javadoc.rs index 21f865d..e8b47b2 100644 --- a/tests/test_handlers/test_javadoc.rs +++ b/tests/test_handlers/test_javadoc.rs @@ -12,7 +12,7 @@ use super::{prepare_dir, make_handler}; fn test_javadoc_example() { let (_dir, input) = prepare_dir("tests/cases/javadoc-example.html").unwrap(); - let javadoc = make_handler(1704106800, false, javadoc::Javadoc::boxed).unwrap(); + let javadoc = make_handler(1704106800, false, false, javadoc::Javadoc::boxed).unwrap(); assert!(javadoc.filter(&*input).unwrap()); @@ -34,7 +34,7 @@ fn test_javadoc_example() { fn test_javadoc_fixed() { let (_dir, input) = prepare_dir("tests/cases/javadoc-example.fixed.html").unwrap(); - let javadoc = make_handler(1704106800, false, javadoc::Javadoc::boxed).unwrap(); + let javadoc = make_handler(1704106800, false, false, javadoc::Javadoc::boxed).unwrap(); assert!(javadoc.filter(&*input).unwrap()); @@ -52,7 +52,7 @@ fn test_javadoc_fixed() { fn test_invalid_utf8() { let (_dir, input) = prepare_dir("tests/cases/invalid-utf8.html").unwrap(); - let javadoc = make_handler(1704106800, false, javadoc::Javadoc::boxed).unwrap(); + let javadoc = make_handler(1704106800, false, false, javadoc::Javadoc::boxed).unwrap(); assert!(javadoc.filter(&*input).unwrap()); diff --git a/tests/test_handlers/test_pyc.rs b/tests/test_handlers/test_pyc.rs index 0707706..c56c7b2 100644 --- a/tests/test_handlers/test_pyc.rs +++ b/tests/test_handlers/test_pyc.rs @@ -28,7 +28,7 @@ fn test_pyc_python_version() { fn test_adapters() { let (_dir, input) = prepare_dir("tests/cases/adapters.cpython-312.pyc").unwrap(); - let pyc = make_handler(111, false, pyc::Pyc::boxed).unwrap(); + let pyc = make_handler(111, false, false, pyc::Pyc::boxed).unwrap(); assert!(pyc.filter(&*input).unwrap()); @@ -47,7 +47,7 @@ fn test_adapters() { fn test_adapters_hardlinked() { let (_dir, input) = prepare_dir("tests/cases/adapters.cpython-312.pyc").unwrap(); - let pyc = make_handler(111, false, pyc::Pyc::boxed).unwrap(); + let pyc = make_handler(111, false, false, pyc::Pyc::boxed).unwrap(); assert!(pyc.filter(&*input).unwrap()); @@ -67,7 +67,7 @@ fn test_adapters_hardlinked() { fn test_adapters_opt_1() { let (_dir, input) = prepare_dir("tests/cases/adapters.cpython-312.opt-1.pyc").unwrap(); - let pyc = make_handler(111, false, pyc::Pyc::boxed).unwrap(); + let pyc = make_handler(111, false, false, pyc::Pyc::boxed).unwrap(); assert!(pyc.filter(&*input).unwrap()); @@ -87,7 +87,7 @@ fn test_adapters_opt_1() { fn test_testrelro_fixed() { let (_dir, input) = prepare_dir("tests/cases/adapters.cpython-312~fixed.pyc").unwrap(); - let pyc = make_handler(111, false, pyc::Pyc::boxed).unwrap(); + let pyc = make_handler(111, false, false, pyc::Pyc::boxed).unwrap(); assert!(pyc.filter(&*input).unwrap()); @@ -101,6 +101,63 @@ fn test_testrelro_fixed() { assert_eq!(orig.st_ino(), new.st_ino()); } +#[test] +fn test_reset_mtime_after_37() { + let (_dir, input) = prepare_dir("tests/cases/adapters.cpython-311~mtime.pyc").unwrap(); + + let pyc = make_handler(111, false, true, pyc::Pyc::boxed).unwrap(); + + assert!(pyc.filter(&*input).unwrap()); + + let orig = input.metadata().unwrap(); + + assert_eq!(pyc.process(&*input).unwrap(), handlers::ProcessResult::Replaced); + + let new = input.metadata().unwrap(); + + assert!(orig.created().unwrap() <= new.created().unwrap()); + assert_eq!(orig.modified().unwrap(), new.modified().unwrap()); + assert_ne!(orig.st_ino(), new.st_ino()); +} + +#[test] +fn test_reset_mtime_before_37() { + let (_dir, input) = prepare_dir("tests/cases/adapters.cpython-36~mtime.pyc").unwrap(); + + let pyc = make_handler(111, false, true, pyc::Pyc::boxed).unwrap(); + + assert!(pyc.filter(&*input).unwrap()); + + let orig = input.metadata().unwrap(); + + assert_eq!(pyc.process(&*input).unwrap(), handlers::ProcessResult::Replaced); + + let new = input.metadata().unwrap(); + + assert!(orig.created().unwrap() <= new.created().unwrap()); + assert_eq!(orig.modified().unwrap(), new.modified().unwrap()); + assert_ne!(orig.st_ino(), new.st_ino()); +} + +#[test] +fn test_reset_mtime_disabled() { + let (_dir, input) = prepare_dir("tests/cases/adapters.cpython-311~mtime.pyc").unwrap(); + + let pyc = make_handler(111, false, false, pyc::Pyc::boxed).unwrap(); + + assert!(pyc.filter(&*input).unwrap()); + + let orig = input.metadata().unwrap(); + + assert_eq!(pyc.process(&*input).unwrap(), handlers::ProcessResult::Noop); + + let new = input.metadata().unwrap(); + assert_eq!(orig.created().unwrap(), new.created().unwrap()); + assert_eq!(orig.modified().unwrap(), new.modified().unwrap()); + assert_eq!(orig.st_ino(), new.st_ino()); +} + + #[test] fn test_python_stdlib_file_1() { // Let's call test_python_stdlib_file() once manually for easier development. @@ -108,7 +165,7 @@ fn test_python_stdlib_file_1() { } fn test_python_stdlib_file(filename: &str) { - let pyc = make_handler(1717842014, false, pyc::Pyc::boxed).unwrap(); + let pyc = make_handler(1717842014, false, false, pyc::Pyc::boxed).unwrap(); test_corpus_file(pyc, filename); }