Tuesday, 3 May 2016

Testing for pending migrations in Django

DB migration support has been added in Django 1.7+, superseding South. More specifically, it's possible to automatically generate migrations steps when one or more changes in the application models are detected. Definitely a nice feature!

I've written a small generic unit-test that one should be able to drop into the tests directory of any Django project and that checks there's no pending migrations, ie. if the models are correctly in sync with the migrations declared in the application. Handy to check nobody has forgotten to git add the migration file or that an innocent looking change in models.py doesn't need a migration step generated. Enjoy!

See the code on djangosnippets or as a github gist!

Monday, 15 February 2016

Continuous Testing with Patchwork

As promised in the post introducing my recent work on Patchwork, I've written some more in-depth documentation to explain how to hook testing to Patchwork. I've also realized that a blog post might not be the best place to put that documentation and opted to put it in the proper manual:


Happy reading!

Saturday, 13 February 2016

Augmenting mailing-lists with Patchwork - Another try

The mailing-list problem


Many software projects use mailing-lists, which usually means mailman, not only for discussions around that project, but also for code contributions. A lot of open source projects work that way, including the one I interact with the most, the Linux kernel. A contributor sends patches to a mailing list, these days using git send-email, and waits for feedback or for his/her patches to be picked up for inclusion if fortunate enough.

Problem is, mailing-lists are awful for code contribution.

A few of the issues at hand:
  • Dealing with patches and emails can be daunting for new contributors,
  • There's no feedback that someone will look into the patch at some point,
  • There's no tracking of which patch has been processed (eg. included into the tree). A shocking number of patches are just dropped as a direct consequence,
  • There's no way to add metadata to a submission. For instance, we can't assign a reviewer from a pool of people working on the project. As a result, review is only working thanks to the good will of people. It's not necessarily a bad thing, but it doesn't work in a corporate environment with deadlines,
  • Mailing-lists are all or nothing: one subscribes to the activity of the full project, but may only care about following the progress of a couple of patches,
  • There's no structure at all actually, it's all just emails,
  • No easy way to hook continuous integration testing,
  • The tools are really bad any time they need to interact with the mailing-list: try to send a patch as a reply to a review comment, addressing it. It starts with going to look at the headers of the review email to copy/paste its Message-ID, followed by an arcane incantation:
    $ git send-email --to=<mailing-list> --cc=<reviewer> \
                     --in-reply-to=<reviewer-mail-message-id> \
                     --reroll-count 2 -1 HEAD~2

Alternative to mailing-lists


Before mentioning Patchwork, it's worth saying that a project can merely decide to switch to using something else than a mailing-list to handle code contributions; To name a few: Gerrit, Phabricator, Github, Gitlab, Crucible.

However, there can be some friction preventing the adoption those tools. People have built their own workflow around mailing-lists for years and it's somewhat difficult to adopt anything else over night. Projects can be big with no clear way to make decisions, so sticking to mailing-lists can just be the result of inertia.

The alternatives also have problems of their own and there's no clear winner, nothing like how git took over the world.


Patchwork


So, the path of least resistance is to keep mailing-lists. Jemery Kerr had the idea to augment mailing-lists with a tool that would track the activity there and build a database of patches and their status (new, reviewed, merged, dropped, ...). Patchwork was born.

Here are some Patchwork instances in the wild:

The KMS and DRI Linux subsystems are using freedesktop.org to host their mailing-lists, which includes the i915 Intel driver, project I've been contributing to since 2012. We have an instance of Patchwork there, and, while somewhat useful, the tool fell short of what we really wanted to do with our code contribution process.

Patches are welcome!


So? it was time to do something about the situation and I started improving Patchwork to answer some of the problems outlined above. Given enough time, it's possible to help on all fronts.

The code can be found on github, along with the current list of issues and enhancements we have thought about. I also maintain freedesktop.org's instance for the graphics team at Intel, but also any freedesktop.org project that would like to give it a try.


Design, Design, Design


First things first, we improved how Patchwork looks and feels. Belén, of OpenEmbedded/Yocto fame, has very graciously spent some of her time to rethink how the interaction should behave.

Before, ...

... and after!

There is still a lot of work remaining to roll out the new design and the new interaction model on all of Patchwork. A glimpse of what that interaction looks like so far:



Series


One thing was clear from the start: I didn't want to have Patches as the main object tracked, but Series, a collection of patches. Typically, developing a  new feature requires more than one patch, especially with the kernel where it's customary to write a lot of orthogonal smaller commits rather than a big (and often all over the place) one. Single isolated commits, like a small bug fix, are treated as a series of one patch.

But that's not all. Series actually evolve over time as the developer answers review comments and the patch-set matures. Patchwork also tracks that evolution, creating several Revisions for the same series. This colour management series from Lionel shows that history tracking (beware, this is not the final design!).

I have started documenting what Patchwork can understand. Two ways can be used to trigger the creation of a new revision: sending a revised patch as a reply to the reviewer email or resending the full series with a similar cover letter subject.

There are many ambiguous cases and some others cases not really handled yet, one of them being sending a series as a reply to another series. That can be quite confusing for the patch submitter but the documented flows should work.

REST API


Next is dusting off Patchwork's XML-RPC API. I wanted to be able to use the same API from both the web pages and git-pw, a command line client.

This new API is close to complete enough to replace the XML-RPC one and already offers a few more features (eg. testing integration). I've also been carefully documenting it.

git-pw


Rob Clark had been asking for years for a better integration with git from the Patchwork's command line tool, especially sharing its configuration file. There also are a number of git "plugins" that have appeared to bridge git with various tools, like git-bz or git-phab.

Patchwork has now his own git-pw, using the REST API. There, again, more work is needed to be in an acceptable shape, but it can already be quite handy to, for instance, apply a full series in one go:

$ git pw apply -s 122
Applying series: DP refactoring v2 (rev 1)
Applying: drm/i915: Don't pass *DP around to link training functions
Applying: drm/i915: Split write of pattern to DP reg from intel_dp_set_link_train
Applying: drm/i915 Call get_adjust_train() from clock recovery and channel eq
Applying: drm/i915: Move register write into intel_dp_set_signal_levels()
Applying: drm/i915: Move generic link training code to a separate file
...

Testing Integration



This is what kept my busy the last couple of months: How to integrate patches sent to a mailing-list with Continuous Integration systems. The flow I came up with is not very complicated but a picture always helps:

Hooking tests to Patchwork


Patchwork is exposing an API so mailing-lists are completely abstracted from systems using that API. Both retrieving the series/patches to test and sending back test results is done through HTTP. That makes testing systems fairly easy to write.

Tomi Sarvela hooked our test-suite, intel-gpu-tools, to patches sent to intel-gfx and we're now gating patch acceptance to the kernel driver with the result of that testing.

Of course, it's not that easy. In our case, we've accumulated some technical debt in both the driver and the test suite, which means it will take time to beat both into be a fully reliable go/no-go signal. People have been actively looking at improving the situation though (thanks!) and I have hope we can reach that reliability sooner rather than later.

As a few words of caution about the above, I'd like to remind everyone that the devil always is in the details:
  • We've restricted the automated testing to a subset of the tests we have (Basic Acceptance Tests aka BATs) to provide a quick answer to developers, but also because some of our tests aren't well bounded,
  • We have no idea how much code coverage that subset really exercises, playing with the kernel gcov support would be interesting for sure,
  • We definitely don't deal with the variety of display sinks (panels and monitors) that are present in the wild.
This means we won't catch all the i915 regressions. Time will definitely improve things as we connect more devices to the testing system and fix our tests and driver.

Anyway, let's leave i915 specific details for another time. A last thing about this testing integration is that Patchwork can be configured to send emails back to the submitter/mailing-list with some test results. As an example, I've written a checkpatch.pl integration that will tell people to fix their patches without the need of a reviewer to do it. I know, living in the future.

For more in-depth documentation about continuous testing with Patchwork, see the testing section of the manual.

What's next?


This blog post is long enough as is, let's finish by the list of things I'd like to be in a acceptable state before I'll happily tag a first version:
  • Series support without without known bugs
  • REST API and git pw able to replace XML-RPC and pwclient
  • Series, Patches and Bundles web pages ported to the REST API and the new filter/action interaction.
  • CI integration
  • Patch and Series life cycle redesigned with more automatic state changes (ie. when someone gives a reviewed-by tag, the patch state should change to reviewed)
There are plenty of other exciting ideas captured in the github issues for when this is done.

Links




Friday, 5 December 2014

Working in a separate prefix

I've been surprised in the past to discover that even some seasoned engineers didn't know how to use the autotools prefix feature. A sign they've been lucky enough and didn't have to deal with Autotools too much. Here's my attempt to provide some introduction to ./configure --prefix.

Working with or in "a separate prefix" is working with libraries and binaries (well, anything produced by 'make install' in an autotooled project really) installed in a different directory than the system-wide ones (/usr or even /usr/local that can become quite messy). It is the preferred way to hack on a full stack without polluting your base distribution and has several advantages:
  • One can hack on the whole stack without the fear of not being able to run your desktop environment you're working with if something goes wrong,
  • More often than not, one needs a relatively recent library that your distribution doesn't ship with (say a recent libdrm). When working with the dependencies in a prefix, it's just a matter of recompiling it.

Let's take an example to make the discussion easier:
  •  We want to compile libdrm and intel-gpu-tools (because intel-gpu-needs needs a more recent libdrm than the one coming with your distribution),
  •  We want to use the ~/gfx directory for our work,
  • git trees with be cloned in ~/gfx/sources,
  • ~/gfx/install is chosen as the prefix.

First, let's clone the needed git repositories:
$ mkdir -p ~/gfx/sources ~/gfx/install
$ cd ~/gfx/sources
$ git clone git://anongit.freedesktop.org/mesa/drm libdrm
$ git clone git://anongit.freedesktop.org/xorg/app/intel-gpu-tools
Then you need to source a script that will set-up your environment with a few variables to tell the system to use the prefix (both at run-time and compile-time). A minimal version of that script for our example is (I store my per-project setup scripts to source at the root of the project, in our case ~/gfx):
$ cat ~/gfx/setup-env
PROJECT=~/gfx
export PATH=$PROJECT/install/bin:$PATH
export LD_LIBRARY_PATH=$PROJECT/install/lib:$LD_LIBRARY_PATH
export PKG_CONFIG_PATH=$PROJECT/install/lib/pkgconfig:$PKG_CONFIG_PATH
export ACLOCAL_FLAGS="-I $PROJECT/install/share/aclocal $ACLOCAL_FLAG"
$ source ~/gfx/setup-env
Then it's time to compile libdrm, telling the configure script that we want to install it in in our prefix:
$ cd ~/gfx/sources/libdrm
$ ./autogen.sh --prefix=/home/damien/gfx/install
$ make
$ make install
Note that you don't need to run "sudo make install" since we'll be installing in our prefix directory that is writeable by the current user.

Now it's time to compile i-g-t:
$ cd ~/gfx/sources/intel-gpu-tools
$ ./autogen.sh --prefix=/home/damien/gfx/install
$ make
$ make install
The configure script may complain about dependencies (eg. cairo, SWIG,...). Different ways to solve those:
  • For dependencies not directly linked with the graphics stack (like SWIG), it's recommended to use the development package provided by the distribution
  • For old enough dependencies that don't change very often (like cairo) you can use the distribution development package or compile them in your prefix
  • For dependencies more recent than your distribution ones, you need to install them in the chosen prefix.

Saturday, 20 September 2014

git commit --fixup and git rebase -i --autosquash

It's not unusual that I need to fix previous commits up when working  on a branch or in the review phase. Until now I used a regular commit with some special marker to remember which commit to squash it with and then git rebase -i to reorder the patches and squash the fixup commits with their corresponding "parent" commits.

Turns out, git can handle quite a few of those manual manipulations for you. git commit --fixup <commit> allows you to commit work, marking it as a fixup of a previous commit. git rebase -i --autosquash will then present the usual git rebase -i screen but with the fixup commits moved just after their parents and ready to be squashed without any extra manipulation.

For instance, I had a couple of changes to a commit buried 100 patches away from HEAD (yes, a big topic branch!):
$ git diff
diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c
index 29f3813..08ea851 100644
--- a/drivers/gpu/drm/i915/intel_display.c
+++ b/drivers/gpu/drm/i915/intel_display.c
@@ -2695,6 +2695,11 @@ static void skylake_update_primary_plane(struct drm_crtc *crtc,

        intel_fb = to_intel_framebuffer(fb);
        obj = intel_fb->obj;
+
+       /*
+        * The stride is expressed either as a multiple of 64 bytes chunks for
+        * linear buffers or in number of tiles for tiled buffers.
+        */
        switch (obj->tiling_mode) {
        case I915_TILING_NONE:
               stride = fb->pitches[0] >> 6;
@@ -2707,7 +2712,6 @@ static void skylake_update_primary_plane(struct drm_crtc *crtc,
BUG();
}

-       plane_ctl &= ~PLANE_CTL_TRICKLE_FEED_DISABLE;
        plane_ctl |= PLANE_CTL_PLANE_GAMMA_DISABLE;

        I915_WRITE(PLANE_CTL(pipe, 0), plane_ctl);
And I wanted to squash those changes with commit 2021785
$ git commit -a --fixup 2021785
git will then go ahead and create a new commit with the subject taken from the referenced commit and prefixed with fixup!
commit d2d278ffbe87d232369b028d0c9ee9e6ecd0ba20
Author: Damien Lespiau <damien.lespiau@intel.com>
Date:   Sat Sep 20 11:09:15 2014 +0100

fixup! drm/i915/skl: Implement thew new update_plane() for primary planes
Then when using the interactive rebase with autosquash:
$ git rebase -i --autosquash drm-intel/drm-intel-nightly
The fixup will be next after the reference commit
pick 2021785 drm/i915/skl: Implement thew new update_plane() for primary planes
fixup d2d278ff fixup! drm/i915/skl: Implement thew new update_plane() for primary planes
validating the proposed change (by in my case leaving vim) will squash the fixup commits. Definitely what I'll be using from now on!

Oh, and there's a config option to have git rebase automatically autosquash if there are some fixup commits:
$ git config --global rebase.autosquash true

Wednesday, 2 October 2013

HDMI stereo 3D & KMS

If everything goes according to plan, KMS in linux 3.13 should have stereo 3D support. Should one be interested in scanning out a stereo frame buffer to a 3D capable HDMI sink, here's a rough description of how those modes are exposed to user space and how to use them.

A reader not well acquainted with the DRM sub-system and its mode setting API (Aka Kernel Mode Setting, KMS) could start by watching the first part of Laurent Pinchart's Anatomy of an Embedded KMS Driver or read David Herrmann's heavily documented mode setting example code.

Stereo modes work by sending a left eye and right eye picture per frame to the monitor. It's then up to the monitor to use those 2 pictures to display a 3D frame and the technology there varies.

There are different ways to organise the 2 pictures inside a bigger frame buffer. For HDMI, those layouts are described in the HDMI 1.4 specification. Provided you give them your contact details, it's possible to download the stereo 3D part of the HDMI 1.4 spec from hdmi.org.

As one inevitably knows, modes supported by a monitor can be retrieved out of the KMS connector object in the form of drmModeModeInfo structures (when using libdrm, it's also possible to write your own wrappers around the KMS ioctls, should you want to):
typedef struct _drmModeModeInfo {
        uint32_t clock;
        uint16_t hdisplay, hsync_start, hsync_end, htotal, hskew;
        uint16_t vdisplay, vsync_start, vsync_end, vtotal, vscan;

        uint32_t vrefresh;

        uint32_t flags;
        uint32_t type;
        char name[...];
} drmModeModeInfo, *drmModeModeInfoPtr;
To keep existing software blissfully unaware of those modes, a DRM client interested in having stereo modes listed starts by telling the kernel to expose them:
drmSetClientCap(drm_fd, DRM_CLIENT_CAP_STEREO_3D, 1);
Stereo modes use the flags field to advertise which layout the mode requires:
uint32_t layout = mode->flags & DRM_MODE_FLAG_3D_MASK;
This will give you a non zero value when the mode is a stereo mode, value among:
DRM_MODE_FLAG_3D_FRAME_PACKING
DRM_MODE_FLAG_3D_FIELD_ALTERNATIVE
DRM_MODE_FLAG_3D_LINE_ALTERNATIVE
DRM_MODE_FLAG_3D_SIDE_BY_SIDE_FULL
DRM_MODE_FLAG_3D_L_DEPTH
DRM_MODE_FLAG_3D_L_DEPTH_GFX_GFX_DEPTH
DRM_MODE_FLAG_3D_TOP_AND_BOTTOM
DRM_MODE_FLAG_3D_SIDE_BY_SIDE_HALF
User space is then responsible for choosing which stereo mode to use and to prepare a buffer that matches the size and left/right placement requirements of that layout. For instance, when choosing Side by Side (half), the frame buffer is the same size as its 2D equivalent (that is hdisplay x vdisplay) with the left and right images sub-sampled by 2 horizontally:

Side by Side (half)

Other modes need a bigger buffer than hdisplay x vdisplay. This is the case with frame packing, where each eye has the the full 2D resolution, separated by the number of vblank lines:


Fame Packing

Of course, anything can be used to draw into the stereo frame buffer, including OpenGL. Further work should enable Mesa to directly render into such buffers, say with the EGL/gbm winsys for a wayland compositor to use.

Wipe Out using Frame Packing on the PS3

Behind the scene, the kernel's job is to parse the EDID to discover which stereo modes the HDMI sink supports and, once user-space instructs to use a stereo mode, to send infoframes (metadata sent during the vblank interval) with the information about which 3D mode is being sent.

A good place to start for anyone wanting to use this API is testdisplay, part of the Intel GPU tools test suite. testdisplay can list the available modes with:
$ sudo ./tests/testdisplay -3 -i
[...]
name refresh (Hz) hdisp hss hse htot vdisp vss vse vtot flags type clock
[0]  1920x1080 60 1920 2008 2052 2200 1080 1084 1089 1125 0x5 0x48 148500
[1]  1920x1080 60 1920 2008 2052 2200 1080 1084 1089 1125 0x5 0x40 148352
[2]  1920x1080i 60 1920 2008 2052 2200 1080 1084 1094 1125 0x15 0x40 74250
[3]  1920x1080i 60 1920 2008 2052 2200 1080 1084 1094 1125 0x20015 0x40 74250 (3D:SBSH)
[4]  1920x1080i 60 1920 2008 2052 2200 1080 1084 1094 1125 0x15 0x40 74176
[5]  1920x1080i 60 1920 2008 2052 2200 1080 1084 1094 1125 0x20015 0x40 74176 (3D:SBSH)
[6]  1920x1080 50 1920 2448 2492 2640 1080 1084 1089 1125 0x5 0x40 148500
[7]  1920x1080i 50 1920 2448 2492 2640 1080 1084 1094 1125 0x15 0x40 74250
[8]  1920x1080i 50 1920 2448 2492 2640 1080 1084 1094 1125 0x20015 0x40 74250 (3D:SBSH)
[9]  1920x1080 24 1920 2558 2602 2750 1080 1084 1089 1125 0x5 0x40 74250
[10]  1920x1080 24 1920 2558 2602 2750 1080 1084 1089 1125 0x1c005 0x40 74250 (3D:TB)
[11]  1920x1080 24 1920 2558 2602 2750 1080 1084 1089 1125 0x4005 0x40 74250 (3D:FP)
[...]
To test a specific mode:
$ sudo ./tests/testdisplay -3 -o 17,10
1920x1080 24 1920 2558 2602 2750 1080 1084 1089 1125 0x1c005 0x40 74250 (3D:TB)
To cycle through all the supported stereo modes:
$ sudo ./tests/testdisplay -3
testdisplay uses cairo to compose the final frame buffer from two separate left and right test images.

Monday, 3 June 2013

Working on more than one line with sed's 'N' command

Yesterday I was asked to help solving a small sed problem. Considering that file (don't look too closely on the engineering of the defined elements):
<root>
  <key>key0</key>
  <string>value0</string>
  <key>key1</key>
  <string>value1</string>
  <key>key2</key>
  <string>value2</string>
</root>
The problem was: How to change value1 to VALUE!. The problem here is that you can't blindly execute a s command matching <string>.*</string>.
Sed maintains a buffer called the "pattern space" and processes commands on this buffer. From the GNU sed manual:
sed operates by performing the following cycle on each line of input: first, sed reads one line from the input stream, removes any trailing newline, and places it in the pattern space. Then commands are executed; each command can have an address associated to it: addresses are a kind of condition code, and a command is only executed if the condition is verified before the command is to be executed.

When the end of the script [(list of sed commands)] is reached, unless the -n option is in use, the contents of pattern space are printed out to the output stream, adding back the trailing newline if it was removed.3 Then the next cycle starts for the next input line.
So the idea is to first, use a /pattern/ address to select the the right <key> line, append the next line to the pattern space (with the N command) and finally run a s command on the buffer now containing both lines:
<key>key1</key>
  <string>value1</string>
And so we end up with:
$ cat input 
<root>
  <key>key0</key>
  <string>value0</string>
  <key>key1</key>
  <string>value1</string>
  <key>key2</key>
  <string>value2</string>
</root>
$ sed -e '/<key>key1<\/key>/{N;s#<string>.*<\/string>#<string>VALUE!<\/string#;}' < input 
<root>
  <key>key0</key>
  <string>value0</string>
  <key>key1</key>
  <string>VALUE!</string
  <key>key2</key>
  <string>value2</string>
</root>