Wednesday, 22 July 2009

wine-patches the black hole of code?

The Problem


On Monday, I took part in the first Wine Bug Day (one of hopefully many). It was a great success, I managed to rope in some support from some friends specifically Sean and Yorik both of whom are Wine users, not Wine developers (yet!).

And it wasn't just us, there was of course Scott Ritchie who organized the thing, but also several users popped up in #winehackers asking for others to change the status of their bugs. It was all quite exciting in a weird kind of way.

It was because of this buzz of excitement that I continued my bug triaging efforts. I knew that Autocad 2008 was supposed to *almost* work, so I thought I'd download the trial and give it a spin in the latest Wine. Surprisingly it worked pretty well, once I'd installed the prerequisites using winetricks that is. The installation went smoothly but on the first load of Autocad all of the icons were black - I've seen this issue before and I knew that using a native gdiplus.dll would fix it, but of course this is a bug in Wine so I thought I'd look at little closer. Sure enough there were 2 gdiplus calls that Autocad uses which are just stubs in Wine: GdipCreateHICONFromBitmap and GdipCreateHBITMAPFromBitmap.

Now, this is where I confirmed (I kinda knew, and mentioned it in a previous blog post) that there is a real problem with sending patches to Wine. Google showed me these two links. The work has already been done before! But it's not in Wine...

I'm not criticizing the fact that these patches were rejected, even skimming over them I can see a few issues that could do with fixing before they are accepted. What I do get annoyed about is these people have spent hours of their lives developing something, they've gone through the effort of creating patches and emailing them to the wine mailing list, and the first patch didn't even get a reply! Yes, they could join #winehackers and chase for comments, yes, they could take on board the comments and resubmit, but why? They've already put in the extra effort to get the patches out there, isn't a bit unfair to expect them to jump through hoops to get the patches in to Wine? I think so.

I know how frustrating it is to get patches into Wine. In fact, of the patches I have submitted I think only one went in first time, and that was only because it was a simple addition to an existing structure. It's taken me 3 attempts to get even simple patches in, this is not encouraging for a developer looking for a project to devote their time to. Even I have just abandoned patches, simply because I couldn't be arsed to go through the loop again for a minor minor thing. So let's break down the actual problems:

  • Patches rarely receive feedback - positive or negative - on the wine-patches list

  • Patches that are rejected aren't labelled as such, or publicly given a reason for why they were

  • Hundreds of patches don't get in, that just need a tweak or two, that would REALLY improve the wine experience

  • These failed patches can't easily be seen by anyone unless they know exactly what to Google for


The solution


What we need is a public notice board for these failed patches. We need a way for everyone to see why a patch was rejected and we need a central point to find these patches so they can be cleaned and resubmitted. Time permitting that's what I intend to develop, and here's how it might work.

wine-patches holds every patch that is sent externally to Wine, whether it was accepted or not. Given a month and year, we can programmatically dig out a list of all the emails that were submitted by parsing the index page at http://www.winehq.org/pipermail/wine-patches/{YEAR}-{MONTH}/thread.html - this is our master list. We can follow those links and also, in code, read the bodies of the patches. Some of them will be attachments, others will be embedded in the page. Either way, we can parse the majority of them, some emails won't have a patch attached we can strip these ones out. At this point we have a list of patches submitted, now we just need to know whether they were committed....

Fortunately, wine-cvs holds the answer. wine-cvs contains emails for every commit to the master repository. What we need to do is match the patches in our master list, to the ones in wine-cvs. If we can't find a close match then we can assume the patch never made it in. So, in theory we can generate a webpage with this information, with the possibility of devs submitting feedback against the patches so they can be cleaned up and resubmitted. We can go back pretty far in time with this, I would say at least 6-8 months back before the patches just become too old to be worth bothering with. Stay tuned...


Edit...
I'd just like to clarify some things. Firstly, I'm not attacking Wine here I'm just pointing out that there is code that slips through the net on wine-patches that could be cleaned up and resubmitted if there was a central point to find these patches and a means to find out what was wrong with them. I'm not just aimlessly moaning either, I've stated that I hope to develop a web app to do just that.

Secondly, I have contributed to Wine A LOT over the last couple of years; submitting patches, application tests, contributing to discussions on wine-devel and aiding developers and community members like Thunderbird, Stefand and Yokozar with their contributions, and I hope to continue to do so. I am not new to Wine development.

2 comments:

  1. There have been a few attempts in academia to track accepted and rejected patches in OSS. Perhaps the most successful is Bird et al. http://wwwcsif.cs.ucdavis.edu/~bird/papers/bird2007dps.pdf

    I've been looking at peer review in OSS for a few years: http://helium.cs.uvic.ca/pubs/Rigby2008ICSE.pdf

    and found your commentary interesting :-)

    ReplyDelete
  2. ... you just have to keep resending, with improvements each time, until
    it's accepted. The process
    gets a lot easier after the first few,
    because the maintainer then trusts you.

    People sometimes link to their wine-patches
    post from bugzilla, which works pretty well
    for finding patches later. A
    repository of old patches has been discussed many times, but shot down because many
    patches go stale quickly, and if the
    author is serious and the patch is good, it
    usually goes in after a few revisions.

    See also discussion at
    http://www.winehq.org/pipermail/wine-devel/2009-July/077562.html

    One thing that'll help is if we get Patchwatcher back online; see
    http://wiki.winehq.org/PatchWatcher

    ReplyDelete