Fix ManifestProject.partial_clone_exclude property.

Bug: b/256358360

Change-Id: Ic6e3a049aa38827123d0324c8b14157562c5986e
Reviewed-on: https://gerrit-review.googlesource.com/c/git-repo/+/353574
Reviewed-by: Mike Frysinger <vapier@google.com>
Tested-by: Joanna Wang <jojwang@google.com>
Reviewed-by: Josip Sokcevic <sokcevic@google.com>

3 files changed, 90 insertions, 5 deletions

diff --git a/git_config.py b/git_config.py
index 94378e9a..af1a1015 100644
--- a/git_config.py
+++ b/git_config.py
@@ -22,6 +22,7 @@ import re
 import ssl
 import subprocess
 import sys
+from typing import Union
 import urllib.error
 import urllib.request
 
@@ -117,7 +118,7 @@ class GitConfig(object):
       return self.defaults.Has(name, include_defaults=True)
     return False
 
-  def GetInt(self, name):
+  def GetInt(self, name: str) -> Union[int, None]:
     """Returns an integer from the configuration file.
 
     This follows the git config syntax.
@@ -126,7 +127,7 @@ class GitConfig(object):
       name: The key to lookup.
 
     Returns:
-      None if the value was not defined, or is not a boolean.
+      None if the value was not defined, or is not an int.
       Otherwise, the number itself.
     """
     v = self.GetString(name)
@@ -152,6 +153,9 @@ class GitConfig(object):
     try:
       return int(v, base=base) * mult
     except ValueError:
+      print(
+          f"warning: expected {name} to represent an integer, got {v} instead",
+          file=sys.stderr)
       return None
 
   def DumpConfigDict(self):
@@ -169,7 +173,7 @@ class GitConfig(object):
       config_dict[key] = self.GetString(key)
     return config_dict
 
-  def GetBoolean(self, name):
+  def GetBoolean(self, name: str) -> Union[str, None]:
     """Returns a boolean from the configuration file.
        None : The value was not defined, or is not a boolean.
        True : The value was set to true or yes.
@@ -183,6 +187,8 @@ class GitConfig(object):
       return True
     if v in ('false', 'no'):
       return False
+    print(f"warning: expected {name} to represent a boolean, got {v} instead",
+            file=sys.stderr)
     return None
 
   def SetBoolean(self, name, value):
@@ -191,7 +197,7 @@ class GitConfig(object):
       value = 'true' if value else 'false'
     self.SetString(name, value)
 
-  def GetString(self, name, all_keys=False):
+  def GetString(self, name: str, all_keys: bool = False) -> Union[str,  None]:
     """Get the first value for a key, or None if it is not defined.
 
        This configuration file is used first, if the key is not
diff --git a/project.py b/project.py
index 9d7476b4..619cdfd1 100644
--- a/project.py
+++ b/project.py
@@ -3505,7 +3505,7 @@ class ManifestProject(MetaProject):
   @property
   def partial_clone_exclude(self):
     """Partial clone exclude string"""
-    return self.config.GetBoolean('repo.partialcloneexclude')
+    return self.config.GetString('repo.partialcloneexclude')
 
   @property
   def manifest_platform(self):
diff --git a/tests/test_project.py b/tests/test_project.py
index 47018d1c..7d07b705 100644
--- a/tests/test_project.py
+++ b/tests/test_project.py
@@ -22,6 +22,7 @@ import tempfile
 import unittest
 
 import error
+import manifest_xml
 import git_command
 import git_config
 import platform_utils
@@ -411,3 +412,81 @@ class MigrateWorkTreeTests(unittest.TestCase):
         self.assertTrue((dotgit / name).is_file())
       for name in self._SYMLINKS:
         self.assertTrue((dotgit / name).is_symlink())
+
+
+class ManifestPropertiesFetchedCorrectly(unittest.TestCase):
+  """Ensure properties are fetched properly."""
+
+  def setUpManifest(self, tempdir):
+    repo_trace._TRACE_FILE = os.path.join(tempdir, 'TRACE_FILE_from_test')
+
+    repodir = os.path.join(tempdir, '.repo')
+    manifest_dir = os.path.join(repodir, 'manifests')
+    manifest_file = os.path.join(
+        repodir, manifest_xml.MANIFEST_FILE_NAME)
+    local_manifest_dir = os.path.join(
+        repodir, manifest_xml.LOCAL_MANIFESTS_DIR_NAME)
+    os.mkdir(repodir)
+    os.mkdir(manifest_dir)
+    manifest = manifest_xml.XmlManifest(repodir, manifest_file)
+
+    return project.ManifestProject(
+        manifest, 'test/manifest', os.path.join(tempdir, '.git'), tempdir)
+
+  def test_manifest_config_properties(self):
+    """Test we are fetching the manifest config properties correctly."""
+
+    with TempGitTree() as tempdir:
+      fakeproj = self.setUpManifest(tempdir)
+
+      # Set property using the expected Set method, then ensure
+      # the porperty functions are using the correct Get methods.
+      fakeproj.config.SetString(
+          'manifest.standalone', 'https://chicken/manifest.git')
+      self.assertEqual(
+          fakeproj.standalone_manifest_url, 'https://chicken/manifest.git')
+
+      fakeproj.config.SetString('manifest.groups', 'test-group, admin-group')
+      self.assertEqual(fakeproj.manifest_groups, 'test-group, admin-group')
+
+      fakeproj.config.SetString('repo.reference', 'mirror/ref')
+      self.assertEqual(fakeproj.reference, 'mirror/ref')
+
+      fakeproj.config.SetBoolean('repo.dissociate', False)
+      self.assertFalse(fakeproj.dissociate)
+
+      fakeproj.config.SetBoolean('repo.archive', False)
+      self.assertFalse(fakeproj.archive)
+
+      fakeproj.config.SetBoolean('repo.mirror', False)
+      self.assertFalse(fakeproj.mirror)
+
+      fakeproj.config.SetBoolean('repo.worktree', False)
+      self.assertFalse(fakeproj.use_worktree)
+
+      fakeproj.config.SetBoolean('repo.clonebundle', False)
+      self.assertFalse(fakeproj.clone_bundle)
+
+      fakeproj.config.SetBoolean('repo.submodules', False)
+      self.assertFalse(fakeproj.submodules)
+
+      fakeproj.config.SetBoolean('repo.git-lfs', False)
+      self.assertFalse(fakeproj.git_lfs)
+
+      fakeproj.config.SetBoolean('repo.superproject', False)
+      self.assertFalse(fakeproj.use_superproject)
+
+      fakeproj.config.SetBoolean('repo.partialclone', False)
+      self.assertFalse(fakeproj.partial_clone)
+
+      fakeproj.config.SetString('repo.depth', '48')
+      self.assertEquals(fakeproj.depth, '48')
+
+      fakeproj.config.SetString('repo.clonefilter', 'blob:limit=10M')
+      self.assertEquals(fakeproj.clone_filter, 'blob:limit=10M')
+
+      fakeproj.config.SetString('repo.partialcloneexclude', 'third_party/big_repo')
+      self.assertEquals(fakeproj.partial_clone_exclude, 'third_party/big_repo')
+
+      fakeproj.config.SetString('manifest.platform', 'auto')
+      self.assertEquals(fakeproj.manifest_platform, 'auto')