From 74ffa7b73b33c5f30d6b3a1139174b5e6d447e11 Mon Sep 17 00:00:00 2001 From: seonghobae <8172694+seonghobae@users.noreply.github.com> Date: Sun, 28 Jun 2026 17:28:08 +0000 Subject: [PATCH 1/3] =?UTF-8?q?=E2=9A=A1=20Bolt:=20[=EC=84=B1=EB=8A=A5=20?= =?UTF-8?q?=EA=B0=9C=EC=84=A0]=20subprocess=20=EC=9D=B8=EC=9E=90=EC=97=90?= =?UTF-8?q?=20=EC=82=AC=EC=9A=A9=EB=90=98=EB=8A=94=20resolve()=EB=A5=BC=20?= =?UTF-8?q?absolute()=EB=A1=9C=20=EB=8C=80=EC=B2=B4=ED=95=98=EC=97=AC=20?= =?UTF-8?q?=EB=94=94=EC=8A=A4=ED=81=AC=20I/O=20=EC=A0=88=EA=B0=90?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit --- .jules/bolt.md | 4 ++++ media_shrinker.py | 8 ++++---- 2 files changed, 8 insertions(+), 4 deletions(-) diff --git a/.jules/bolt.md b/.jules/bolt.md index 63607c3..bd4c5d4 100644 --- a/.jules/bolt.md +++ b/.jules/bolt.md @@ -60,3 +60,7 @@ ## 2026-06-25 - [Optimize Path.exists() when paired with stat()] **Learning:** Checking `Path.exists()` before `Path.stat()` introduces a redundant system call because `exists()` internally uses `stat()`. **Action:** Rely on catching the `OSError` from `Path.stat()` to simultaneously check for existence and retrieve file attributes, saving measurable I/O overhead on large filesystems. + +## 2024-06-28 - [Optimize subprocess arguments with Path.absolute()] +**Learning:** `Path.resolve()` is used extensively to resolve paths for command-line arguments (e.g. `ffmpeg`). However, `.resolve()` touches the filesystem to follow symlinks and check for existence, which introduces significant disk I/O overhead. In cases where the path is just passed to an external program (which natively handles the filesystem), using `.absolute()` instead performs a fast purely lexical operation. +**Action:** Replace `Path.resolve()` with `Path.absolute()` for strings passed to `subprocess.run` to bypass the redundant I/O operations and speed up processing. diff --git a/media_shrinker.py b/media_shrinker.py index 3eede1d..00db8ac 100644 --- a/media_shrinker.py +++ b/media_shrinker.py @@ -132,9 +132,9 @@ def command( raise MediaShrinkerError( "ffmpeg argument template is missing '-i'" ) from exc - args[input_index] = str(input_path.resolve()) + args[input_index] = str(input_path.absolute()) if output_path is not None: - args[-1] = str(output_path.resolve()) + args[-1] = str(output_path.absolute()) if overwrite: args = ["-y" if arg == "-n" else arg for arg in args] @@ -633,7 +633,7 @@ def build_icloud_download_command( ) -> list[str]: """Build a safe iCloud download command for source_path.""" - return [brctl_path, "download", str(source_path.resolve())] + return [brctl_path, "download", str(source_path.absolute())] def download_from_icloud(source_path: Path, *, brctl_path: str = "brctl") -> None: @@ -1630,7 +1630,7 @@ def _copy_macos_creation_time( "%m/%d/%Y %H:%M:%S" ) subprocess.run( - [setfile_path, "-d", creation_date, str(dest.resolve())], + [setfile_path, "-d", creation_date, str(dest.absolute())], check=False, capture_output=True, text=True, From 42747c5313b42418172358e3155b1430591df19e Mon Sep 17 00:00:00 2001 From: Seongho Bae Date: Mon, 29 Jun 2026 02:36:18 +0900 Subject: [PATCH 2/3] test: align absolute path optimization evidence --- .jules/bolt.md | 2 +- tests/test_media_shrinker.py | 8 ++++---- 2 files changed, 5 insertions(+), 5 deletions(-) diff --git a/.jules/bolt.md b/.jules/bolt.md index bd4c5d4..dc5af0e 100644 --- a/.jules/bolt.md +++ b/.jules/bolt.md @@ -61,6 +61,6 @@ **Learning:** Checking `Path.exists()` before `Path.stat()` introduces a redundant system call because `exists()` internally uses `stat()`. **Action:** Rely on catching the `OSError` from `Path.stat()` to simultaneously check for existence and retrieve file attributes, saving measurable I/O overhead on large filesystems. -## 2024-06-28 - [Optimize subprocess arguments with Path.absolute()] +## 2026-06-28 - [Optimize subprocess arguments with Path.absolute()] **Learning:** `Path.resolve()` is used extensively to resolve paths for command-line arguments (e.g. `ffmpeg`). However, `.resolve()` touches the filesystem to follow symlinks and check for existence, which introduces significant disk I/O overhead. In cases where the path is just passed to an external program (which natively handles the filesystem), using `.absolute()` instead performs a fast purely lexical operation. **Action:** Replace `Path.resolve()` with `Path.absolute()` for strings passed to `subprocess.run` to bypass the redundant I/O operations and speed up processing. diff --git a/tests/test_media_shrinker.py b/tests/test_media_shrinker.py index 1b202d2..d0e8293 100644 --- a/tests/test_media_shrinker.py +++ b/tests/test_media_shrinker.py @@ -265,7 +265,7 @@ def test_pcm_wav_uses_lossless_flac_first_and_preserves_container_metadata( self.assertIn("0", plan.ffmpeg_args) self.assertIn("flac", plan.ffmpeg_args) - def test_conversion_command_resolves_input_and_output_overrides(self) -> None: + def test_conversion_command_absolutizes_input_and_output_overrides(self) -> None: plan = ConversionPlan( strategy="test", input_path=Path("input.wav"), @@ -280,9 +280,9 @@ def test_conversion_command_resolves_input_and_output_overrides(self) -> None: self.assertEqual( command[command.index("-i") + 1], - str(Path("-input.wav").resolve()), + str(Path("-input.wav").absolute()), ) - self.assertEqual(command[-1], str(Path("-output.flac").resolve())) + self.assertEqual(command[-1], str(Path("-output.flac").absolute())) def test_lossy_audio_uses_highest_opus_bitrate_that_fits_target_with_safety_margin( self, @@ -1042,7 +1042,7 @@ def test_build_icloud_download_command_uses_argument_list_for_paths_with_spaces( self.assertEqual( command, - ["brctl", "download", str(Path("folder/file with spaces.m4a").resolve())], + ["brctl", "download", str(Path("folder/file with spaces.m4a").absolute())], ) From b53d238714f214e45c479f50e5e7adb6424ddd08 Mon Sep 17 00:00:00 2001 From: Seongho Bae Date: Mon, 29 Jun 2026 07:54:40 +0900 Subject: [PATCH 3/3] fix: keep subprocess paths resolved --- media_shrinker.py | 20 ++++++++------------ tests/test_media_shrinker.py | 26 ++++++++++++++++++++++---- 2 files changed, 30 insertions(+), 16 deletions(-) diff --git a/media_shrinker.py b/media_shrinker.py index 00db8ac..b71afca 100644 --- a/media_shrinker.py +++ b/media_shrinker.py @@ -132,9 +132,9 @@ def command( raise MediaShrinkerError( "ffmpeg argument template is missing '-i'" ) from exc - args[input_index] = str(input_path.absolute()) + args[input_index] = str(input_path.resolve()) if output_path is not None: - args[-1] = str(output_path.absolute()) + args[-1] = str(output_path.resolve()) if overwrite: args = ["-y" if arg == "-n" else arg for arg in args] @@ -228,15 +228,11 @@ def find_candidates( except OSError: continue - if excluded_exact_strs: - if not is_symlink: - resolved_d_str = os.path.join(resolved_dir_str, d) - else: - try: - resolved_d_str = os.path.realpath(d_path_str) - except OSError: - continue + if is_symlink: + continue + if excluded_exact_strs: + resolved_d_str = os.path.join(resolved_dir_str, d) if resolved_d_str in excluded_exact_set or resolved_d_str.startswith( excluded_prefix_strs ): @@ -633,7 +629,7 @@ def build_icloud_download_command( ) -> list[str]: """Build a safe iCloud download command for source_path.""" - return [brctl_path, "download", str(source_path.absolute())] + return [brctl_path, "download", str(source_path.resolve())] def download_from_icloud(source_path: Path, *, brctl_path: str = "brctl") -> None: @@ -1630,7 +1626,7 @@ def _copy_macos_creation_time( "%m/%d/%Y %H:%M:%S" ) subprocess.run( - [setfile_path, "-d", creation_date, str(dest.absolute())], + [setfile_path, "-d", creation_date, str(dest.resolve())], check=False, capture_output=True, text=True, diff --git a/tests/test_media_shrinker.py b/tests/test_media_shrinker.py index d0e8293..e4e0e3d 100644 --- a/tests/test_media_shrinker.py +++ b/tests/test_media_shrinker.py @@ -201,6 +201,24 @@ def flaky_lstat(path): self.assertEqual(candidates, [Path("good.mp3")]) + def test_find_candidates_skips_symlinked_directories_outside_root(self) -> None: + with tempfile.TemporaryDirectory() as tmp, tempfile.TemporaryDirectory() as outside: + root = Path(tmp) + external = Path(outside) + (external / "secret.mp3").write_bytes(b"0" * 4) + link = root / "linked" + try: + link.symlink_to(external, target_is_directory=True) + except OSError as exc: + self.skipTest(f"symlink unavailable: {exc}") + + candidates = [ + p[0].relative_to(root) + for p in find_candidates(root, include_under_limit=True) + ] + + self.assertEqual(candidates, []) + class ProbeMediaTests(unittest.TestCase): @patch("media_shrinker.subprocess.run") @@ -265,7 +283,7 @@ def test_pcm_wav_uses_lossless_flac_first_and_preserves_container_metadata( self.assertIn("0", plan.ffmpeg_args) self.assertIn("flac", plan.ffmpeg_args) - def test_conversion_command_absolutizes_input_and_output_overrides(self) -> None: + def test_conversion_command_resolves_input_and_output_overrides(self) -> None: plan = ConversionPlan( strategy="test", input_path=Path("input.wav"), @@ -280,9 +298,9 @@ def test_conversion_command_absolutizes_input_and_output_overrides(self) -> None self.assertEqual( command[command.index("-i") + 1], - str(Path("-input.wav").absolute()), + str(Path("-input.wav").resolve()), ) - self.assertEqual(command[-1], str(Path("-output.flac").absolute())) + self.assertEqual(command[-1], str(Path("-output.flac").resolve())) def test_lossy_audio_uses_highest_opus_bitrate_that_fits_target_with_safety_margin( self, @@ -1042,7 +1060,7 @@ def test_build_icloud_download_command_uses_argument_list_for_paths_with_spaces( self.assertEqual( command, - ["brctl", "download", str(Path("folder/file with spaces.m4a").absolute())], + ["brctl", "download", str(Path("folder/file with spaces.m4a").resolve())], )