Merge ~nacc/git-ubuntu:bugfixes-after-refactor into git-ubuntu:master

[Nish Aravamudan]

[Server Team CI bot (server-team-bot)]

FAILED: Continuous integration, rev:3f2aa06526fe716e980ac3766abf673dbfda84b5
No commit message was specified in the merge proposal. Click on the following link and set the commit message (if you want a jenkins rebuild you need to trigger it yourself):
https://code.launchpad.net/~nacc/usd-importer/+git/usd-importer/+merge/330614/+edit-commit-message

https://jenkins.ubuntu.com/server/job/git-ubuntu-ci/65/
Executed test runs:
    SUCCESS: Checkout
    SUCCESS: Style Check
    FAILED: Unit Tests

Click here to trigger a rebuild:
https://jenkins.ubuntu.com/server/job/git-ubuntu-ci/65/rebuild

review: Needs Fixing (continuous-integration)

[Server Team CI bot (server-team-bot)]

FAILED: Continuous integration, rev:1c85b9925c2ac4104df77230a81d8d54e98ac16d
No commit message was specified in the merge proposal. Click on the following link and set the commit message (if you want a jenkins rebuild you need to trigger it yourself):
https://code.launchpad.net/~nacc/usd-importer/+git/usd-importer/+merge/330614/+edit-commit-message

https://jenkins.ubuntu.com/server/job/git-ubuntu-ci/67/
Executed test runs:
    SUCCESS: Checkout
    FAILED: Style Check

Click here to trigger a rebuild:
https://jenkins.ubuntu.com/server/job/git-ubuntu-ci/67/rebuild

review: Needs Fixing (continuous-integration)

[Server Team CI bot (server-team-bot)]

FAILED: Continuous integration, rev:05e459aa4164129a398899d605f9e94a0c3e02ba
No commit message was specified in the merge proposal. Click on the following link and set the commit message (if you want a jenkins rebuild you need to trigger it yourself):
https://code.launchpad.net/~nacc/usd-importer/+git/usd-importer/+merge/330614/+edit-commit-message

https://jenkins.ubuntu.com/server/job/git-ubuntu-ci/69/
Executed test runs:
    SUCCESS: Checkout
    SUCCESS: Style Check
    SUCCESS: Unit Tests
    FAILED: Integration Tests

Click here to trigger a rebuild:
https://jenkins.ubuntu.com/server/job/git-ubuntu-ci/69/rebuild

review: Needs Fixing (continuous-integration)

[Server Team CI bot (server-team-bot)]

FAILED: Continuous integration, rev:dce62ca00595cee0d8f285f0353a33bfb8518199
No commit message was specified in the merge proposal. Click on the following link and set the commit message (if you want a jenkins rebuild you need to trigger it yourself):
https://code.launchpad.net/~nacc/usd-importer/+git/usd-importer/+merge/330614/+edit-commit-message

https://jenkins.ubuntu.com/server/job/git-ubuntu-ci/71/
Executed test runs:
    SUCCESS: Checkout
    SUCCESS: Style Check
    SUCCESS: Unit Tests
    SUCCESS: Integration Tests
    IN_PROGRESS: Declarative: Post Actions

Click here to trigger a rebuild:
https://jenkins.ubuntu.com/server/job/git-ubuntu-ci/71/rebuild

review: Needs Fixing (continuous-integration)

[Andreas Hasenack (ahasenack)]

+1 with just an optional suggestion. I confess that this module is a bit hard to follow, and I understand that versioning has tons of rules and is complex. Glad that it has tests and that they pass :) (tested on artful)

review: Approve

diff --git a/gitubuntu/git_repository.py b/gitubuntu/git_repository.py
index 9fdf7bb..7ed7bdb 100644
--- a/gitubuntu/git_repository.py
+++ b/gitubuntu/git_repository.py
@@ -16,6 +16,7 @@ from subprocess import CalledProcessError
 import sys
 import tempfile
 import time
+from gitubuntu.__main__ import top_level_defaults
 from gitubuntu.dsc import component_tarball_matches
 from gitubuntu.run import run, runq, decode_binary
 try:
@@ -426,9 +427,14 @@ class GitUbuntuRepository:
 
     To access the underlying pygit2.Repository object, use the raw_repo
     property.
+
     """
 
     def __init__(self, local_dir, lp_user=None, fetch_proto=None):
+        """
+        If fetch_proto is None, the default value from
+        gitubuntu.__main__ will be used (top_level_defaults.proto).
+        """
         if local_dir is None:
             self._local_dir = tempfile.mkdtemp()
         else:
@@ -465,6 +471,9 @@ class GitUbuntuRepository:
                 logging.error("Unable to determine Launchpad user")
                 sys.exit(1)
 
+        if fetch_proto is None:
+            fetch_proto = top_level_defaults.proto
+
         self._fetch_proto = fetch_proto
 
     def create_orphan_branch(self, branch_name, msg):
@@ -764,51 +773,65 @@ class GitUbuntuRepository:
         # self.git_run(['config', 'url.%s.insteadof' % self.lp_user, 'lpme'])
 
     def fetch_remote(self, remote_name, verbose=False):
+        # Does not seem to be working with https
+        # https://github.com/libgit2/pygit2/issues/573
+        # https://github.com/libgit2/libgit2/issues/3786
+        # self.raw_repo.remotes[remote_name].fetch()
+        kwargs = {}
+        kwargs['verbose_on_failure'] = True
+        if verbose:
+            # If we are redirecting stdout/stderr to the console, we
+            # do not need to have run() also emit it
+            kwargs['verbose_on_failure'] = False
+            kwargs['stdout'] = None
+            kwargs['stderr'] = None
         try:
-            # Does not seem to be working with https
-            # https://github.com/libgit2/pygit2/issues/573
-            # https://github.com/libgit2/libgit2/issues/3786
-            # self.raw_repo.remotes[remote_name].fetch()
-            logging.debug('Fetching remote %s' % remote_name)
-            kwargs = {}
-            kwargs['verbose_on_failure'] = True
-            if verbose:
-                # If we are redirecting stdout/stderr to the console, we
-                # do not need to have run() also emit it
-                kwargs['verbose_on_failure'] = False
-                kwargs['stdout'] = None
-                kwargs['stderr'] = None
-            self.git_run(['fetch', remote_name], **kwargs)
+            logging.debug("Fetching remote %s", remote_name)
+            cp = self.git_run(
+                args=['fetch', remote_name],
+                env={'GIT_TERMINAL_PROMPT': '0',},
+                **kwargs
+            )
         except CalledProcessError:
             raise GitUbuntuRepositoryFetchError(
-                'Unable to fetch remote %s' % remote_name
+                "Unable to fetch remote %s" % remote_name
             )
 
     def fetch_base_remotes(self, verbose=False):
         self.fetch_remote(remote_name='pkg', verbose=verbose)
 
     def fetch_remote_refspecs(self, remote_name, refspecs, verbose=False):
-        try:
-            # Does not seem to be working with https
-            # https://github.com/libgit2/pygit2/issues/573
-            # https://github.com/libgit2/libgit2/issues/3786
-            # self.raw_repo.remotes[remote_name].fetch()
-            for refspec in refspecs:
-                logging.debug('Fetching refspec %s from remote %s' %
-                             (refspec, remote_name))
-                kwargs = {}
-                kwargs['verbose_on_failure'] = True
-                if verbose:
-                    # If we are redirecting stdout/stderr to the console, we
-                    # do not need to have run() also emit it
-                    kwargs['verbose_on_failure'] = False
-                    kwargs['stdout'] = None
-                    kwargs['stderr'] = None
-                self.git_run(['fetch', remote_name, refspec], **kwargs)
-        except CalledProcessError:
-            raise GitUbuntuRepositoryFetchError(
-                'Unable to fetch %s from remote %s' % (refspecs, remote_name)
-            )
+        # Does not seem to be working with https
+        # https://github.com/libgit2/pygit2/issues/573
+        # https://github.com/libgit2/libgit2/issues/3786
+        # self.raw_repo.remotes[remote_name].fetch()
+        for refspec in refspecs:
+            kwargs = {}
+            kwargs['verbose_on_failure'] = True
+            if verbose:
+                # If we are redirecting stdout/stderr to the console, we
+                # do not need to have run() also emit it
+                kwargs['verbose_on_failure'] = False
+                kwargs['stdout'] = None
+                kwargs['stderr'] = None
+            try:
+                logging.debug(
+                    "Fetching refspec %s from remote %s",
+                    refspec,
+                    remote_name,
+                )
+                self.git_run(
+                    args=['fetch', remote_name, refspec],
+                    env={'GIT_TERMINAL_PROMPT': '0',},
+                    **kwargs,
+                )
+            except CalledProcessError:
+                raise GitUbuntuRepositoryFetchError(
+                    "Unable to fetch %s from remote %s" % (
+                        refspecs,
+                        remote_name,
+                    )
+                )
 
     def fetch_lpuser_remote(self, verbose=False):
         if not self._fetch_proto:
diff --git a/gitubuntu/versioning.py b/gitubuntu/versioning.py
index c3187f5..3a5dc53 100644
--- a/gitubuntu/versioning.py
+++ b/gitubuntu/versioning.py
@@ -27,7 +27,7 @@ __all__ = [
 # see _decompose_version_string for explanation of members
 _VersionComps = namedtuple(
     "_VersionComps",
-    ["prefix_parts", "ubuntu_maj", "ubuntu_series", "ubuntu_min"]
+    ["prefix_parts", "ubuntu_maj", "ubuntu_series", "ubuntu_other", "ubuntu_min"]
 )
 
 def _decompose_version_string(version_string):
@@ -52,10 +52,19 @@ def _decompose_version_string(version_string):
     "1.0.2ubuntu3.17.04.4", 17.04 is the Ubuntu series string. In both
     "1.0-2ubuntu3.4" and "1.0-2", the Ubuntu series is not present.
 
+    ubuntu_other is any non-series information as a string between the
+    major and minor version numbers. For example, in the version strings
+    "1.0-2ubuntu3.4" and "1.0-2ubuntu3.17.04.4", the other Ubuntu
+    version string is not present. In both "1.0-2" and "1.0-2ubuntu3",
+    the other Ubuntu version number is not present. In
+    "1.2.2-0ubuntu13.1.23", "1" is the Ubuntu other version string.
+
     ubuntu_min is the "minor" Ubuntu version number as an integer, or None if
     not present. For example, in the version strings "1.0-2ubuntu3.4" and
     "1.0-2ubuntu3.17.04.4", 4 is the Ubuntu minor version number. In both
-    "1.0-2" and "1.0-2ubuntu3", the minor Ubuntu version number is not present.
+    "1.0-2" and "1.0-2ubuntu3", the minor Ubuntu version number is not
+    present. In "1.2.2-0ubuntu13.1.23", 23 is the Ubuntu minor version
+    number.
 
     For version strings not following these standard forms, the result is
     undefined except where specific test cases exist for them.
@@ -73,7 +82,7 @@ def _decompose_version_string(version_string):
         )
     elif 'build' in parts:
         return _VersionComps._make(
-            [parts[:parts.index('build')], None, None, None]
+            [parts[:parts.index('build')], None, None, None, None]
         )
     elif 'ubuntu' in parts:
         ubuntu_idx = parts.index('ubuntu')
@@ -85,6 +94,7 @@ def _decompose_version_string(version_string):
             # nothing after "Xubuntu", treated as 1
             ubuntu_maj = 1
         ubuntu_series = None
+        ubuntu_other = None
         if len(suffix_parts) < 2:
             ubuntu_min = None
         else:
@@ -95,15 +105,13 @@ def _decompose_version_string(version_string):
             if len(suffix_parts[1:-1]) > 2:
                 ubuntu_series = ''.join(suffix_parts[2:-2])
                 if not re.match(r'\d\d\.\d\d', ubuntu_series):
-                    raise ValueError("Not sure how to decompose %s: "
-                        "series %s not in expected format" %
-                        (version_string, ubuntu_series)
-                    )
+                    ubuntu_series = None
+                    ubuntu_other = ''.join(suffix_parts[2:-2])
         return _VersionComps._make(
-            [static_parts, ubuntu_maj, ubuntu_series, ubuntu_min]
+            [static_parts, ubuntu_maj, ubuntu_series, ubuntu_other, ubuntu_min]
         )
     else:
-        return _VersionComps._make([parts, None, None, None])
+        return _VersionComps._make([parts, None, None, None, None])
 
 
 def _bump_version_object(old_version_object, bump_function):
@@ -169,6 +177,8 @@ def _bump_sru_version_suffix_string(befores, series_string, afters,
     bumped_parts = ['ubuntu', str(new_maj), '.']
     if old_suffix_comps.ubuntu_series:
         bumped_parts.extend([old_suffix_comps.ubuntu_series, '.'])
+    elif old_suffix_comps.ubuntu_other:
+        bumped_parts.extend([old_suffix_comps.ubuntu_other, '.'])
     elif str(version) in [str(v) for v in befores + afters]:
         bumped_parts.extend([series_string, '.'])
     elif any(
@@ -251,12 +261,13 @@ def version_compare(a, b):
 
 
 @pytest.mark.parametrize('test_input, expected', [
-    ('2', _VersionComps._make([['2'], None, None, None])),
-    ('2ubuntu1', _VersionComps._make([['2'], 1, None, None])),
-    ('2ubuntu1.3', _VersionComps._make([['2'], 1, None, 3])),
-    ('2ubuntu0.16.04.3', _VersionComps._make([['2'], 0, '16.04', 3])),
-    ('2build1', _VersionComps._make([['2'], None, None, None])),
-    ('2build1.3', _VersionComps._make([['2'], None, None, None])),
+    ('2', _VersionComps._make([['2'], None, None, None, None])),
+    ('2ubuntu1', _VersionComps._make([['2'], 1, None, None, None])),
+    ('2ubuntu1.3', _VersionComps._make([['2'], 1, None, None, 3])),
+    ('2ubuntu0.16.04.3', _VersionComps._make([['2'], 0, '16.04', None, 3])),
+    ('2build1', _VersionComps._make([['2'], None, None, None, None])),
+    ('2build1.3', _VersionComps._make([['2'], None, None, None, None])),
+    ('0ubuntu13.1.22', _VersionComps._make([['0'], 13, None, '1', 22])),
 ])
 def test_decompose_version_string(test_input, expected):
     assert _decompose_version_string(test_input) == expected