Skip to content

Commit c93fcc0

Browse files
weiguangli-iocopybara-github
authored andcommitted
fix: handle read-only .git files in deploy cleanup on Windows
On Windows, files inside `.git/objects/` are marked read-only by default. When `adk deploy` cleans up the temporary directory in its `finally` block, `shutil.rmtree()` raises `PermissionError: [WinError 5] Access denied` on these files, causing the CLI to crash even after a successful deployment. This PR introduces a `_robust_rmtree()` helper that passes an error handler to `shutil.rmtree`. On Windows, the handler clears the read-only bit (`os.chmod(path, stat.S_IWRITE)`) and retries the deletion. On non-Windows platforms, `shutil.rmtree` is called without any handler. The fix uses `onexc` for Python >= 3.12 and `onerror` for Python < 3.12 to avoid deprecation warnings. All six `shutil.rmtree()` call sites in `cli_deploy.py` (`to_cloud_run`, `to_agent_engine`, `to_gke`) are updated. Fixes #4635 Merge #4719 PiperOrigin-RevId: 964392331
1 parent 69c9090 commit c93fcc0

2 files changed

Lines changed: 66 additions & 6 deletions

File tree

src/google/adk/cli/cli_deploy.py

Lines changed: 24 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -18,6 +18,7 @@
1818
import json
1919
import os
2020
import shutil
21+
import stat
2122
import subprocess
2223
import sys
2324
import traceback
@@ -42,6 +43,23 @@
4243
)
4344

4445

46+
def _on_rm_error(func: Callable[..., Any], path: str, exc_info: Any) -> None:
47+
"""Error handler for shutil.rmtree to handle read-only files on Windows."""
48+
os.chmod(path, stat.S_IWRITE)
49+
func(path)
50+
51+
52+
def _robust_rmtree(path: str) -> None:
53+
"""Remove a directory tree, handling read-only files on Windows."""
54+
if _IS_WINDOWS:
55+
if sys.version_info >= (3, 12):
56+
shutil.rmtree(path, onexc=lambda fn, p, exc: _on_rm_error(fn, p, None))
57+
else:
58+
shutil.rmtree(path, onerror=_on_rm_error)
59+
else:
60+
shutil.rmtree(path)
61+
62+
4563
def _ensure_agent_engine_dependency(requirements_txt_path: str) -> None:
4664
"""Ensures staged requirements include Agent Platform dependencies."""
4765
if not os.path.exists(requirements_txt_path):
@@ -717,7 +735,7 @@ def to_cloud_run(
717735
# remove temp_folder if exists
718736
if os.path.exists(temp_folder):
719737
click.echo('Removing existing files')
720-
shutil.rmtree(temp_folder)
738+
_robust_rmtree(temp_folder)
721739

722740
try:
723741
# copy agent source code
@@ -839,7 +857,7 @@ def to_cloud_run(
839857
subprocess.run(gcloud_cmd, check=True)
840858
finally:
841859
click.echo(f'Cleaning up the temp folder: {temp_folder}')
842-
shutil.rmtree(temp_folder)
860+
_robust_rmtree(temp_folder)
843861

844862

845863
def _print_agent_engine_url(resource_name: str) -> None:
@@ -997,7 +1015,7 @@ def to_agent_engine(
9971015
temp_folder_path = os.path.join(parent_folder, temp_folder)
9981016
if os.path.exists(temp_folder_path):
9991017
click.echo('Removing existing files')
1000-
shutil.rmtree(temp_folder_path)
1018+
_robust_rmtree(temp_folder_path)
10011019

10021020
try:
10031021
ignore_func = _get_ignore_patterns_func(agent_folder)
@@ -1308,7 +1326,7 @@ def create_dockerfile_for_agent_engine(resource_name: str) -> None:
13081326
temp_folder_path = os.path.join(parent_folder, temp_folder)
13091327
click.echo(f'Cleaning up the temp folder: {temp_folder_path}')
13101328
os.chdir(original_cwd)
1311-
shutil.rmtree(temp_folder_path)
1329+
_robust_rmtree(temp_folder_path)
13121330

13131331

13141332
def to_gke(
@@ -1387,7 +1405,7 @@ def to_gke(
13871405
# remove temp_folder if exists
13881406
if os.path.exists(temp_folder):
13891407
click.echo(' - Removing existing temporary directory...')
1390-
shutil.rmtree(temp_folder)
1408+
_robust_rmtree(temp_folder)
13911409

13921410
try:
13931411
# copy agent source code
@@ -1558,7 +1576,7 @@ def to_gke(
15581576
finally:
15591577
click.secho('\nSTEP 5: Cleaning up...', bold=True)
15601578
click.echo(f' - Removing temporary directory: {temp_folder}')
1561-
shutil.rmtree(temp_folder)
1579+
_robust_rmtree(temp_folder)
15621580
click.secho(
15631581
'\n🎉 Deployment to GKE finished successfully!', fg='cyan', bold=True
15641582
)

tests/unittests/cli/utils/test_cli_deploy.py

Lines changed: 42 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1164,3 +1164,45 @@ def test_to_agent_engine_extra_packages_requirements_txt_is_not_clobbered(
11641164
assert (tmp_dir / "requirements.txt").read_text() == (
11651165
"some-unrelated-package\n"
11661166
)
1167+
1168+
1169+
# _robust_rmtree / _on_rm_error tests
1170+
1171+
1172+
class TestRobustRmtree:
1173+
"""Tests for the _robust_rmtree helper."""
1174+
1175+
def test_removes_directory_tree(self, tmp_path: Path) -> None:
1176+
"""It should remove a normal directory tree."""
1177+
d = tmp_path / "subdir"
1178+
d.mkdir()
1179+
(d / "file.txt").write_text("hello")
1180+
cli_deploy._robust_rmtree(str(d))
1181+
assert not d.exists()
1182+
1183+
def test_removes_readonly_files(self, tmp_path: Path) -> None:
1184+
"""It should remove a tree containing read-only files."""
1185+
import os
1186+
import stat
1187+
1188+
d = tmp_path / "ro_dir"
1189+
d.mkdir()
1190+
ro_file = d / "readonly.txt"
1191+
ro_file.write_text("locked")
1192+
ro_file.chmod(stat.S_IREAD)
1193+
cli_deploy._robust_rmtree(str(d))
1194+
assert not d.exists()
1195+
1196+
def test_on_rm_error_clears_readonly_and_retries(
1197+
self, tmp_path: Path
1198+
) -> None:
1199+
"""_on_rm_error should chmod the file and call the removal function."""
1200+
import os
1201+
import stat
1202+
1203+
ro_file = tmp_path / "locked.txt"
1204+
ro_file.write_text("data")
1205+
ro_file.chmod(stat.S_IREAD)
1206+
1207+
cli_deploy._on_rm_error(os.remove, str(ro_file), None)
1208+
assert not ro_file.exists()

0 commit comments

Comments
 (0)