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

Document that SAGE_DEBUG is three-state #13865

Closed
vbraun opened this issue Dec 26, 2012 · 22 comments
Closed

Document that SAGE_DEBUG is three-state #13865

vbraun opened this issue Dec 26, 2012 · 22 comments

Comments

@vbraun
Copy link
Member

vbraun commented Dec 26, 2012

Currently, SAGE_DEBUG is documented as

If it is unset (the default) or set to “yes”, then debugging is turned on.

This is fine if you just add gcc -g, but the utility of having a real Python debug build shows that there should be more to debugging than just adding symbols. However, this has a real performance impact and should not be the default. So I propose that SAGE_DEBUG have three possible values:

  • SAGE_DEBUG=no means no debugging symbols (no gcc -g), which mostly saves disk space.
  • SAGE_DEBUG=yes builds debug versions if possible (in particular, Python and Singular). These will be notable slower.
  • Anything else (including unset SAGE_DEBUG) is the same as the old default, compile with debugging symbols but no debugging options that influence performance.

CC: @nexttime

Component: documentation

Author: Volker Braun

Reviewer: Jeroen Demeyer

Merged: sage-5.6.beta2

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

@vbraun
Copy link
Member Author

vbraun commented Dec 26, 2012

Author: Volker Braun

@vbraun
Copy link
Member Author

vbraun commented Dec 26, 2012

comment:2

I've posted to sage-devel since this might be of interest to a larger audience: https://groups.google.com/d/topic/sage-devel/Lmo6mmDylj4/discussion

@nbruin
Copy link
Contributor

nbruin commented Dec 26, 2012

comment:3

Ah, 'anything else' sounds like a great 3rd case.

Is there any standard on whether "yes/no", "!Yes/No", "true/false", "on/off" etc. is preferred? I always end up having to google the environment variables and then hope and check later that the value I used had the desired effect.

@vbraun
Copy link
Member Author

vbraun commented Dec 26, 2012

comment:4

I think all documented environment variables use yes/no for a binary setting. In Python we always use !True/False since those are Python bools.

@nexttime
Copy link
Mannequin

nexttime mannequin commented Dec 26, 2012

comment:5

Component: Documentation?

@vbraun
Copy link
Member Author

vbraun commented Dec 26, 2012

comment:6

You want to file it under something else? It is just a doc patch.

@simon-king-jena
Copy link
Member

comment:7

Is this ticket about fixing documentation of SAGE_DEBUG, or is it about proper behaviour/use of SAGE_DEBUG? If it is the latter, then I suggest one should also raise the singular-spkg from #13731 to a new patch level and replace/add the environment variable SINGULAR_XALLOC by SAGE_DEBUG.

@vbraun
Copy link
Member Author

vbraun commented Dec 26, 2012

comment:8

I intended it just as a discussion and fix for the docstring. But if you want to add a new singular spkg then thats fine with me, too.

@nexttime
Copy link
Mannequin

nexttime mannequin commented Dec 26, 2012

comment:9

Replying to @vbraun:

You want to file it under something else? It is just a doc patch.

Well, the documentation is certainly wrong (or not up-to-date), as the default is actually rather no. (We decided to add debug symbols by default since that doesn't have a performance impact -- except for larger files and hence probably slower start-up/loading, but one can strip these symbols later. SAGE_DEBUG=yes in contrast does -- depending on the spkg -- e.g. disable optimization, add extra code [probably with some runtime overhead] , so at least may have a major performance impact.)

W.r.t. the component: I just meant changing the meaning certainly extends to more than a documentation change (cf. the ticket's title; otherwise I'd call it "Clarify SAGE_DEBUG", say).

Actually, what you propose is IMHO what we already have (or shouldTM have)... ;-)

So just correcting or clarifying the documentation may indeed be sufficient. :-) (But I'd then change the ticket's title, as mentioned.)

@vbraun vbraun changed the title Make SAGE_DEBUG three-state Document that SAGE_DEBUG is three-state Dec 26, 2012
@nexttime
Copy link
Mannequin

nexttime mannequin commented Dec 26, 2012

comment:11

Ok...

s/notable/notably/

s/to pinpoint/to e.g. pinpoint/

@nexttime
Copy link
Mannequin

nexttime mannequin commented Dec 26, 2012

comment:12

Not sure whether the Sage Developer's Guide (on spkgs) is up-to-date right now.

@nexttime
Copy link
Mannequin

nexttime mannequin commented Dec 26, 2012

comment:13

We should probably also mention that (or how) debug symbols can be stripped afterwards...

@vbraun
Copy link
Member Author

vbraun commented Dec 26, 2012

comment:14

Fixed. I grepped the docs and this is the only place that mentions SAGE_DEBUG. No idea how to strip besides doing it yourself, and that definitely shouldn't be covered in the install guide.

@nexttime
Copy link
Mannequin

nexttime mannequin commented Dec 26, 2012

comment:15

Replying to @vbraun:

Fixed. I grepped the docs and this is the only place that mentions SAGE_DEBUG.

Oh. I thought it was at least in some spkg-installtemplate.

No idea how to strip besides doing it yourself, and that definitely shouldn't be covered in the install guide.

There used to be sage [-]-strip, but that got removed at some point because it did more than just stripping debug symbols (which also failed on some -- presumably MacOS X -- systems) IIRC.

(We now -- or still -- have the "micro-release" make target though, see also $SAGE_LOCAL/bin/sage-micro_release...)

@jdemeyer
Copy link

comment:16

The patch needs a proper commit message.

@vbraun
Copy link
Member Author

vbraun commented Dec 27, 2012

comment:17

Fixed

@jdemeyer
Copy link

Reviewer: Jeroen Demeyer

@jdemeyer
Copy link

comment:18

Fine for me. Actually changing packages to be consistent with this documentation should be done in separate tickets.

@nexttime
Copy link
Mannequin

nexttime mannequin commented Dec 27, 2012

comment:19

Replying to @jdemeyer:

changing packages to be consistent with this documentation should be done in separate tickets.

Of course.

@jdemeyer
Copy link

Merged: sage-5.6.beta2

@jdemeyer
Copy link

Attachment: trac_13865_SAGE_DEBUG_documentation.patch.gz

Updated patch

@jdemeyer
Copy link

comment:21

Rebased to #13032.

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