Changes between and of Version 1Version 6Ticket #5907

11/09/2012 03:53:47 PM (6 years ago)

Replying to teratorn:

It's great to know this issue has be languishing here for 3 months, and missed inclusion in Twisted 12.2. Because apparently we need unit tests to tell us that including the right C header files is a good thing to do.

teratorn, snarky comments about the review process really aren't helpful, and I would thank you to please never do this on a ticket again. If you want to complain, come up with some constructive criticism, not sarcasm, and put it in a dedicated place for such discussion, like a new, dedicated thread on the mailing list, or a chat on the IRC channel; don't derail an existing discussion.

Better yet though, just ask a question without the pointless rudeness. If you don't know how the review process works, or what our acceptance criteria are, or why they're there, I'm always happy to explain, provide references, et cetera.

exarkun, what is your point exactly? and what can we do to resolve it? The unit test for a build problem is the build system itself (afaict). If the build system moves forward toward a more working state, then I consider that "test passed."

In the broader context, exarkun's point is that OpenBSD is not a supported platform and therefore we don't know anything about this patch. As you can see on our full buildbot status page, we have only unsupported builders on OpenBSD, and they've been offline for hundreds of builds; we really have no idea whether this platform works.

In a much narrower sense, though, all exarkun asked for at this point in the conversation was assurance that somebody, somewhere had even run the existing unit tests with the change here. In the last 6 weeks, nobody, including yourself, thought this bug was important enough to reply.

We are never, ever, ever going to change Twisted's quality-control process to allow random people who claim to have problems that we don't have the infrastructure to reproduce to apply dubious changes with no effect. Maybe this breaks compilation on some other random, unsupported platform. Every change is a risk, especially changes to C code. This change is no different. Maybe it breaks builds on a supported platform. Maybe it's detrimental to performance. The fact is that we don't know, and there's no clear indication of a benefit; someone has to do the work necessary to demonstrate a benefit.

If you care about OpenBSD, please work with the OpenBSD buildbots' maintainer (soyt, apparently?) to get them back online, or contribute some new ones yourself, and work towards getting a full clean run of the tests on those bots, so we can put them in the "supported" category and prevent this sort of breakage in the future.

I will assume that you care enough about this bug to get it fixed now though, so I'll reassign it to you.

(epdittmer, if you intend to deal with this feedback soon, and want to take it back, by all means please do so.)


  • Ticket #5907

    • Property Cc teratorn added
    • Property Owner set to teratorn
  • Ticket #5907 – Description

    v1 v6  
    8 cc -pthread -fno-strict-aliasing -O2 -pipe -DNDEBUG -O2 -pipe -DTHREAD_STACK_SIZE=0x20000 -fPIC -fPIC -I/usr/local/include/python2.7 -c twisted/python/sendmsg.c -o build/temp.openbsd-5.1-amd64-2.7/twisted/python/sendmsg.o
     8cc -pthread -fno-strict-aliasing -O2 -pipe -DNDEBUG -O2 -pipe \
     9    -DTHREAD_STACK_SIZE=0x20000 -fPIC -fPIC -I/usr/local/include/python2.7 \
     10    -c twisted/python/sendmsg.c \
     11    -o build/temp.openbsd-5.1-amd64-2.7/twisted/python/sendmsg.o
    912twisted/python/sendmsg.c: In function 'sendmsg_sendmsg':
    1013twisted/python/sendmsg.c:174: error: array type has incomplete element type