From ada0ebd47d38e12d73ec4a5897eb2257ff03fd65 Mon Sep 17 00:00:00 2001 From: Aravind Vasudevan Date: Fri, 22 Apr 2022 18:03:18 +0000 Subject: [PATCH] Revert "Break make_encoded_file into two functions" This reverts commit 5e49eda5c480d3bb11eeb6f982397c0ad33dc2e3. Reason for revert: chromiumos rollout fails 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 Change-Id: Ibfb51614f9e19b575b99704aa4ca33c5525ab294 No-Presubmit: true No-Tree-Checks: true No-Try: true Reviewed-on: https://chromium-review.googlesource.com/c/chromium/tools/depot_tools/+/3602168 Auto-Submit: Aravind Vasudevan Commit-Queue: Rubber Stamper Bot-Commit: Rubber Stamper --- 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, 15 insertions(+), 54 deletions(-) diff --git a/recipes/README.recipes.md b/recipes/README.recipes.md index de9b65f26..bff2f5830 100644 --- a/recipes/README.recipes.md +++ b/recipes/README.recipes.md @@ -550,7 +550,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#231)(self, repo_url):** +— **def [canonicalize\_repo\_url](/recipes/recipe_modules/gitiles/api.py#227)(self, repo_url):** Returns a canonical form of repo_url. If not recognized, returns as is. @@ -565,7 +565,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#171)(self, repository_url, destination, revision='refs/heads/main'):** +— **def [download\_archive](/recipes/recipe_modules/gitiles/api.py#167)(self, repository_url, destination, revision='refs/heads/main'):** Downloads an archive of the repo and extracts it to `destination`. @@ -618,7 +618,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#220)(self, repo_url):** +— **def [parse\_repo\_url](/recipes/recipe_modules/gitiles/api.py#216)(self, repo_url):** Returns (host, project) pair. @@ -628,7 +628,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#227)(self, host, project):** +— **def [unparse\_repo\_url](/recipes/recipe_modules/gitiles/api.py#223)(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 e422063d5..4787592c3 100644 --- a/recipes/recipe_modules/gitiles/api.py +++ b/recipes/recipe_modules/gitiles/api.py @@ -158,15 +158,11 @@ 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']) - 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 + if sys.version_info >= (3,): + return value.decode('utf-8') + 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 7f3205803..b61b1bf5c 100644 --- a/recipes/recipe_modules/gitiles/examples/full.expected/basic.json +++ b/recipes/recipe_modules/gitiles/examples/full.expected/basic.json @@ -529,22 +529,6 @@ ], "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 502aa8c18..9a96d80a2 100644 --- a/recipes/recipe_modules/gitiles/examples/full.py +++ b/recipes/recipe_modules/gitiles/examples/full.py @@ -23,10 +23,6 @@ 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]) @@ -77,10 +73,6 @@ 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 06b6f43c2..7256a5bd1 100644 --- a/recipes/recipe_modules/gitiles/test_api.py +++ b/recipes/recipe_modules/gitiles/test_api.py @@ -90,23 +90,12 @@ class GitilesTestApi(recipe_test_api.RecipeTestApi): return hashlib.sha1(':'.join(bases).encode('utf-8')).hexdigest() def make_encoded_file(self, data): - """Encodes data into base64. - - Args: - data (str): unicode-encodable string. - Returns: (str) base64-encoded data string. - """ + 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) return self.m.json.output({ - 'value': base64.b64encode(data.encode('utf-8')).decode('utf-8'), + 'value': value, }) - - 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