From ac08e98e5dd6d0764beba24275193004bdc3df2d Mon Sep 17 00:00:00 2001
From: Alex Lowe <alex.lowe@canonical.com>
Date: Mon, 22 Jul 2024 14:04:35 -0400
Subject: [PATCH] fix(zip): include linked directories in created zips (#1745)

Makes `build_zip` use the Charmcraft 2.x behaviour of following symlinks
even for directories:

https://github.com/canonical/charmcraft/blob/2.7.1/charmcraft/package.py#L440

This ensures that not only symlinked files, but also symlinked dirs are
included in the resulting charm or bundle.

Fixes #1719
---
 charmcraft/utils/file.py      | 11 ++++----
 tests/unit/utils/test_file.py | 53 +++++++++++++++++++----------------
 2 files changed, 35 insertions(+), 29 deletions(-)

diff --git a/charmcraft/utils/file.py b/charmcraft/utils/file.py
index cb4d5be38..a88c73936 100644
--- a/charmcraft/utils/file.py
+++ b/charmcraft/utils/file.py
@@ -63,8 +63,9 @@ def build_zip(zip_path: PathOrString, prime_dir: PathOrString) -> None:
     """
     zip_path = pathlib.Path(zip_path).resolve()
     prime_dir = pathlib.Path(prime_dir).resolve()
-    with zipfile.ZipFile(zip_path, "w", zipfile.ZIP_DEFLATED) as file:
-        for file_path in prime_dir.rglob("*"):
-            if not file_path.is_file():
-                continue
-            file.write(file_path, file_path.relative_to(prime_dir))
+    with zipfile.ZipFile(zip_path, "w", zipfile.ZIP_DEFLATED) as zip_file:
+        # Using os.walk() because Path.walk() is only added in 3.12
+        for dir_path_str, _, filenames in os.walk(prime_dir, followlinks=True):
+            for filename in filenames:
+                file_path = pathlib.Path(dir_path_str, filename)
+                zip_file.write(file_path, file_path.relative_to(prime_dir))
diff --git a/tests/unit/utils/test_file.py b/tests/unit/utils/test_file.py
index e13c2de0f..04efda2cc 100644
--- a/tests/unit/utils/test_file.py
+++ b/tests/unit/utils/test_file.py
@@ -105,41 +105,46 @@ def test_zipbuild_simple(tmp_path):
 
 
 @pytest.mark.skipif(sys.platform == "win32", reason="Windows not [yet] supported")
-def test_zipbuild_symlink_simple(tmp_path):
+def test_zipbuild_symlinks(tmp_path: pathlib.Path):
     """Symlinks are supported."""
     build_dir = tmp_path / "somedir"
     build_dir.mkdir()
 
-    testfile1 = build_dir / "real.txt"
-    testfile1.write_bytes(b"123\x00456")
-    testfile2 = build_dir / "link.txt"
-    testfile2.symlink_to(testfile1)
+    outside_dir = tmp_path / "another_dir"
+    outside_dir.mkdir()
+    outside_file = outside_dir / "some_file"
+    outside_file.write_bytes(b"123\x00456")
 
-    zip_filepath = tmp_path / "testresult.zip"
-    build_zip(zip_filepath, build_dir)
+    internal_dir = build_dir / "subdirectory"
+    internal_dir.mkdir()
+    real_file = internal_dir / "real.txt"
+    real_file.write_bytes(b"123\x00456")
 
-    zf = zipfile.ZipFile(zip_filepath)
-    assert sorted(x.filename for x in zf.infolist()) == ["link.txt", "real.txt"]
-    assert zf.read("real.txt") == b"123\x00456"
-    assert zf.read("link.txt") == b"123\x00456"
+    internal_file_link = build_dir / "link.txt"
+    internal_file_link.symlink_to(real_file)
 
+    internal_dir_link = build_dir / "link_dir"
+    internal_dir_link.symlink_to(internal_dir)
 
-@pytest.mark.skipif(sys.platform == "win32", reason="Windows not [yet] supported")
-def test_zipbuild_symlink_outside(tmp_path):
-    """No matter where the symlink points to."""
-    # outside the build dir
-    testfile1 = tmp_path / "real.txt"
-    testfile1.write_bytes(b"123\x00456")
+    external_file_link = build_dir / "external_link.txt"
+    external_file_link.symlink_to(outside_file)
 
-    # inside the build dir
-    build_dir = tmp_path / "somedir"
-    build_dir.mkdir()
-    testfile2 = build_dir / "link.txt"
-    testfile2.symlink_to(testfile1)
+    external_dir_link = build_dir / "external_link_dir"
+    external_dir_link.symlink_to(outside_dir)
 
     zip_filepath = tmp_path / "testresult.zip"
     build_zip(zip_filepath, build_dir)
 
     zf = zipfile.ZipFile(zip_filepath)
-    assert sorted(x.filename for x in zf.infolist()) == ["link.txt"]
-    assert zf.read("link.txt") == b"123\x00456"
+
+    expected_files = [
+        "external_link.txt",
+        "external_link_dir/some_file",
+        "link.txt",
+        "link_dir/real.txt",
+        "subdirectory/real.txt",
+    ]
+
+    assert sorted(x.filename for x in zf.infolist()) == expected_files
+    for file_name in expected_files:
+        assert zf.read(file_name) == b"123\x00456"