Bug 1968280 - Draft: Landoscript should fetch file modes for newly added or files being removed.

landoscript/src/landoscript/actions/android_l10n_import.py

@@ -47,7 +47,7 @@ async def run(
     async with GithubClient(github_config, l10n_owner, l10n_repo) as l10n_github_client:
         toml_files = [info.toml_path for info in android_l10n_import_info.toml_info]
         # we always take the tip of the default branch when importing new strings
-        toml_contents = await l10n_github_client.get_files(toml_files)
+        toml_contents = await l10n_github_client.get_files(toml_files, mode=None)
         l10n_files: list[L10nFile] = []
 
         missing = [fn for fn, contents in toml_contents.items() if contents is None]
@@ -75,7 +75,7 @@ async def run(
         # fetch l10n_files from android-l10n
         src_files = [f.src_name for f in l10n_files]
         log.info(f"fetching updated files from l10n repository: {src_files}")
-        new_files = await l10n_github_client.get_files(src_files)
+        new_files = await l10n_github_client.get_files(src_files, mode=None)
         # we also need to update the toml files with locale metadata. we've
         # already fetched them above; so just add their contents by hand
         new_files.update(toml_contents)
@@ -88,7 +88,7 @@ async def run(
             dst_files.append(f"{toml_info.dest_path}/l10n.toml")
 
         log.info(f"fetching original files from l10n repository: {dst_files}")
-        orig_files = await github_client.get_files(dst_files, branch=to_branch)
+        orig_files = await github_client.get_files(dst_files, branch=to_branch, mode=None)
 
         files_to_diff = []
         for l10n_file in l10n_files:

landoscript/src/landoscript/actions/android_l10n_sync.py

@@ -39,7 +39,7 @@ async def run(github_client: GithubClient, public_artifact_dir: str, android_l10
     from_branch = android_l10n_sync_info.from_branch
 
     toml_files = [info.toml_path for info in android_l10n_sync_info.toml_info]
-    toml_contents = await github_client.get_files(toml_files, branch=from_branch)
+    toml_contents = await github_client.get_files(toml_files, branch=from_branch, mode=None)
     l10n_files: list[L10nFile] = []
 
     missing = [fn for fn, contents in toml_contents.items() if contents is None]
@@ -67,7 +67,7 @@ async def run(github_client: GithubClient, public_artifact_dir: str, android_l10
     # fetch l10n_files from `from_branch` in the gecko repo
     src_files = [f.src_name for f in l10n_files]
     log.info(f"fetching updated files from l10n repository: {src_files}")
-    new_files = await github_client.get_files(src_files, branch=from_branch)
+    new_files = await github_client.get_files(src_files, branch=from_branch, mode=None)
     # we also need to update the toml files with locale metadata. we've
     # already fetched them above; so just add their contents by hand
     new_files.update(toml_contents)
@@ -80,7 +80,7 @@ async def run(github_client: GithubClient, public_artifact_dir: str, android_l10
         dst_files.append(toml_info.toml_path)
 
     log.info(f"fetching original files from l10n repository: {dst_files}")
-    orig_files = await github_client.get_files(dst_files, branch=to_branch)
+    orig_files = await github_client.get_files(dst_files, branch=to_branch, mode=None)
 
     files_to_diff = []
     for l10n_file in l10n_files:

landoscript/src/landoscript/actions/l10n_bump.py

@@ -66,7 +66,7 @@ async def run(
             files = [bump_config.path, *platform_config_files]
             try:
                 log.info(f"fetching bump files from github: {files}")
-                orig_files = await github_client.get_files(files, branch)
+                orig_files = await github_client.get_files(files, branch, mode=None)
             except TransportError as e:
                 raise LandoscriptError("couldn't retrieve bump files from github") from e
 

landoscript/src/landoscript/actions/merge_day.py

@@ -161,7 +161,7 @@ async def run(
         for r in regex_replacements:
             needed_files.append(r[0])
 
-        orig_contents = await github_client.get_files(needed_files, bump_branch)
+        orig_contents = await github_client.get_files(needed_files, bump_branch, mode=None)
         # At the moment, there are no known cases of needing to replace with
         # a suffix...so we simply don't handle that here!
         new_contents = process_replacements(bump_version, replacements, regex_replacements, orig_contents)
@@ -171,7 +171,7 @@ async def run(
             files_to_diff.append((fn, str(orig_contents[fn]), new_contents[fn]))
 
     log.info("Touching clobber file")
-    orig_clobber_file = (await github_client.get_files("CLOBBER", bump_branch))["CLOBBER"]
+    orig_clobber_file = (await github_client.get_files("CLOBBER", bump_branch, mode=None))["CLOBBER"]
     if orig_clobber_file is None:
         raise LandoscriptError("Couldn't find CLOBBER file in repository!")
 
@@ -206,7 +206,7 @@ async def run(
 
 
 async def get_version(github_client: GithubClient, version_file: str, branch: str):
-    resp = await github_client.get_files(version_file, branch)
+    resp = await github_client.get_files(version_file, branch, mode=None)
     contents = resp[version_file]
     if contents is None:
         raise LandoscriptError(f"Couldn't find {version_file} in repository!")

landoscript/src/landoscript/actions/version_bump.py

@@ -60,7 +60,7 @@ async def run(
 
         try:
             log.info("fetching bump files from github")
-            orig_files = await github_client.get_files(version_bump_info.files, branch)
+            orig_files = await github_client.get_files(version_bump_info.files, branch, mode=None)
         except TransportError as e:
             raise LandoscriptError("couldn't retrieve bump files from github") from e
 

landoscript/src/landoscript/util/diffs.py

@@ -3,7 +3,7 @@
 NO_NEWLINE_SUFFIX = "\\ No newline at end of file"
 
 
-def diff_contents(orig, modified, file):
+def diff_contents(orig, modified, file, mode=None):
     """Create a git-style unified diff of `orig` and `modified` with the filename `file`."""
     add_remove_metadata = ""
     if orig:
@@ -14,7 +14,7 @@ def diff_contents(orig, modified, file):
         # orig does not exist yet; ie: it will be added
         orig_contents = ""
         fromfile = "/dev/null"
-        add_remove_metadata = "new file mode 100644\n"
+        add_remove_metadata = f"new file mode {mode or 100644}\n"
     if modified:
         # modified exists already
         modified_contents = modified.splitlines()
@@ -23,7 +23,7 @@ def diff_contents(orig, modified, file):
         # modified does not exist yet; ie: it will be added
         modified_contents = ""
         tofile = "/dev/null"
-        add_remove_metadata = "deleted file mode 100644\n"
+        add_remove_metadata = f"deleted file mode {mode or 100644}\n"
 
     diff_lines = [line for line in unified_diff(orig_contents, modified_contents, fromfile=fromfile, tofile=tofile, lineterm="")]
     if not diff_lines:

landoscript/tests/conftest.py

@@ -219,7 +219,7 @@ def assert_tag_response(req, tag_info, target_revision):
     assert revisions.pop() == target_revision
 
 
-def assert_add_commit_response(action, commit_msg_strings, initial_values, expected_bumps):
+def assert_add_commit_response(action, commit_msg_strings, initial_values, expected_bumps, mode=None):
     # ensure metadata is correct
     assert action["author"] == "Release Engineering Landoscript <[email protected]>"
     # we don't actually verify the value here; it's not worth the trouble of mocking
@@ -261,12 +261,12 @@ def assert_add_commit_response(action, commit_msg_strings, initial_values, expec
                 # the diff.
                 if not before:
                     # addition
-                    assert "new file mode 100644" in diff
+                    assert f"new file mode {mode or 100644}" in diff
                     if "\n" not in after and f"+{after}" in diff:
                         break
                 elif not after:
                     # removal
-                    assert "deleted file mode 100644" in diff
+                    assert f"deleted file mode {mode or 100644}" in diff
                     if not "\n" in before and f"-{before}" in diff:
                         break
                 else:

scriptworker_client/packages/pytest-scriptworker-client/src/pytest_scriptworker_client/__init__.py

@@ -58,6 +58,9 @@ def get_files_payload(file_contents={}):
             hash_ = "a" + hashlib.md5(file.encode()).hexdigest()
             if contents is None:
                 payload["data"]["repository"][hash_] = None
+            elif isinstance(contents, dict):
+                payload["data"]["repository"][hash_] = {"mode": contents.get("mode"), "text": contents.get("text")}
+
             else:
                 payload["data"]["repository"][hash_] = {"text": contents}
     else:

scriptworker_client/src/scriptworker_client/github_client.py

@@ -93,7 +93,9 @@ async def _execute():
 
         await retry_async(_execute, attempts=3, retry_exceptions=(TransportQueryError,), sleeptime_kwargs={"delay_factor": 0})
 
-    async def get_files(self, files: Union[str, List[str]], branch: Optional[str] = None, files_per_request: int = 200) -> Dict[str, Union[str, None]]:
+    async def get_files(
+        self, files: Union[str, List[str]], branch: Optional[str] = None, files_per_request: int = 200, mode: Optional[str] = None
+    ) -> Dict[str, Union[str, Dict[str, Optional[str]]]]:
         """Get the contents of the specified files.
 
         Args:
@@ -127,10 +129,18 @@ async def get_files(self, files: Union[str, List[str]], branch: Optional[str] =
             dedent(
                 """
             $name: object(expression: "$branch:$file") {
-              ... on Blob {
-                text
-              }
+              ... on Tree {
+              entries {
+                    name
+                    mode
+                    object {
+                        ... on Blob {
+                            text
+                        }
+                    }
+                }
             }
+        }
             """
             )
         )
@@ -140,7 +150,7 @@ async def get_files(self, files: Union[str, List[str]], branch: Optional[str] =
         # file paths to their hashes, and use the hashes as the key names
         # in the query.
         aliases = {}
-        ret: Dict[str, Union[str, None]] = {}
+        ret: Dict[str, Union[str, Dict[str, Optional[str]]]] = {}
 
         # yields the starting index for each batch
         for i in range(0, len(files), files_per_request):
@@ -165,7 +175,10 @@ async def get_files(self, files: Union[str, List[str]], branch: Optional[str] =
                 if v is None:
                     ret[name] = None
                 else:
-                    ret[name] = v["text"]
+                    if mode:
+                        ret[name] = {"mode": v.get("mode"), "text": v.get("text")}
+                    else:
+                        ret[name] = v.get("text")
 
         return ret
 

scriptworker_client/tests/test_github_client.py

@@ -109,7 +109,7 @@ async def test_get_files(aioresponses, github_client):
 
     aioresponses.post(GITHUB_GRAPHQL_ENDPOINT, status=200, payload={"data": {"repository": {aliases[k]: {"text": v} for k, v in expected.items()}}})
 
-    result = await github_client.get_files(files, branch)
+    result = await github_client.get_files(files, branch, mode=None)
     assert result == expected
 
     aioresponses.assert_called()
@@ -123,13 +123,29 @@ async def test_get_files(aioresponses, github_client):
             query getFileContents {{
               repository(owner: "{github_client.owner}", name: "{github_client.repo}") {{
                 {aliases[str(files[0])]}: object(expression: "{branch}:{files[0]}") {{
-                  ... on Blob {{
-                    text
+                  ... on Tree {{
+                    entries {{
+                      name
+                      mode
+                      object {{
+                        ... on Blob {{
+                          text
+                        }}
+                      }}
+                    }}
                   }}
                 }}
                 {aliases[str(files[1])]}: object(expression: "{branch}:{files[1]}") {{
-                  ... on Blob {{
-                    text
+                  ... on Tree {{
+                    entries {{
+                      name
+                      mode
+                      object {{
+                        ... on Blob {{
+                          text
+                        }}
+                      }}
+                    }}
                   }}
                 }}
               }}
@@ -152,7 +168,7 @@ async def test_get_files_multiple_requests(aioresponses, github_client):
     aioresponses.post(GITHUB_GRAPHQL_ENDPOINT, status=200, payload={"data": {"repository": {aliases["README.md"]: {"text": expected["README.md"]}}}})
     aioresponses.post(GITHUB_GRAPHQL_ENDPOINT, status=200, payload={"data": {"repository": {aliases["version.txt"]: {"text": expected["version.txt"]}}}})
 
-    result = await github_client.get_files(files, branch, files_per_request=1)
+    result = await github_client.get_files(files, branch, files_per_request=1, mode=None)
     assert result == expected
 
     aioresponses.assert_called()
@@ -167,8 +183,16 @@ async def test_get_files_multiple_requests(aioresponses, github_client):
             query getFileContents {{
               repository(owner: "{github_client.owner}", name: "{github_client.repo}") {{
                 {aliases["README.md"]}: object(expression: "{branch}:README.md") {{
-                  ... on Blob {{
-                    text
+                  ... on Tree {{
+                    entries {{
+                      name
+                      mode
+                      object {{
+                        ... on Blob {{
+                          text
+                        }}
+                      }}
+                    }}
                   }}
                 }}
               }}
@@ -182,8 +206,16 @@ async def test_get_files_multiple_requests(aioresponses, github_client):
             query getFileContents {{
               repository(owner: "{github_client.owner}", name: "{github_client.repo}") {{
                 {aliases["version.txt"]}: object(expression: "{branch}:version.txt") {{
-                  ... on Blob {{
-                    text
+                  ... on Tree {{
+                    entries {{
+                      name
+                      mode
+                      object {{
+                        ... on Blob {{
+                          text
+                        }}
+                      }}
+                    }}
                   }}
                 }}
               }}
@@ -210,7 +242,7 @@ async def test_get_files_with_missing(aioresponses, github_client):
         payload={"data": {"repository": {aliases["README.md"]: {"text": "Hello!"}, aliases["version.txt"]: {"text": "109.1.0"}, aliases["missing.txt"]: None}}},
     )
 
-    result = await github_client.get_files(files, branch)
+    result = await github_client.get_files(files, branch, mode=None)
     assert result == expected
 
     aioresponses.assert_called()
@@ -224,18 +256,42 @@ async def test_get_files_with_missing(aioresponses, github_client):
             query getFileContents {{
               repository(owner: "{github_client.owner}", name: "{github_client.repo}") {{
                 {aliases[str(files[0])]}: object(expression: "{branch}:{files[0]}") {{
-                  ... on Blob {{
-                    text
+                  ... on Tree {{
+                    entries {{
+                      name
+                      mode
+                      object {{
+                        ... on Blob {{
+                          text
+                        }}
+                      }}
+                    }}
                   }}
                 }}
                 {aliases[str(files[1])]}: object(expression: "{branch}:{files[1]}") {{
-                  ... on Blob {{
-                    text
+                  ... on Tree {{
+                    entries {{
+                      name
+                      mode
+                      object {{
+                        ... on Blob {{
+                          text
+                        }}
+                      }}
+                    }}
                   }}
                 }}
                 {aliases[str(files[2])]}: object(expression: "{branch}:{files[2]}") {{
-                  ... on Blob {{
-                    text
+                  ... on Tree {{
+                    entries {{
+                      name
+                      mode
+                      object {{
+                        ... on Blob {{
+                          text
+                        }}
+                      }}
+                    }}
                   }}
                 }}
               }}

treescript/src/treescript/github/versionmanip.py

@@ -125,7 +125,7 @@ async def do_bump_version(client: GithubClient, files: List[str], next_version:
     """
     changes = {}
     diff = []
-    file_contents = await client.get_files(files, branch=branch)
+    file_contents = await client.get_files(files, branch=branch, mode=None)
     for file_ in files:
         if file_ not in ALLOWED_BUMP_FILES:
             raise TaskVerificationError("{} is not in version bump whitelist".format(file_))