From af69249965141de50193144674b2db3fe30f5d55 Mon Sep 17 00:00:00 2001 From: Michael Savigny Date: Fri, 17 Nov 2023 20:16:53 +0000 Subject: [PATCH] Fix deletion of old build logs The code rotating the old build logs expects to delete directories (since it uses rmtree) but tries to delete all items returned by os.listdir(...) which may includes symbolic links or regular files. Change it to only consider the old build logs directories instead. Based on crrev/5028323, with fix to unit test. Fixed: b/310900283 Change-Id: I4c618e6618c0193331c063028ebf02d8c4e7baee Reviewed-on: https://chromium-review.googlesource.com/c/chromium/tools/depot_tools/+/5038467 Auto-Submit: Michael Savigny Reviewed-by: Ramy Medhat Commit-Queue: Michael Savigny --- reclient_helper.py | 10 +++++++--- tests/ninja_reclient_test.py | 7 ++++++- 2 files changed, 13 insertions(+), 4 deletions(-) diff --git a/reclient_helper.py b/reclient_helper.py index 6bcc962c7f..a7000e8d08 100644 --- a/reclient_helper.py +++ b/reclient_helper.py @@ -219,7 +219,11 @@ def set_reproxy_path_flags(out_dir, make_dirs=True): os.makedirs(run_log_dir, exist_ok=True) os.makedirs(cache_dir, exist_ok=True) os.makedirs(racing_dir, exist_ok=True) - old_log_dirs = os.listdir(log_dir) + old_log_dirs = [ + d for d in os.listdir(log_dir) + if os.path.isdir(os.path.join(log_dir, d)) + ] + if len(old_log_dirs) > 5: old_log_dirs.sort(key=lambda dir: dir.split("_"), reverse=True) for d in old_log_dirs[5:]: @@ -269,8 +273,8 @@ def build_context(argv, tool): try: set_reproxy_path_flags(ninja_out) - except OSError: - print("Error creating reproxy_tmp in output dir", file=sys.stderr) + except OSError as e: + print(f"Error creating reproxy_tmp in output dir: {e}", file=sys.stderr) yield 1 return diff --git a/tests/ninja_reclient_test.py b/tests/ninja_reclient_test.py index 268435f412..a7376da957 100755 --- a/tests/ninja_reclient_test.py +++ b/tests/ninja_reclient_test.py @@ -325,7 +325,12 @@ expiry: { '20170316T200042.000000_SOME_RANDOM_ID_2', '20170316T200045.000000_SOME_RANDOM_ID_5', ] - self.assertCountEqual(os.listdir(log_dir), want_remaining_dirs) + + existing_log_dirs = [ + d for d in os.listdir(log_dir) + if os.path.isdir(os.path.join(log_dir, d)) + ] + self.assertCountEqual(existing_log_dirs, want_remaining_dirs) for d in want_remaining_dirs: self.assertTrue( os.path.isfile(