Opened 9 years ago

Closed 8 years ago

#710 closed defect (fixed)

Compilation error on Python 3.2 systems

Reported by: shane Owned by: jelte
Priority: low Milestone: Sprint-20110628
Component: build system Version:
Keywords: Cc:
CVSS Scoring: Parent Tickets:
Sensitive: no Defect Severity: N/A
Sub-Project: DNS Feature Depending on Ticket:
Estimated Difficulty: 2.0 Add Hours to Ticket: 0
Total Hours: 0 Internal?: no

Description

It seems like the Python wrapper doesn't want to compile without warnings under Arch Linux:

/bin/sh ../../../../libtool  --tag=CXX   --mode=compile g++ -DHAVE_CONFIG_H -I. -I../../../..  -I../../../../src/lib -I../../../../src/lib  -I/usr/include/python3.2mu -I/usr/include/python3.2mu -I../../../../ext/asio -I../../../../ext/coroutine -DASIO_DISABLE_THREADS=1 -Wall -Wextra -Wwrite-strings -Woverloaded-virtual -Wno-sign-compare -Werror -fPIC -g -O2 -MT pydnspp_la-pydnspp.lo -MD -MP -MF .deps/pydnspp_la-pydnspp.Tpo -c -o pydnspp_la-pydnspp.lo `test -f 'pydnspp.cc' || echo './'`pydnspp.cc
libtool: compile:  g++ -DHAVE_CONFIG_H -I. -I../../../.. -I../../../../src/lib -I../../../../src/lib -I/usr/include/python3.2mu -I/usr/include/python3.2mu -I../../../../ext/asio -I../../../../ext/coroutine -DASIO_DISABLE_THREADS=1 -Wall -Wextra -Wwrite-strings -Woverloaded-virtual -Wno-sign-compare -Werror -fPIC -g -O2 -MT pydnspp_la-pydnspp.lo -MD -MP -MF .deps/pydnspp_la-pydnspp.Tpo -c pydnspp.cc  -fPIC -DPIC -o .libs/pydnspp_la-pydnspp.o
cc1plus: warnings being treated as errors
In file included from /usr/include/python3.2mu/Python.h:52:0,
                 from pydnspp.cc:27:
/usr/include/python3.2mu/pyatomic.h:59:1: error: unused parameter 'address'
make[1]: *** [pydnspp_la-pydnspp.lo] Error 1
make[1]: Leaving directory `/home/shane/isc/bind10/git/bind10/src/lib/dns/python'
make: *** [all-recursive] Error 1

Arch tends to use bleeding-edge versions of stuff. Here's what is installed:

[shane@shane-eeepc ~]$ python
Python 3.2 (r32:88445, Feb 21 2011, 01:54:01) 
[GCC 4.5.2 20110127 (prerelease)] on linux2
Type "help", "copyright", "credits" or "license" for more information.
>>>
[shane@shane-eeepc ~]$ g++ --version
g++ (GCC) 4.5.2 20110127 (prerelease)
Copyright (C) 2010 Free Software Foundation, Inc.
This is free software; see the source for copying conditions.  There is NO
warranty; not even for MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.

I can work around this by removing -Werror in the Makefile in that directory.

Subtickets

Change History (17)

comment:1 Changed 9 years ago by jinmei

"This is not our problem" (TM).

Considering this is specific to cutting-edge version of python (3.2
doesn't even have pyatomic.h), I think the right solution is to report
it to the python developer community and have them fix it.

And, actually, there seems to be a report about it already:
http://bugs.python.org/issue11147

Do we still want to keep this ticket open?

comment:2 Changed 9 years ago by jinmei

  • Owner changed from jreed to shane
  • Status changed from new to assigned

comment:3 Changed 9 years ago by jinmei

Oops, I meant "3.1 doesn't even have pyatomic.h". Sorry for the noise and possible confusion.

comment:4 Changed 9 years ago by shane

  • Resolution set to wontfix
  • Status changed from assigned to closed

Python 3.2 was released almost a month ago! It's practically stale! ;)

My concern is that people will start to have build problems with BIND 10 on systems as Python 3.2 gets adopted and the fix is not yet added. :(

If that happens, we may need to check for Python 3.2 and disable -Werror on those systems. Preferably only for Python-related compilation, but we can do it for the whole system if that's too much work.

I'll close the ticket. We can reopen it if we need to.

comment:5 Changed 9 years ago by stephen

  • Milestone A-Team-Task-Backlog deleted

Milestone A-Team-Task-Backlog deleted

comment:6 Changed 9 years ago by shane

  • Defect Severity set to N/A
  • Milestone set to Year 3 Task Backlog
  • Resolution wontfix deleted
  • Status changed from closed to reopened
  • Sub-Project set to DNS

Hm... we may need to address this in some way, since the Python bug seems to be going nowhere.

comment:7 Changed 9 years ago by shane

  • Summary changed from Compilation error on Arch Linux to Compilation error on Python 3.2 systems

comment:8 Changed 9 years ago by shane

  • Milestone changed from Year 3 Task Backlog to Next-Sprint-Proposed

comment:9 Changed 8 years ago by stephen

  • Estimated Difficulty changed from 0.0 to 2

comment:10 Changed 8 years ago by stephen

  • Milestone changed from Next-Sprint-Proposed to Sprint-20110628

comment:11 Changed 8 years ago by jelte

  • Owner changed from shane to jelte
  • Status changed from reopened to assigned

comment:12 follow-up: Changed 8 years ago by jelte

  • Owner changed from jelte to UnAssigned
  • Status changed from assigned to reviewing

Three commits, the most important one is the first; I've added a configure check that checks if we need -Wno-unused-parameter to include Python.h. If so, it is added to PYTHON_INCLUDES (not the very best place to put it, but this way we do know it'll only be used for things that actually use Python.h, and we don't need to add any workarounds to the various Makefiles).

The second fix is for a failing test (and presumably for failing code); apparently one of the APIs changed (or rather, a default parameter in a call), so I added an explicit str() around it.

And lastly, python 3.2 now stores .pyc files in __pycache__ directories, which need to be removed (for distcheck, mainly). This does update a lot of makefiles. I could not find a way to completely remove directories through a magic automake variable, so I added CLEANDIRS and a clean-local target in all directories containing python code.

comment:13 Changed 8 years ago by jinmei

  • Owner changed from UnAssigned to jinmei

comment:14 Changed 8 years ago by jinmei

one obvious point first: we'll probably need a changelog entry for this.

comment:15 in reply to: ↑ 12 Changed 8 years ago by jinmei

Replying to jelte:

Three commits, the most important one is the first; I've added a configure check that checks if we need -Wno-unused-parameter to include Python.h. If so, it is added to PYTHON_INCLUDES (not the very best place to put it, but this way we do know it'll only be used for things that actually use Python.h, and we don't need to add any workarounds to the various Makefiles).

Unfortunately this doesn't (always) work. (and even if it does, I'd
probably object to abusing PYTHON_INCLUDES for this purpose:-). In my
system -Wno-something must be placed after the offending warning flag
(normally -Wall or -Wextra). I thought it's quite common, so it's
actually surprising this worked for you:-)

I committed a counter proposal to the branch: 97131f9. I also moved
the check within a g++ (and its variants) specific block in order to
not disturb other compilers that don't understand -Wxxx.

The second fix is for a failing test (and presumably for failing code); apparently one of the APIs changed (or rather, a default parameter in a call), so I added an explicit str() around it.

I suspect the revised code (with Python 3.2) doesn't produce the
expected result. The problem is that (as you pointed out) in Python
3.2 xml.etree.ElementTree?.tostring() always returns bytes, not a
string (it seems to be an intentional change, see
http://bugs.python.org/issue10942). But simply wrapping a bytes
object with str() results in a string in the form of "b'original
string'", so it would result in "b'<foo>bar</foo>'" etc, instead of
"<foo>bar</foo>".

Assuming my concern is correct, I made a counter proposal and
committed it (ed6ec07).

And, this makes me think it indicates tests for stats_httpd are not
sufficient. We should have a test to see if StatsHttpd?.xml_handler()
produces expected results. Then (again assuming my concern is
correct) you should have been able to notice the first fix wasn't
sufficient. This is not a problem of this branch per se, but for the
original code of stats. So I suggest we create a separate ticket for
that.

And lastly, python 3.2 now stores .pyc files in __pycache__ directories, which need to be removed (for distcheck, mainly). This does update a lot of makefiles. I could not find a way to completely remove directories through a magic automake variable, so I added CLEANDIRS and a clean-local target in all directories containing python code.

This one looks okay. It would be nicer if we have a common template
for python Makefile.am's including this one so that we won't have to
copy-paste this block every time we create a new Makefile.am for a new
python lib/program, but for now I'm okay with it.

Finally, I think we should have at least one buildbot that uses Python
3.2. I suggest we create a separate ticket for that.

If you are okay with my changes, the only remaining point is the
missing changelog.

comment:16 Changed 8 years ago by jinmei

  • Owner changed from jinmei to jelte

comment:17 Changed 8 years ago by jelte

  • Resolution set to fixed
  • Status changed from reviewing to closed

I'm okay with these changes/additions. I've created the a followup tickets (#1026, the other one was already created at #1021).

Thanks for the fixes. Merged as discussed on the call, closing ticket.

Note: See TracTickets for help on using tickets.