Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Remove debugging output from Python command line #13867

Closed
jpflori opened this issue Dec 27, 2012 · 31 comments
Closed

Remove debugging output from Python command line #13867

jpflori opened this issue Dec 27, 2012 · 31 comments

Comments

@jpflori
Copy link

jpflori commented Dec 27, 2012

The extra debugging output "[number of refs]" before each command line prompt breaks the pynad build. the problem is that the test used by the AX_PYTHON_DEVEL autoconf macro to check for distutils fails when Python is built with pydebug.

Also, breaks the Sage test suite.

I'm just removing this extra output. Its not that useful, really, since object counts in Sage are always pretty large and widely fluctuating with stuff like coercion running in the background. It also doesn't offer any extra info over gc.get_count().

Install http://www.stp.dias.ie/~vbraun/Sage/spkg/python-2.7.3.p5.spkg

CC: @burcin @vbraun @simon-king-jena

Component: packages: standard

Keywords: python spkg debug

Author: Volker Braun

Reviewer: Jean-Pierre Flori

Merged: sage-5.7.beta4

Issue created by migration from https://trac.sagemath.org/ticket/13867

@jpflori jpflori added this to the sage-5.7 milestone Dec 27, 2012
@vbraun
Copy link
Member

vbraun commented Dec 27, 2012

comment:1

I'm thinking we should just patch Python to not print out extra crap. Otherwise we have basically no hope of running the Sage doctests, no?

@jpflori
Copy link
Author

jpflori commented Dec 27, 2012

comment:2

Good point.
And that should be even easier.

@jpflori
Copy link
Author

jpflori commented Dec 27, 2012

comment:3

In fact I do not really agree, especially now that I've checked that the crappy additional Python output does not pertub the doctest framework (surely because its spit on stderr and not stdout).

@vbraun
Copy link
Member

vbraun commented Dec 27, 2012

comment:4

Surely the sage pexpect interface to itself will not work. Or various checks in sage.tests.cmdline that check that stderr is empty.

@vbraun
Copy link
Member

vbraun commented Dec 28, 2012

Author: Volker Braun

@vbraun

This comment has been minimized.

@vbraun
Copy link
Member

vbraun commented Dec 28, 2012

comment:5

I'm taking over this ticket to patch up python

@vbraun
Copy link
Member

vbraun commented Dec 28, 2012

Changed keywords from pynac spkg debug to python spkg debug

@vbraun vbraun changed the title Fix Pynac spkg so that it builds on top of a debug build of python Remove debugging output from Python command line Dec 28, 2012
@vbraun

This comment has been minimized.

@simon-king-jena
Copy link
Member

comment:7

Just to be on the safe side: To use this one with SAGE_DEBUG=yes on top of an existing Sage install, one needs to install this new python spkg, then (re-)install the new cython spkg from #13832, and then sage -ba, right?

@vbraun
Copy link
Member

vbraun commented Dec 28, 2012

comment:8

to get a working Sage you also need a patch to the Sage library (will post here soon) and a fixed Singular spkg... stay tuned ;-)

@vbraun
Copy link
Member

vbraun commented Dec 28, 2012

comment:9

Patch to the Sage library is at #13868

@simon-king-jena
Copy link
Member

comment:10

We now have an abundance of tickets needed to make SAGE_DEBUG work. Shouldn't there be a meta-ticket, listing all relevant single-purpose tickets (#13876, #13867, #13868, #13832, #13864, and what else)?

@vbraun
Copy link
Member

vbraun commented Dec 28, 2012

comment:11

#13864 is now the meta-ticket.

@simon-king-jena
Copy link
Member

comment:12

Replying to @simon-king-jena:

We now have an abundance of tickets needed to make SAGE_DEBUG work. Shouldn't there be a meta-ticket, listing all relevant single-purpose tickets (#13876, #13867, #13868, #13832, #13864, and what else)?

FWIW: I created a metaticket at #13877.

@simon-king-jena
Copy link
Member

comment:13

Replying to @simon-king-jena:

Replying to @simon-king-jena:

We now have an abundance of tickets needed to make SAGE_DEBUG work. Shouldn't there be a meta-ticket, listing all relevant single-purpose tickets (#13876, #13867, #13868, #13832, #13864, and what else)?

FWIW: I created a metaticket at #13877.

OK, forget about that, Volker was faster with his change to #13864.

@jdemeyer
Copy link

comment:14

In spkg-install, are you sure you don't mean CFLAGS and/or CXXFLAGS instead CPPFLAGS? Because CPPFLAGS normally means "C preprocessor flags".

@jdemeyer
Copy link

Dependencies: #13864

@jdemeyer
Copy link

Changed dependencies from #13864 to none

@vbraun
Copy link
Member

vbraun commented Dec 30, 2012

comment:17

The spkg-install already used CPPFLAGS before I edited it, so I stuck with it. It does get picked up on the gcc command line, fwiw.

@jdemeyer
Copy link

comment:18

Yes, it does use CPPFLAGS for its intended purpose. The -I option is related to preprocessing and belongs in $CPPFLAGS. Flags like -g or -O0 do not.

Note that adding -I$SAGE_LOCAL/include to CPPFLAGS is actually unneeded since that path gets picked up automatically because of the $CPATH environment variable. So you might as well remove

CPPFLAGS="-I$SAGE_LOCAL/include $CPPFLAGS"
export CPPFLAGS

@jdemeyer
Copy link

comment:19

Replying to @vbraun:

The spkg-install already used CPPFLAGS before I edited it

It also uses CFLAGS by the way:

export CFLAGS="$CFLAGS `testcflags.sh -fwrapv`"

@jpflori
Copy link
Author

jpflori commented Dec 31, 2012

comment:20

I'm ok with the solution Volker proposes, we don't need that "extra" output, but I still think that Pynac should be able to deal with a vanilla Python debug build.
So I'm just posting that here so that Burcin can take notice (I'll post on pynac-devel as well).

@vbraun
Copy link
Member

vbraun commented Dec 31, 2012

Attachment: python-p5.patch.gz

diff for review only

@vbraun
Copy link
Member

vbraun commented Dec 31, 2012

comment:21

I've removed the superfluous CPPFLAGS. Also --with-pydebug implies -O0 -g so we don't have to set it by hand.

@simon-king-jena
Copy link
Member

comment:23

I use the package successfully with SAGE_DEBUG=yes.

Some criticism: SPKG.txt contains a section "Patches", but the new patch disable_print_refs_debug.patch is not mentioned.

The content of environment variables is sometimes tested like

if ["x$SAGE_DEBUG"="xyes"]; then

if I am not mistaken, but here it is

if ["$SAGE_DEBUG"="yes"]; then

Is that a problem?

@vbraun
Copy link
Member

vbraun commented Jan 21, 2013

comment:24

I've added a line documenting the disable_print_refs_debug.patch

The xyes thing is a historical artifact.

@jpflori
Copy link
Author

jpflori commented Jan 29, 2013

Reviewer: Jean-Pierre Flori

@jpflori
Copy link
Author

jpflori commented Jan 29, 2013

comment:25

Looks good.

If you really wants me to rant, I'd say:

  • first, there is a useless line at the end of the new patch for some emacs generated tmp file.
  • or even less problematic, you could remove the historical artifacts from spkg-install while you're at it.

@jdemeyer
Copy link

comment:26

Server down :-(

@jdemeyer
Copy link

jdemeyer commented Feb 9, 2013

Merged: sage-5.7.beta4

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

4 participants