From 23ddab223518bcc9ad953686628247e4d7a0bd6e Mon Sep 17 00:00:00 2001 From: Aravind Vasudevan Date: Wed, 4 May 2022 17:39:59 +0000 Subject: [PATCH] Reland "Break make_encoded_file into two functions" This is a reland of commit 5e49eda5c480d3bb11eeb6f982397c0ad33dc2e3 Original change's description: > Break make_encoded_file into two functions > > The python3 fallback for `make_encoded_file()` produces different results on py2 and py3 when bytes are passed. This CL makes it clear breaking it into two methods, one for text and another for bytes. This CL also adds a fallback in `download_file` for py3 to return bytes when it cannot decode the downloaded file into utf-8. > > Recipe-Manual-Change: chromiumos > Change-Id: I3d313e430c852e179825bc24bf4a58ce84440b2a > Reviewed-on: https://chromium-review.googlesource.com/c/chromium/tools/depot_tools/+/3595026 > Reviewed-by: Gavin Mak > Reviewed-by: Greg Edelston > Commit-Queue: Aravind Vasudevan Recipe-Manual-Change: chromiumos Change-Id: I321f797ae4a00deed7920ee6f80d666954b07c7d Reviewed-on: https://chromium-review.googlesource.com/c/chromium/tools/depot_tools/+/3625907 Reviewed-by: Greg Edelston Commit-Queue: Aravind Vasudevan --- recipes/README.recipes.md | 8 +++--- recipes/recipe_modules/gitiles/api.py | 12 ++++++--- .../gitiles/examples/full.expected/basic.json | 16 ++++++++++++ .../recipe_modules/gitiles/examples/full.py | 8 ++++++ recipes/recipe_modules/gitiles/test_api.py | 25 +++++++++++++------ 5 files changed, 54 insertions(+), 15 deletions(-) diff --git a/recipes/README.recipes.md b/recipes/README.recipes.md index fdfe7c991..f4b54861b 100644 --- a/recipes/README.recipes.md +++ b/recipes/README.recipes.md @@ -553,7 +553,7 @@ PYTHON_VERSION_COMPATIBILITY: PY2+3 Module for polling a git repository using the Gitiles web interface. -— **def [canonicalize\_repo\_url](/recipes/recipe_modules/gitiles/api.py#227)(self, repo_url):** +— **def [canonicalize\_repo\_url](/recipes/recipe_modules/gitiles/api.py#231)(self, repo_url):** Returns a canonical form of repo_url. If not recognized, returns as is. @@ -568,7 +568,7 @@ Args: * step_name (str): If not None, override the step name. * attempts (int): Number of times to try the request before failing. -— **def [download\_archive](/recipes/recipe_modules/gitiles/api.py#167)(self, repository_url, destination, revision='refs/heads/main'):** +— **def [download\_archive](/recipes/recipe_modules/gitiles/api.py#171)(self, repository_url, destination, revision='refs/heads/main'):** Downloads an archive of the repo and extracts it to `destination`. @@ -621,7 +621,7 @@ Returns: Cursor can be used for subsequent calls to log for paging. If None, signals that there are no more commits to fetch. -— **def [parse\_repo\_url](/recipes/recipe_modules/gitiles/api.py#216)(self, repo_url):** +— **def [parse\_repo\_url](/recipes/recipe_modules/gitiles/api.py#220)(self, repo_url):** Returns (host, project) pair. @@ -631,7 +631,7 @@ Returns (None, None) if repo_url is not recognized. Returns a list of refs in the remote repository. -— **def [unparse\_repo\_url](/recipes/recipe_modules/gitiles/api.py#223)(self, host, project):** +— **def [unparse\_repo\_url](/recipes/recipe_modules/gitiles/api.py#227)(self, host, project):** Generates a Gitiles repo URL. See also parse_repo_url. ### *recipe_modules* / [gsutil](/recipes/recipe_modules/gsutil) diff --git a/recipes/recipe_modules/gitiles/api.py b/recipes/recipe_modules/gitiles/api.py index 4787592c3..e422063d5 100644 --- a/recipes/recipe_modules/gitiles/api.py +++ b/recipes/recipe_modules/gitiles/api.py @@ -158,11 +158,15 @@ class Gitiles(recipe_api.RecipeApi): **kwargs) if step_result.json.output['value'] is None: return None - # TODO(crbug.com/1227140): Clean up when py2 is no longer supported. + value = base64.b64decode(step_result.json.output['value']) - if sys.version_info >= (3,): - return value.decode('utf-8') - return value + try: + # TODO(crbug.com/1227140): Clean up when py2 is no longer supported. + # If the file is not utf-8 encodable, return the bytes + if sys.version_info >= (3,): + value = value.decode('utf-8') + finally: + return value def download_archive(self, repository_url, destination, revision='refs/heads/main'): diff --git a/recipes/recipe_modules/gitiles/examples/full.expected/basic.json b/recipes/recipe_modules/gitiles/examples/full.expected/basic.json index b61b1bf5c..7f3205803 100644 --- a/recipes/recipe_modules/gitiles/examples/full.expected/basic.json +++ b/recipes/recipe_modules/gitiles/examples/full.expected/basic.json @@ -529,6 +529,22 @@ ], "name": "fetch main:OWNERS" }, + { + "cmd": [ + "vpython3", + "-u", + "RECIPE_MODULE[depot_tools::gitiles]/resources/gerrit_client.py", + "--json-file", + "/path/to/tmp/json", + "--url", + "https://chromium.googlesource.com/chromium/src/+/main/BYTES", + "--format", + "text", + "--attempts", + "5" + ], + "name": "fetch main:BYTES" + }, { "cmd": [ "vpython3", diff --git a/recipes/recipe_modules/gitiles/examples/full.py b/recipes/recipe_modules/gitiles/examples/full.py index 9a96d80a2..502aa8c18 100644 --- a/recipes/recipe_modules/gitiles/examples/full.py +++ b/recipes/recipe_modules/gitiles/examples/full.py @@ -23,6 +23,10 @@ def RunSteps(api): data = api.gitiles.download_file(url, 'OWNERS', attempts=5) assert data == 'foobar' + + data = api.gitiles.download_file(url, 'BYTES', attempts=5) + assert data == b'\xab' + data = api.gitiles.download_file(url, 'NONEXISTENT', attempts=1, accept_statuses=[404]) @@ -73,6 +77,10 @@ def GenTests(api): 'fetch main:OWNERS', api.gitiles.make_encoded_file('foobar') ) + + api.step_data( + 'fetch main:BYTES', + api.gitiles.make_encoded_file_from_bytes(b'\xab') + ) + api.step_data( 'fetch main:NONEXISTENT', api.json.output({'value': None}) diff --git a/recipes/recipe_modules/gitiles/test_api.py b/recipes/recipe_modules/gitiles/test_api.py index 7256a5bd1..06b6f43c2 100644 --- a/recipes/recipe_modules/gitiles/test_api.py +++ b/recipes/recipe_modules/gitiles/test_api.py @@ -90,12 +90,23 @@ class GitilesTestApi(recipe_test_api.RecipeTestApi): return hashlib.sha1(':'.join(bases).encode('utf-8')).hexdigest() def make_encoded_file(self, data): - value = None - # TODO(crbug.com/1227140): Clean up when py2 is no longer supported. - try: - value = base64.b64encode(data.encode('utf-8')).decode('utf-8') - except UnicodeDecodeError: #pragma: nocover - value = base64.b64encode(data) + """Encodes data into base64. + + Args: + data (str): unicode-encodable string. + Returns: (str) base64-encoded data string. + """ return self.m.json.output({ - 'value': value, + 'value': base64.b64encode(data.encode('utf-8')).decode('utf-8'), }) + + def make_encoded_file_from_bytes(self, data): + """Encodes data into base64. + + Args: + data (bytes): byte string to encode. + Returns: (str) base64-encoded data string. + """ + return self.m.json.output({ + 'value': base64.b64encode(data).decode('utf-8'), + }) \ No newline at end of file