Friday 19 February 2021

Reviewing SRUs using git-ubuntu

I've been asked to write up how I use git-ubuntu to review SRU uploads.
This is mainly of use only to other members of the SRU team.

There are just two common steps:

1. I use `git ubuntu clone <package>`. This step gives me the normal
git-ubuntu repository for the package, but doesn't include the
unapproved SRU upload.

2. Entering the git directory, I run `git ubuntu queue sync`. This
pulls in, as `queue/` tags, the uploads from the unapproved queues
as single commits based on the tips of the corresponding series'
`-devel` branches.

At this point, you have the Unapproved upload(s) in your git tree and
can examine it/them in any way that you wish using regular git commands.
Here are some examples:

* `git log -1 -p q<Tab>f<Tab>` shows me the "diff" of the Focal Unapproved
upload. If the upstream orig tarball is being replaced (eg. for an
exceptional major update), I can append `-- debian/` to see packaging
changes only[1].

* `git show q<Tab>f<Tab>:src/foo` shows me the `src/foo` file as it
appears in the Unapproved queue. This is useful when I'm looking at
the diff from the previous example, but want to see wider context.

* `gbp pq import` can be used to flatten quilt patches to use git to
make similar inspections with regard to the effect of quilt patches,
and how they might vary between the upload and other existing branch
tips.

* `git range-diff pkg/ubuntu/focal..q<Tab>f<Tab>
pkg/ubuntu/bionic..q<Tab>b<Tab>` shows me the differences between
what was being proposed to be changed in Focal and what is being
proposed to be changed in Bionic. This is really useful when the same
SRU is landing in two series at once. I only need review it once (I
usually use the most recent series being SRU'd), and then following
that I only have to review the differences when considering the other
series.

* If I want to see how a fix in an SRU differs from a fix in the
development release, then I can use `git diff pkg/ubuntu/devel
q<Tab>f<Tab>` - possibly limiting the output to a single file by
using something like `-- debian/rules`.

* If review changes are needed, I can save the queue tag by tagging it
with a name meaningful to me, ask for the changes, run `git ubuntu
queue sync` again, and diff my old tag with the replacement queue tag
to see what the uploader changed. This saves me from reviewing the
whole thing again.

After review, I use `sru-review --no-diff` to accept the upload, as it
does some additional automated checking and I don't want to miss those.
At some point, I'd like to integrate its functionality so it can all be
done from a single tool.

## Further background

* The queue tag names generated by `git ubuntu queue sync` are intended
to be autocomplete-able. They are tags because multiple uploads for a
single source package can exist in the queue at once, and adding
further commits to them doesn't usually make sense. To disambiguate
they are suffixed by a unique string, but in the common case you can
ignore this and just use autocomplete as there will be only one.

* `git ubuntu queue sync` will delete local tags that no longer exist
in the queue, bringing the local set of queue tags in "sync" with the
queue. If you want to keep some specific old queue uploads around
locally, you can tag them yourself with names that are meaningful to
you. There's also a `--no-trim` option that skips the "delete
obsoleted tags" part of the sync.

* In the rare case that the parent of an imported queue tag needs to be
somewhere else, `git ubuntu queue sync` has various options to help
with that. It can also import from the New queue, though that only
really makes sense for package renames and similar types of corner
cases.

* Pocket copies are automatically followed, so there is no change to
the workflow for these cases. I suppose the SRU team ought to check
that any binary pocket copies have been built in an appropriate way,
but I don't think we have any process for that in the established
non-git-ubuntu method either at the moment, except for manual
observation and checking.

* Since we aren't using Launchpad-generated diffs, and the git-ubuntu
importer already understands the "parent" of an upload, the diffs are
generally correct and don't have the "includes security updates"
issue we have with Launchpad-generated diffs in that particular edge
case.

* Since git is generating the diffs and understands and represents
symlinks well, we can easily spot if symlinks have been accidentally
flattened in an upload.

* These steps only work if the package in question is already being
imported by git-ubuntu. Right now, most of universe is yet to be
imported. As we expand coverage, I'm pondering automatically
including any package that has an upload in an SRU queue. This might
work well to effectively prioritise packages that are actively
maintained.

Robie

[1] In the case of a version 3.0 package, at least.