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 <gavinmak@google.com>
Reviewed-by: Greg Edelston <gredelston@google.com>
Commit-Queue: Aravind Vasudevan <aravindvasudev@google.com>

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#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.
     
@@ -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#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`.
 
@@ -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#216)(self, repo_url):**
+— **def [parse\_repo\_url](/recipes/recipe_modules/gitiles/api.py#220)(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#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)

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,):
+    try:
-      return value.decode('utf-8')
+      # TODO(crbug.com/1227140): Clean up when py2 is no longer supported.
-    return value
+      # 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'):

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",

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})

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
+    """Encodes data into base64.
-    # TODO(crbug.com/1227140): Clean up when py2 is no longer supported.
+
-    try:
+    Args:
-      value = base64.b64encode(data.encode('utf-8')).decode('utf-8')
+      data (str): unicode-encodable string.
-    except UnicodeDecodeError:  #pragma: nocover
+    Returns: (str) base64-encoded data string.
-      value = base64.b64encode(data)
+    """
+    return self.m.json.output({
+        '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': value,
+        'value': base64.b64encode(data).decode('utf-8'),
     })