[Date Prev][Date Next] [Thread Prev][Thread Next] [Date Index] [Thread Index]

Re: dpkg semi-hijack - an announcement (also, triggers)



On Sun, Mar 09, 2008 at 10:38:44PM -0500, Steve Greenland wrote:
> On 09-Mar-08, 19:30 (CDT), Daniel Stone <daniel@fooishbar.org> wrote: 
> > I was going to ask on which grounds exactly you were judging the dpkg
> > team's competence (and that of iwj's: have you reviewed the branch
> > yourself? can you confidently say that it's all fine?),
> The problem is not the dpkg team has reviewed the patch and had problems
> with it, it's that they've ignored it for 6 months.

That's not the full picture. 

Ian's been trying to be involved in the dpkg team for longer than six
months, and not been incredibly effective at that; eg from July:

   I will of course be careful about controversial changes.  For example,
   I'll refrain from committing my formatting fixup from #375711 until
   we've come to a conclusion.  I hope you can trust my judgement about
   what would be a controversial change.

   -- http://lists.debian.org/debian-dpkg/2007/07/msg00013.html 

Ian then reposted the triggers spec and published a git tree for the patch,
with the following comment:

   I'd appreciate it if no-one reformatted it or change the style.  If
   you have disagreements with the code please discuss it here on the
   list before making any changes.  Checking a significantly changed
   version into the main Debian git will cause snarl-ups because our
   merges will constantly need resolving.

   -- http://lists.debian.org/debian-dpkg/2007/08/msg00014.html
   -- http://lists.debian.org/debian-dpkg/2007/08/msg00009.html

Ian then reimplemented the status file parsing using flex, in a way he
reported was faster, but hadn't tested, and suggested it get included in
sid.

   I have written over the weekend a replacement for lib/fields.c and
   most of lib/parse.c, which uses flex (and flex start conditions) to
   generate a table-driven scanner-cum-parser.  I haven't tested this
   fully for correctness yet, but I have done basic functionality tests
   and some performance tests.

   ...

   The downside is that it's 100K longer in code size. ...

   Anyway, I think we should deploy the flex-based scanner in sid (after
   I've tested it a bit more) and then think at our leisure about how to
   improve the shared code situation.

   -- http://lists.debian.org/debian-dpkg/2007/08/msg00028.html

Guillem replied to that patch within about four days, but the git tree
Ian developed it in was based on the triggers tree, so can't be easily
merged independently.

In October, Joey proposed his v3 source package format, which is one of
the other major outstanding patches. I don't believe it has any stylistic
problems, but it still seems to be being reinvented/redisgned, rather than
being applied.

   -- http://lists.debian.org/debian-dpkg/2007/10/msg00009.html
   -- http://lists.debian.org/debian-dpkg/2008/02/msg00012.html
      http://lists.debian.org/debian-dpkg/2008/02/msg00079.html
      http://lists.debian.org/debian-dpkg/2008/02/msg00131.html

Also in October, Colin Watson asked what the status of triggers was. Ian
replied:

   The implementation of triggers is not going to change incompatibly.
   It's well-tested now in Ubuntu and should just be merged into Debian's
   dpkg.

   -- http://lists.debian.org/debian-dpkg/2007/10/msg00107.html

Guillem replied to that:

   Err, well of course it's highly desirable to not do such kind of
   changes without good reason, but I don't think the fact that it has been
   deployed in a derivative distro means that we should blindly merge any
   such code drops without review and/or possible changes.

   -- http://lists.debian.org/debian-dpkg/2007/10/msg00132.html

Ian replied to that:

   Don't be ridiculous.  This is offensive to me personally.

   This code has been
    * designed with extensive input from this mailing list in Debian fora
    * written by dpkg's original maintainer
    * tested extensively
    * available for merging for several months

   -- http://lists.debian.org/debian-dpkg/2007/10/msg00138.html

Guillem didn't reply to that; his next comment on triggers a couple of weeks
later was, in response to Ian:

   > This isn't a matter of preference, I'm afraid.  I reverted this
   > because the change was wrong.  NULL is incorrect in that context (a
   > stdarg function expecting a char*), because it may be #define'd to 0.

   That's true, but then I agree with others [0] that an environment that
   defines NULL to 0 (even if the standard allows that) is not sane. Such
   ancient environment will also not be able to cope with modern software
   like gtk, anyway.

   [0] http://lwn.net/Articles/93577/

   -- http://lists.debian.org/debian-dpkg/2007/10/msg00228.html

There were a couple of messages that sounded promising in regards to a
productive conclusion at the end of that month:

   -- http://lists.debian.org/debian-dpkg/2007/10/msg00230.html
   -- http://lists.debian.org/debian-dpkg/2007/10/msg00231.html

Nothing much happened in Nov or Dec; both Ian and Guillem posted about
random other stuff, a couple of uploads were made.

In January, Tollef posted about his file exclusion branch, for review.
Guillem did some review and proposed merging it for .17; afaics Ian
couldn't find the git repository at first attempt, then offered to review,
but never did.

   -- http://lists.debian.org/debian-dpkg/2008/01/msg00011.html
      http://lists.debian.org/debian-dpkg/2008/01/msg00100.html
      http://lists.debian.org/debian-dpkg/2008/01/msg00118.html
      http://lists.debian.org/debian-dpkg/2008/01/msg00126.html
      http://lists.debian.org/debian-dpkg/2008/01/msg00140.html

At that point we get into February which is the point where Ian started
pressuring the dpkg team on both -devel and -dpkg to apply his triggers
patch and add him as a maintainer, in what quickly became the 'git
bikeshedding' thread.

   -- http://lists.debian.org/debian-dpkg/2008/02/msg00068.html
      http://lists.debian.org/debian-dpkg/2008/02/msg00082.html
      http://lists.debian.org/debian-dpkg/2008/02/msg00083.html
      http://lists.debian.org/debian-dpkg/2008/02/msg00086.html
      http://lists.debian.org/debian-dpkg/2008/02/msg00087.html
      http://lists.debian.org/debian-dpkg/2008/02/msg00109.html
      http://lists.debian.org/debian-dpkg/2008/02/msg00110.html
      http://lists.debian.org/debian-dpkg/2008/02/msg00111.html
      http://lists.debian.org/debian-dpkg/2008/02/msg00112.html
      http://lists.debian.org/debian-dpkg/2008/02/msg00113.html
      http://lists.debian.org/debian-dpkg/2008/02/msg00119.html
      http://lists.debian.org/debian-dpkg/2008/02/msg00120.html
      http://lists.debian.org/debian-dpkg/2008/02/msg00122.html
      http://lists.debian.org/debian-dpkg/2008/02/msg00123.html
      http://lists.debian.org/debian-dpkg/2008/03/msg00022.html
      http://lists.debian.org/debian-dpkg/2008/03/msg00023.html
      http://lists.debian.org/debian-dpkg/2008/03/msg00024.html
      http://lists.debian.org/debian-dpkg/2008/03/msg00025.html
      http://lists.debian.org/debian-dpkg/2008/03/msg00027.html
      http://lists.debian.org/debian-dpkg/2008/03/msg00030.html
      http://lists.debian.org/debian-dpkg/2008/03/msg00039.html
      http://lists.debian.org/debian-dpkg/2008/03/msg00040.html
      http://lists.debian.org/debian-dpkg/2008/03/msg00053.html

That's now escalated into the "semi" hijack attempt.

> I don't approve of IanJ's hijack attempt, but in this he's got a
> legitimate complaint. 

Against the wishes of, afaict, Guillem and Raphael, Ian's made applying
his triggers patch dependent on:

	- reversion to two space indenting

	- a policy of bulk conversion of intentation style, instead of
	  the current policy of gradual conversion from two-space to
	  four-space

	- having explicit casts to (char*) of NULL in order to support
	  some non-Debian architectures

	- having the git log not be bisectable or particularly meaningful
	  except historically

	- having Ian be part of the dpkg team

If he hadn't done that, afaict the patch would've been handled pretty
much the way Tollef's was.

I don't believe anyone on the dpkg team at any point gave Ian a definite
answer on any of the above issues over the past months; though I doubt
he would have accepted a "no" on any of them anyway.

For reference, the changes file Ian uploaded included the following changes
committed by Guillem:

   * Replace strdup plus error checking usage with a new m_strdup function.
     Closes: #379028
   * Add new keybinding in dselect to restore all selections back to
     whatever's currently installed. Closes: #151540
     Thanks to Colin Watson.
   * Use system timersub and fix timeval normalization in multiplication in
     start-stop-daemon. Thanks to Andreas P??hlsson. Closes: #462225
   * Cosmetic fixes to start-stop-daemon output and man page. Document that
     --chuid will change the group even if it has not been specified. Add
     EXIT STATUS and EXAMPLE sections to man page. Thanks to Justin Pryzby.
   * Add Raphael Hertzog to Uploaders, and remove Brendan O'Dea and
     Christian Perrier with their permission.
   * Use functions from libcompat when those are not provided by the system.
   * Change dpkg-gencontrol to not output the Homapage field on udeb.
   * Reintroduce 'no-debsig' back in dpkg.cfg to avoid failing to install any
     package when debsig-verify is installed. Closes: #311843
   * Fix some small memory leaks. Closes: #469520
     Thanks to Sean Finney.

Cheers,
aj

Attachment: signature.asc
Description: Digital signature


Reply to: