Merge ~nacc/git-ubuntu:bugfixes-lint-clean into git-ubuntu:master

[Nish Aravamudan]

To make CI happy.

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

FAILED: Continuous integration, rev:bfba3507bf4ebffc0526495eec9f2de260b2eb82
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/330451/+edit-commit-message

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

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

review: Needs Fixing (continuous-integration)

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

FAILED: Continuous integration, rev:bfba3507bf4ebffc0526495eec9f2de260b2eb82
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/330451/+edit-commit-message

https://jenkins.ubuntu.com/server/job/git-ubuntu-ci/42/
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/42/rebuild

review: Needs Fixing (continuous-integration)

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

FAILED: Continuous integration, rev:bfba3507bf4ebffc0526495eec9f2de260b2eb82
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/330451/+edit-commit-message

https://jenkins.ubuntu.com/server/job/git-ubuntu-ci/45/
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/45/rebuild

review: Needs Fixing (continuous-integration)

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

FAILED: Continuous integration, rev:a237f87d2c02b64f9d4c9d62aaa8a7d354ea041d
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/330461/+edit-commit-message

https://jenkins.ubuntu.com/server/job/git-ubuntu-ci/55/
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/55/rebuild

review: Needs Fixing (continuous-integration)

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

PASSED: Continuous integration, rev:e1ad1e0560ab15f4a39347fbea0a13f2747cf853
https://jenkins.ubuntu.com/server/job/git-ubuntu-ci/59/
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/59/rebuild

review: Approve (continuous-integration)

[Andreas Hasenack (ahasenack)]

+1

review: Approve

b0ee670... by Nish Aravamudan on 2017-09-11

snap: main part can be obtained from .

The jenkins job now clones a repo and builds a snap from
potential branches. This does not quite work if our yaml
always builds master. Have it use the local dir instead.

[Nish Aravamudan (nacc)]

On Mon, Sep 11, 2017 at 1:10 PM, Andreas Hasenack <email address hidden> wrote:
> Review: Approve
>
> +1
>
> Diff comments:
>
>> diff --git a/gitubuntu/lint.py b/gitubuntu/lint.py
>> index 435e7f2..6258e03 100644
>> --- a/gitubuntu/lint.py
>> +++ b/gitubuntu/lint.py
>> @@ -364,24 +364,28 @@ class GitUbuntuLint:
>> ret = False
>>
>> added_lines = [l.content for l in changelog_patch.hunks[0].lines if l.origin == "+"]
>> - if added_lines[-1] != "\n":
>> - error("last added line to changelog must be blank")
>> - ret = False
>> -
>> - changelog = debian.changelog.Changelog("".join(added_lines[:-1]),
>> - strict=True
>> - )
>> - if len(changelog.versions) != 1:
>> - error("must add exactly one changelog entry")
>> - ret = False
>> -
>> - if any(["\t" in l for l in added_lines]):
>> - warning("Tab characters found in new changelog entry")
>> - ret = False
>> -
>> - if any([l.endswith(" \n") for l in added_lines]):
>> - warning("Trailing whitespace found in new changelog entry")
>> + if len(added_lines) == 0:
>> + error("No lines added to changelog")
>> ret = False
>> + else:
>> + if added_lines[-1] != "\n":
>> + error("last added line to changelog must be blank")
>> + ret = False
>> +
>> + changelog = debian.changelog.Changelog("".join(added_lines[:-1]),
>> + strict=True
>> + )
>> + if len(changelog.versions) != 1:
>> + error("must add exactly one changelog entry")
>> + ret = False
>> +
>> + if any(["\t" in l for l in added_lines]):
>> + warning("Tab characters found in new changelog entry")
>> + ret = False
>> +
>
> There are some empty lines in this changeset with whitespace in them (visible in git diff, but not here in lp).

Good catch, a new jenkins job is running now with the fix to the snap
(to build the proposed branch as a snap and test it) and then I'll
land this on master with the above fixed.

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

PASSED: Continuous integration, rev:b0ee6701bb0fa4b24660643b378a85375b7cd2e3
https://jenkins.ubuntu.com/server/job/git-ubuntu-ci/62/
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/62/rebuild

review: Approve (continuous-integration)

[Nish Aravamudan (nacc)]

review: Approve

[Nish Aravamudan (nacc)]

Self-approving. The actual branch that lands will be slightly different, due to the whitespace changes.

diff --git a/bin/git-ubuntu b/bin/git-ubuntu
index 23f908e..2b15471 100755
--- a/bin/git-ubuntu
+++ b/bin/git-ubuntu
@@ -1,4 +1,4 @@
-#!/usr/bin/python3
+#!/usr/bin/env python3
 
 # PYTHON_ARGCOMPLETE_OK
 
diff --git a/bin/import-cron b/bin/import-cron
index b23e512..0044b74 100755
--- a/bin/import-cron
+++ b/bin/import-cron
@@ -1,4 +1,4 @@
-#!/usr/bin/python3
+#!/usr/bin/env python3
 
 from copy import copy
 import os
diff --git a/bin/snap/quilt b/bin/snap/quilt
new file mode 100755
index 0000000..547f65f
--- /dev/null
+++ b/bin/snap/quilt
@@ -0,0 +1,8 @@
+#!/bin/bash
+
+CORE_SNAP="/snap/core/current"
+CORE_SNAP_PATH="$CORE_SNAP/usr/local/sbin:$CORE_SNAP/usr/local/bin:$CORE_SNAP/usr/sbin:$CORE_SNAP/usr/bin:$CORE_SNAP/sbin:$CORE_SNAP:/bin"
+export PATH="$SNAP/bin/snap:$SNAP/usr/local/sbin:$SNAP/usr/local/bin:$SNAP/usr/sbin:$SNAP/usr/bin:$SNAP/sbin:$SNAP/bin:$CORE_SNAP_PATH"
+export LD_LIBRARY_PATH="$SNAP/lib/x86_64-linux-gnu:$SNAP/usr/lib/x86_64-linux-gnu"
+export QUILT_DIR="$SNAP/usr/share/quilt"
+exec "$SNAP/usr/bin/quilt" "$@"
diff --git a/gitubuntu/__main__.py b/gitubuntu/__main__.py
index ba4a435..165fc59 100644
--- a/gitubuntu/__main__.py
+++ b/gitubuntu/__main__.py
@@ -1,4 +1,4 @@
-#!/usr/bin/python3
+#!/usr/bin/env python3
 
 def main():
     try:
@@ -90,7 +90,7 @@ def main():
                                     metavar='[%s]' % '|'.join(known_protos),
                                     choices=known_protos,
                                     help='Specify protocol to use for fetch. Default: %(default)s',
-                                    default='git'
+                                    default='https'
                                    )
 
         width, _ = shutil.get_terminal_size()
@@ -107,20 +107,6 @@ def main():
                                                        break_on_hyphens=False
                                                       )
                                         )
-        help_text = '\n'.join(textwrap.wrap('File specifying '
-                                            'parent-child relationship '
-                                            'overrides as:',
-                                            subhelp_width
-                                           )
-                             )
-        help_text += '\n'
-        help_text += '<pkgname> <child version> <publish parent version> <changelog parent version>\n'
-        help_text += '\n'.join(textwrap.wrap('with one override per '
-                                             'line.',
-                                             subhelp_width
-                                            )
-                              )
-        help_text += '\ndefault=%(default)s'
         parser.add_argument('-P', '--parentfile', type=str,
                             help=argparse.SUPPRESS,
                             default=pkg_resources.resource_filename(
@@ -128,22 +114,6 @@ def main():
                                 'parent_overrides.txt'
                                                                    )
                            )
-        help_text = '\n'.join(textwrap.wrap('File specifying source '
-                                            'files to use instead of '
-                                            'Launchpad to work around '
-                                            'source file shadowing: ',
-                                            subhelp_width
-                                           )
-                             )
-        help_text += '\n'
-        help_text += '<pkgname> <version> <URL of DSC file> <URLs of orig files>\n'
-        help_text += '\n'.join(textwrap.wrap('with one package per line. '
-                                             '\'-\' for the DSC file '
-                                             'indicates to use the one '
-                                             'from Launchpad.'
-                                            )
-                              )
-        help_text += '\ndefault=%(default)s'
         parser.add_argument('-L', '--pullfile', type=str,
                             help=argparse.SUPPRESS,
                             default=pkg_resources.resource_filename(
diff --git a/gitubuntu/build.py b/gitubuntu/build.py
index 8143995..a8f9cf3 100644
--- a/gitubuntu/build.py
+++ b/gitubuntu/build.py
@@ -38,12 +38,11 @@ from gitubuntu.run import run, runq
 from gitubuntu.source_information import (
     GitUbuntuSourceInformation,
     NoPublicationHistoryException,
+    derive_source_from_series,
 )
 try:
     pkg = 'python3-debian'
     from debian.debfile import PART_EXTS
-    pkg = 'python3-distro-info'
-    from distro_info import DebianDistroInfo, UbuntuDistroInfo
     pkg = 'python3-pytest'
     import pytest
 except ImportError:
@@ -228,8 +227,7 @@ def fetch_orig_from_cache(changelog, source, dl_cache=None):
         return None
     if dsc.verify() is None:
         logging.warn(
-            "Cache dir found, but verification of orig tarball %s failed.",
-            orig,
+            "Cache dir found, but verification of orig tarball(s) failed."
         )
         return None  # XXX decide - see docstring
     _symlink_paths_into_parent_dir(dsc.all_tarball_paths)
@@ -320,8 +318,8 @@ def fetch_orig_from_launchpad(changelog, source, pullfile, retries,
     except NoPublicationHistoryException:
         logging.warning(
             "No publication history found for %s in %s. ",
-            pkgname,
-            dist_name,
+            changelog.srcpkg,
+            source,
         )
         return None
 
@@ -513,12 +511,7 @@ class GitUbuntuBuild:
 
 
 def derive_source_from_changelog(changelog):
-    series = changelog.distribution
-    if DebianDistroInfo().valid(codename=series):
-        return 'debian'
-    if UbuntuDistroInfo().valid(codename=series):
-        return 'ubuntu'
-    raise ValueError("Unable to determine distribution from %s" % series)
+    return derive_source_from_series(changelog.distribution)
 
 def expand_changelog_source_aliases(orig_search_list, changelog):
     """Replace 'changelog' sources by reading debian/changelog
diff --git a/gitubuntu/clone.py b/gitubuntu/clone.py
index a1bc8cb..9a15c9c 100644
--- a/gitubuntu/clone.py
+++ b/gitubuntu/clone.py
@@ -45,7 +45,7 @@ Example:
         )
         parser.add_argument('directory', type=str,
             help='Local directory to clone to. If not specified, a '
-                 ' directory with the same name as PACKAGE will be '
+                 'directory with the same name as PACKAGE will be '
                  'used',
             default=None,
             nargs='?'
diff --git a/gitubuntu/importer.py b/gitubuntu/importer.py
index 22fd08a..cce1999 100644
--- a/gitubuntu/importer.py
+++ b/gitubuntu/importer.py
@@ -361,17 +361,14 @@ class GitUbuntuImport:
         except PristineTarError as e:
             raise GitUbuntuImportOrigError from e
 
-    def import_orig(self, spi):
+    def import_orig(self, dsc, version, dist):
         """Imports the orig-tarball using gbp import-org --pristine-tar
 
         Arguments:
-        spi - A GitUbuntuSourcePackageInformation instance
+        dsc - A GitUbuntuRepository object
+        version - the string package version
+        dist - the string distribution, either 'ubuntu' or 'debian'
         """
-        dsc = GitUbuntuDsc(spi.dsc_pathname)
-        upstream_version = str(spi.upstream_version)
-        version = str(spi.version)
-        dist = spi.distribution.name.lower()
-
         try:
             orig_tarball_path = dsc.orig_tarball_path
         except GitUbuntuDscError as e:
@@ -785,7 +782,11 @@ class GitUbuntuImport:
             spi.distribution.name.lower()
         )
         if not self.skip_orig:
-            self.import_orig(spi)
+            self.import_orig(
+                GitUbuntuDsc(spi.dsc_pathname),
+                version = str(spi.version),
+                dist = spi.distribution.name.lower(),
+            )
 
         unapplied_import_tree_hash = self.import_patches_unapplied_tree(spi.dsc_pathname)
         logging.debug('Imported patches-unapplied version %s as tree %s',
diff --git a/gitubuntu/importppa.py b/gitubuntu/importppa.py
index a4a3ec1..2ae1f81 100644
--- a/gitubuntu/importppa.py
+++ b/gitubuntu/importppa.py
@@ -83,7 +83,7 @@ class GitUbuntuImportPPA(GitUbuntuImport):
                           'ppa:<name>', args.ppa)
             sys.exit(1)
         ppa = args.ppa
-        pkgname = args.package
+        self.pkgname = args.package
         owner = args.lp_owner
         user = args.lp_user
         no_clean = args.no_clean
@@ -110,14 +110,14 @@ class GitUbuntuImportPPA(GitUbuntuImport):
 
         atexit.register(self.cleanup, no_clean, self.local_repo.local_dir)
 
-        self.local_repo.add_remote(pkgname, owner, self.namespace, user)
+        self.local_repo.add_remote(self.pkgname, owner, self.namespace)
         if not args.no_fetch:
             try:
                 self.local_repo.fetch_remote(self.namespace)
             except GitUbuntuRepositoryFetchError:
                 pass
 
-        source_information = UbuntuSourceInformation(ppa, pkgname,
+        source_information = GitUbuntuSourceInformation(ppa, self.pkgname,
             os.path.abspath(args.pullfile),
             args.retries, args.retry_backoffs)
 
@@ -151,7 +151,7 @@ class GitUbuntuImportPPA(GitUbuntuImport):
 
         os.makedirs(workdir, exist_ok=True)
 
-        self.parse_parentfile(pkgname, args.parentfile)
+        self.parse_parentfile(args.parentfile)
 
         history_found = False
         try:
@@ -168,11 +168,11 @@ class GitUbuntuImportPPA(GitUbuntuImport):
             history_found = True
         except NoPublicationHistoryException:
             logging.warning("No publication history found for %s in %s. ",
-                            pkgname, distname)
+                            self.pkgname, ppa)
 
         if not history_found:
             logging.error("No publication history for '%s' in %s. "
-                          "Wrong source package name?", pkgname, ppa)
+                          "Wrong source package name?", self.pkgname, ppa)
             sys.exit(1)
 
         try:
@@ -188,7 +188,7 @@ class GitUbuntuImportPPA(GitUbuntuImport):
                     pass
         except NoPublicationHistoryException:
             logging.warning("No publication history found for %s in %s. ",
-                            pkgname, ppa)
+                            self.pkgname, ppa)
         except:
             logging.error("Unable to import patches-applied to %s", ppa)
 
diff --git a/gitubuntu/lint.py b/gitubuntu/lint.py
index 435e7f2..6258e03 100644
--- a/gitubuntu/lint.py
+++ b/gitubuntu/lint.py
@@ -364,24 +364,28 @@ class GitUbuntuLint:
             ret = False
 
         added_lines = [l.content for l in changelog_patch.hunks[0].lines if l.origin == "+"]
-        if added_lines[-1] != "\n":
-            error("last added line to changelog must be blank")
-            ret = False
-
-        changelog = debian.changelog.Changelog("".join(added_lines[:-1]),
-            strict=True
-        )
-        if len(changelog.versions) != 1:
-            error("must add exactly one changelog entry")
-            ret = False
-
-        if any(["\t" in l for l in added_lines]):
-            warning("Tab characters found in new changelog entry")
-            ret = False
-
-        if any([l.endswith(" \n") for l in added_lines]):
-            warning("Trailing whitespace found in new changelog entry")
+        if len(added_lines) == 0:
+            error("No lines added to changelog")
             ret = False
+        else:
+            if added_lines[-1] != "\n":
+                error("last added line to changelog must be blank")
+                ret = False
+         
+            changelog = debian.changelog.Changelog("".join(added_lines[:-1]),
+                strict=True
+            )
+            if len(changelog.versions) != 1:
+                error("must add exactly one changelog entry")
+                ret = False
+         
+            if any(["\t" in l for l in added_lines]):
+                warning("Tab characters found in new changelog entry")
+                ret = False
+         
+            if any([l.endswith(" \n") for l in added_lines]):
+                warning("Trailing whitespace found in new changelog entry")
+                ret = False
 
         if ret:
             self.success("Verified that only new modifications to changelog "
@@ -670,7 +674,6 @@ class GitUbuntuLint:
                            )
         self.directory = args.directory
         self.local_repo = GitUbuntuRepository(self.directory)
-        self.pullfile = args.pullfile
         self.verbose = args.verbose
 
         if args.commitish:
@@ -684,7 +687,7 @@ class GitUbuntuLint:
                 sys.exit(1)
         else:
             commitish_obj = self.local_repo.raw_repo.head
-            if self.local_repo.head_is_detached:
+            if self.local_repo.raw_repo.head_is_detached:
                 commitish_string = 'HEAD'
             else:
                 for head in self.local_repo.local_branches:
diff --git a/gitubuntu/source_information.py b/gitubuntu/source_information.py
index a759b53..ead7522 100644
--- a/gitubuntu/source_information.py
+++ b/gitubuntu/source_information.py
@@ -10,6 +10,8 @@ _lp_service = 'production'
 _lp_api_version = 'devel'
 
 try:
+    pkg = 'python3-distro-info'
+    from distro_info import DebianDistroInfo, UbuntuDistroInfo
     pkg = 'python3-launchpadlib'
     from launchpadlib.launchpad import Launchpad as LP
     pkg = 'python3-ubuntutools'
@@ -48,12 +50,21 @@ def launchpad_login_auth():
     return _LP_LOGIN_AUTH
 
 
+def derive_source_from_series(series):
+    if DebianDistroInfo().valid(codename=series):
+        return 'debian'
+    if UbuntuDistroInfo().valid(codename=series):
+        return 'ubuntu'
+    raise ValueError("Unable to determine distribution from %s" % series)
+
+
 class GitUbuntuPPASourcePackage(UbuntuSourcePackage):
     "Download / unpack an PPA source package"
     def __init__(self, ppa_spec, *args, **kwargs):
         super(GitUbuntuPPASourcePackage, self).__init__(*args, **kwargs)
         _, ppa = ppa_spec.split(':')
         self._ppa_owner, self._ppa_name = ppa.split('/', 1)
+        self._spph = None
 
     def _lp_url(self, filename):
         "Build a source package URL on Launchpad"
@@ -257,10 +268,15 @@ class GitUbuntuSourceInformation(object):
             _, ppa = dist_name.split(':')
             ppa_owner, ppa_name = ppa.split('/', 1)
             possible_people = self.launchpad.people.findPerson(text=ppa_owner)
+            person = None
             for person in possible_people:
                 if person.name == ppa_owner:
                     ppa_owner = person
                     break
+            if person is None:
+                raise ValueError(
+                    "Unable to find owner for PPA: %s" % dist_name
+                )
             self.archive = person.getPPAByName(name=ppa_name)
         else:
             self.dist = self.launchpad.distributions[dist_name]
diff --git a/gitubuntu/submit.py b/gitubuntu/submit.py
index 4dc0d32..6e46a6e 100644
--- a/gitubuntu/submit.py
+++ b/gitubuntu/submit.py
@@ -42,7 +42,7 @@ class GitUbuntuSubmit:
         )
         parser.add_argument('--reviewer', action='append',
             help='Specify reviewers for the proposed merge. This '
-		 'will default to the canonical-server team. This option can be '
+                 'will default to the canonical-server team. This option can be '
                  'specified multiple times.',
             default=['canonical-server']
         )
@@ -73,7 +73,6 @@ class GitUbuntuSubmit:
     def main(self, args):
         self.directory = args.directory
         self.force = args.force
-        self.pullfile = args.pullfile
         self.target_user = args.target_user
         try:
             self.user = args.lp_user
@@ -88,15 +87,19 @@ class GitUbuntuSubmit:
                 logging.warning("Specified branch (%s) does not exist locally.",
                     args.branch
                 )
+            self.commitish_string = args.branch
         else:
             if self.local_repo.raw_repo.head_is_detached:
                 logging.error("Please create a local branch before submitting.")
                 sys.exit(1)
             self.source_branch = self.local_repo.raw_repo.head.name
+            self.commitish_string = 'HEAD'
 
         logging.debug("source branch: %s", self.source_branch)
 
-        self.pkgname = self.local_repo.get_changelog_srcpkg_from_treeish('HEAD')
+        self.pkgname = self.local_repo.get_changelog_srcpkg_from_treeish(
+            self.commitish_string
+        )
 
         logging.debug("source package: %s", self.pkgname)
 
diff --git a/snap/snapcraft.yaml b/snap/snapcraft.yaml
index 89c5a30..a5c17c0 100644
--- a/snap/snapcraft.yaml
+++ b/snap/snapcraft.yaml
@@ -32,9 +32,7 @@ parts:
          - -usr/lib/python2.7
    main:
       plugin: dump
-      source: https://git.launchpad.net/usd-importer
-      source-type: git
-      source-branch: master
+      source: .
       stage-packages:
          - python3-argcomplete
          - python3-debian