Code review comment for ~paelzer/ubuntu/+source/libvirt-python:merge-7.6-impish

What orig tarball are you planning to use for this upload?

Your "New upstream version 7.6.0" shows me:

$ git log -1 -p --name-status c3539dc
commit c3539dc
Author: Christian Ehrhardt <email address hidden>
Date: Mon Aug 16 12:37:50 2021 +0200

    New upstream version 7.6.0

A .ctags
A .dir-locals.el
A .github/lockdown.yml
A .gitignore
A .gitlab-ci.yml
A .mailmap
D AUTHORS
A AUTHORS.in
A CONTRIBUTING.rst
D ChangeLog
A HACKING
D MANIFEST
A Makefile
D PKG-INFO
M README
A ci/containers/README.rst
A ci/containers/ci-centos-8.Dockerfile
A ci/containers/ci-centos-stream-8.Dockerfile
A ci/containers/ci-debian-10.Dockerfile
A ci/containers/ci-debian-sid.Dockerfile
A ci/containers/ci-fedora-33.Dockerfile
A ci/containers/ci-fedora-34.Dockerfile
A ci/containers/ci-fedora-rawhide.Dockerfile
A ci/containers/ci-opensuse-leap-152.Dockerfile
A ci/containers/ci-opensuse-tumbleweed.Dockerfile
A ci/containers/ci-ubuntu-1804.Dockerfile
A ci/containers/ci-ubuntu-2004.Dockerfile
A ci/containers/refresh
A examples/README
A examples/dhcpleases.py
M generator.py
M libvirt-override-api.xml
M libvirt-override.c
D libvirt-python.spec
A libvirt-python.spec.in
A requirements-test.txt
M setup.py
M tests/test_conn.py
M tests/test_domain.py
A tests/test_domain_checkpoint.py
A tests/test_domain_snapshot.py
A tests/test_interface.py
A tests/test_network.py
A tests/test_nodedev.py
A tests/test_storage.py
M tox.ini

This mismatches the upstream tarball fetched (and gpg validated) by uscan as follows:

Extraneous files:

    .ctags
    .dir-locals.el
    .github/lockdown.yml
    .gitignore
    .gitlab-ci.yml
    .mailmap
    AUTHORS.in
    CONTRIBUTING.rst
    HACKING
    Makefile
    ci/containers/README.rst
    ci/containers/ci-centos-8.Dockerfile
    ci/containers/ci-centos-stream-8.Dockerfile
    ci/containers/ci-debian-10.Dockerfile
    ci/containers/ci-debian-sid.Dockerfile
    ci/containers/ci-fedora-33.Dockerfile
    ci/containers/ci-fedora-34.Dockerfile
    ci/containers/ci-fedora-rawhide.Dockerfile
    ci/containers/ci-opensuse-leap-152.Dockerfile
    ci/containers/ci-opensuse-tumbleweed.Dockerfile
    ci/containers/ci-ubuntu-1804.Dockerfile
    ci/containers/ci-ubuntu-2004.Dockerfile
    ci/containers/refresh
    examples/README
    examples/dhcpleases.py
    libvirt-python.spec.in
    requirements-test.txt
    tests/test_domain_checkpoint.py
    tests/test_domain_snapshot.py
    tests/test_interface.py
    tests/test_network.py
    tests/test_nodedev.py
    tests/test_storage.py

Missing files:

    AUTHORS
    ChangeLog
    MANIFEST
    PKG-INFO
    libvirt-python.spec

This seems to roughly inversely match your commit. It appears as though you've switched from the release tarball to a git snapshot. Wouldn't it be better to stick to what Debian are doing here? This has the additional advantage that uscan can be used to verify the orig tarball comes from and is signed by the same upstream used previously.

Looking at the diff, I don't see anything that looks like it would affect packaging, except for a change around the tests. I wondered if this would require a build dependency change, and if tests might silently stop running. From your PPA build log, I found that no tests are being run, so I thought this might indeed be a regression against the current version in Ubuntu. However, looking at the build log for Hirsute 7.0.0-2 (the current version), tests aren't running there either. So this isn't a regression, and therefore I wouldn't consider this issue to be a blocker for this MP.

It's possible that tests aren't useful and/or intended to run as part of the package build, but this isn't mentioned in the original MIR bug (https://bugs.launchpad.net/ubuntu/+source/libvirt-python/+bug/1262758) so could probably do with some investigation separate to this MP. I didn't look back as far as libvirt itself previous to the upstream release split though. Maybe it's worth filing a bug for this?

Needs fixing: for the orig tarball question only.