Merge ~bryce/git-ubuntu:fix_export_orig_empty_tarballs_traceback into git-ubuntu:master

[Bryce Harrington]

Sometimes a traceback is displayed when trying to do `git ubuntu export-orig`. Some examples can be seen at LP: #1836286 and LP: #1868631, with traces ending in "object of type 'NoneType' has no len()".

The underlying problem here is that build.fetch_orig() sometimes can't find tarballs successfully by any of its known mechanisms. It should be returning [] (an empty list) in this case, but the function lacks a defined return statement, and so python defaults to returning None.

This branch fixes the problem by adding the missing return [] and adding code docs and an assert to spec the routine's return behavior.

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

FAILED: Continuous integration, rev:547c8bc37fc79631f79d5239cde8f44e2da5bcdc
https://jenkins.ubuntu.com/server/job/git-ubuntu-ci/46/
Executed test runs:
    SUCCESS: VM Setup
    FAILED: Build

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

review: Needs Fixing (continuous-integration)

[Robie Basak (racb)]

Looks good. Thanks!

The CI failure appears to be because you have a "0.8.0" tag that conflicts with the one in the main repository. Let's leave it there for now - I filed https://github.com/canonical/server-jenkins-jobs/pull/202 that should stop tag conflicts mattering. We can see if that fixes CI, and worry about the conflicting tag later.

One inline comment. I'm happy to leave that to your judgement, but let's get CI passed before landing.

review: Needs Fixing

[Utkarsh Gupta (utkarsh)]

Hello,

Maybe not very related but can we redirect `git ubuntu export-orig` to `git ubuntu export-orig --for-merge` whenever we hit this?

For instance, I am just doing a ruby2.7 merge now and here's what happens:

$ git ubuntu export-orig

07/09/2021 17:53:02 - WARNING:No pristine-tar data found for 2.7.4
07/09/2021 17:53:13 - INFO:New upstream version detected (2.7.4) which is after the last published upstream version (2.7.3).
Traceback (most recent call last):
  File "/snap/git-ubuntu/562/usr/bin/git-ubuntu", line 11, in <module>
    load_entry_point('gitubuntu==1.0', 'console_scripts', 'git-ubuntu')()
  File "/snap/git-ubuntu/562/usr/lib/python3/dist-packages/gitubuntu/__main__.py", line 269, in main
    sys.exit(args.func(args))
  File "/snap/git-ubuntu/562/usr/lib/python3/dist-packages/gitubuntu/exportorig.py", line 117, in cli_main
    if len(tarballs) > 0:
TypeError: object of type 'NoneType' has no len()

$ git ubuntu export-orig --for-merge
07/09/2021 17:54:11 - WARNING:No pristine-tar data found for 2.7.4
07/09/2021 17:54:24 - INFO:Downloading ruby2.7_2.7.4.orig.tar.xz from launchpad.net (10.310 MiB)
[======================> ] 39%

...assuming this is working and will download the right orig.tar.xz.

[Robie Basak (racb)]

On Fri, Jul 09, 2021 at 12:28:58PM -0000, Utkarsh Gupta wrote:
> Maybe not very related but can we redirect `git ubuntu export-orig` to `git ubuntu export-orig --for-merge` whenever we hit this?

Bryce created https://bugs.launchpad.net/bugs/1935564 so let's discuss
this there. I wouldn't want to hold back this (much more
straightforward) MP on that change :)

[Bryce Harrington (bryce)]

On Fri, Jul 09, 2021 at 12:21:28PM -0000, Robie Basak wrote:
> Review: Needs Fixing
>
> Looks good. Thanks!
>
> The CI failure appears to be because you have a "0.8.0" tag that conflicts with the one in the main repository. Let's leave it there for now - I filed https://github.com/canonical/server-jenkins-jobs/pull/202 that should stop tag conflicts mattering. We can see if that fixes CI, and worry about the conflicting tag later.
>
> One inline comment. I'm happy to leave that to your judgement, but let's get CI passed before landing.
>
> Diff comments:
>
> > diff --git a/gitubuntu/build.py b/gitubuntu/build.py
> > index 8b006a8..35f654b 100644
> > --- a/gitubuntu/build.py
> > +++ b/gitubuntu/build.py
> > @@ -499,12 +505,14 @@ def fetch_orig(
> > entry.source,
> > )
> > continue # search returned negative; try next search entry
> > + assert type(tarballs) is list
>
> Is this a common idiom? If so it's fine; otherwise I'd expect isinstance(tarballs, list).

isinstance() WFM.

I've updated the branch with just this one change.

StackExchange suggests they both do the same thing in this case for
built-in types, but isinstance() is more helpful in situations where
subclasses are involved. I used `type(foo) is bar` just out of habit.

Bryce

> > logging.info('Successfully fetched%susing %s(source=%s)',
> > ':\n' + '\n'.join(tarballs) + '\n' if tarballs else ' ',
> > mechanism_name,
> > entry.source,
> > )
> > return tarballs
> > + return []
> >
> >
> > class NativenessMismatchError(Exception): pass
>
>
> --
> https://code.launchpad.net/~bryce/usd-importer/+git/usd-importer/+merge/405422
> You are the owner of ~bryce/usd-importer:fix_export_orig_empty_tarballs_traceback.

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

FAILED: Continuous integration, rev:ba53af3beeef6a0d51b57dececf984d0c5940b9b
https://jenkins.ubuntu.com/server/job/git-ubuntu-ci/47/
Executed test runs:
    SUCCESS: VM Setup
    FAILED: Build

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

review: Needs Fixing (continuous-integration)

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

PASSED: Continuous integration, rev:ba53af3beeef6a0d51b57dececf984d0c5940b9b
https://jenkins.ubuntu.com/server/job/git-ubuntu-ci/48/
Executed test runs:
    SUCCESS: VM Setup
    SUCCESS: Build
    SUCCESS: VM Reset
    SUCCESS: Unit Tests
    IN_PROGRESS: Declarative: Post Actions

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

review: Approve (continuous-integration)

diff --git a/gitubuntu/build.py b/gitubuntu/build.py
index 8b006a8..037dadc 100644
--- a/gitubuntu/build.py
+++ b/gitubuntu/build.py
@@ -481,6 +481,12 @@ def fetch_orig(
     orig_search_list,
     changelog,
 ):
+    """
+    :param orig_search_list: list of OrigSearchListEntry namedtuples.
+    :rtype: list
+    :returns: A list of successfully fetched tarballs, or an empty list
+        if none were found.
+    """
     unaliased_orig_search_list = expand_changelog_source_aliases(
         orig_search_list,
         changelog,
@@ -499,12 +505,14 @@ def fetch_orig(
                 entry.source,
             )
             continue  # search returned negative; try next search entry
+        assert isinstance(tarballs, list)
         logging.info('Successfully fetched%susing %s(source=%s)',
             ':\n' + '\n'.join(tarballs) + '\n' if tarballs else ' ',
             mechanism_name,
             entry.source,
         )
         return tarballs
+    return []
 
 
 class NativenessMismatchError(Exception): pass