Opened 11 years ago
Closed 9 years ago
#10138 closed defect (worksforme)
Doctest failure in trace.py with 64-bit OS X build
Reported by: | mpatel | Owned by: | GeorgSWeber |
---|---|---|---|
Priority: | major | Milestone: | sage-duplicate/invalid/wontfix |
Component: | build | Keywords: | |
Cc: | kcrisman, jhpalmieri, leif | Merged in: | |
Authors: | Reviewers: | David Roe | |
Report Upstream: | N/A | Work issues: | |
Branch: | Commit: | ||
Dependencies: | Stopgaps: |
Description
With a 64-bit build (export SAGE64=yes
) of Sage 4.6.alpha3 on bsd.math (OS X 10.6 i386), I get this doctest failure:
$ ./sage -t -long -force_lib devel/sage/sage/misc/trace.py Detected SAGE64 flag Building Sage on OS X in 64-bit mode sage -t -long -force_lib "devel/sage/sage/misc/trace.py" ********************************************************************** File "/Users/buildbot/build/sage/bsd-2/bsd_64_full/build/sage-4.6.alpha3/devel/sage/sage/misc/trace.py", line 61: sage: print s.before[s.before.find('-'):] Expected: ---... ipdb> c 2 * 5 Got: -bit mode Creating SAGE_LOCAL/lib/sage-64.txt since it does not exist Detected SAGE64 flag Building Sage on OS X in 64-bit mode ---------------------------------------------------------------------- | Sage Version 4.6.alpha3, Release Date: 2010-10-08 | | Type notebook() for the GUI, and license() for information. | ---------------------------------------------------------------------- ********************************************************************** * * * Warning: this is a prerelease version, and it may be unstable. * * * ********************************************************************** trace('print factor(10)'); print 3+97 s c sage: trace('print factor(10)'); print 3+97 > <string>(1)<module>() ipdb> s --Call-- > /Users/buildbot/build/sage/bsd-2/bsd_64_full/build/sage-4.6.alpha3/local/lib/python2.6/site-packages/sage/rings/arith.py(2153)factor() 2152 -> 2153 def factor(n, proof=None, int_=False, algorithm='pari', verbose=0, **kwds): 2154 """ ipdb> c 2 * 5 <BLANKLINE>
I think we can fix this by using -...
instead of ---...
in the "Expected" output, but I'm not sure about why
-bit mode Creating SAGE_LOCAL/lib/sage-64.txt since it does not exist
appears in the "Got" output but not with ./sage
, say.
Ticket #9446 is about a different problem in trace.py
.
Change History (18)
comment:1 Changed 11 years ago by
- Summary changed from Doctest failure in trace.py on OS X 10.6 Intel to Doctest failure in trace.py with 64-bit OS X build
comment:2 Changed 11 years ago by
comment:3 Changed 11 years ago by
I tried the suggestion (replacing "---..." with "-..." and it does "fix" the problem: prior to the change, every test of "trace.py" failed as shown above; after the change, all tests pass for this file.
I have no clue why that works, but then I don't know how any of the testing mechanisms work.
comment:4 follow-up: ↓ 6 Changed 11 years ago by
- Cc leif added
I think we seeing this now because we merged
-. $SAGE_ROOT/local/bin/sage-env 1>/dev/null 2>/dev/null +# TODO: Perhaps test for existence of "$SAGE_ROOT"/local/bin/sage-env first... +# (to catch a specific simple error earlier) +# Do NOT redirect stderr and stdout to /dev/null, +# since sage-env might print important error messages: +. "$SAGE_ROOT"/local/bin/sage-env
at #9644.
comment:5 Changed 11 years ago by
are
comment:6 in reply to: ↑ 4 Changed 11 years ago by
Replying to mpatel:
I think we seeing this now because we merged
-. $SAGE_ROOT/local/bin/sage-env 1>/dev/null 2>/dev/null +# TODO: Perhaps test for existence of "$SAGE_ROOT"/local/bin/sage-env first... +# (to catch a specific simple error earlier) +# Do NOT redirect stderr and stdout to /dev/null, +# since sage-env might print important error messages: +. "$SAGE_ROOT"/local/bin/sage-env
at #9644.
Yep. We've noticed the odd message earlier (you also get it when you start anything by running ./sage ...
, i.e. even if you don't want to build anything.)
Simple fix: Redirect all output to stderr
, and don't redirect stderr
to /dev/null
when sourcing sage-check-64
in sage-env
(if at all).
Note that previously one would never have seen that message, even when it was perhaps important.
On the other hand, I see no reason for having it there (sage-env
):
... if [ "$SAGE64" != "yes" -a "$SAGE64" != "no" ]; then echo "The environment variable SAGE64 (=$SAGE64) must be either unset, yes or no." exit 1 fi # In case SAGE64 has been set to yes before re-inject it into the environment # This is only done on OSX and Solaris since those are the only real multi lib # arches we support. Eventually Linux PPC on the PS3 might need to be added here source $SAGE_LOCAL/bin/sage-check-64 2> /dev/null export SAGE64 ...
sage-check-64
:
... if [ "$SAGE64" = "yes" ]; then CHECKFILE="no" if [ `uname` = "SunOS" ]; then echo "Building Sage on Solaris in 64-bit mode" CHECKFILE="yes" fi if [ `uname` = "Darwin" ]; then echo "Building Sage on OS X in 64-bit mode" CHECKFILE="yes" fi if [ $CHECKFILE = "yes" ]; then if ! [ -d "$SAGE_LOCAL"/lib ]; then echo "Creating SAGE_LOCAL/lib since it does not exist" mkdir "$SAGE_LOCAL"/lib fi echo "Creating SAGE_LOCAL/lib/sage-64.txt since it does not exist" touch "$SAGE_LOCAL"/lib/sage-64.txt fi fi # Check if SAGE_LOCAL/lib/sage-64.txt exists. If it does on Solaris and OSX # force the setting of SAGE64 if [ -a "$SAGE_LOCAL"/lib/sage-64.txt ]; then echo "Detected SAGE64 flag" if [ `uname` = "SunOS" ]; then echo "Building Sage on Solaris in 64-bit mode" SAGE64="yes"; export SAGE64 fi if [ `uname` = "Darwin" ]; then echo "Building Sage on OS X in 64-bit mode" SAGE64="yes"; export SAGE64 fi fi
So in principle, sage-check-64
shouldn't be sourced from sage-env
, but only by those scripts actually building something (and/or perhaps sage-sage
if it calls such a script).
comment:7 Changed 11 years ago by
:-) Apparently sage-64.txt
will always be touched whenever SAGE64=yes
(on MacOS X or Solaris); I can't see a test there.
Weird logic... (and not very efficient btw.)
comment:8 Changed 11 years ago by
I don't see the point in having these Solaris and OS X specific tests. Better to make it work for any platform.
The whole bit of code needs looking at, since apart from the messages it prints, it works sub-optimally as noted above.
Dave
comment:9 Changed 11 years ago by
Something like
flagfile=... # whatever its name / location will be test -z "$SAGE64" && export SAGE64=no # default to no if not set case "$SAGE64" in yes|no) if [ -f "$flagfile" ]; then saved64=`cat "$flagfile"` if [ "$saved64" != "$SAGE64" ]; then # inconsistent -> error fi else echo $SAGE64 > "$flagfile" fi ;; *) # invalid value -> error esac # And no messages, only errors to stderr
comment:10 Changed 11 years ago by
Just wondering: Doesn't MacOS X 10.6 default to 64-bit builds? (In which case you wouldn't have to set SAGE64=yes
.)
Doesn't help on other OSs (needing SAGE64
) of course.
This is from MPIR's spkg-install
, if SAGE64
is not "yes"...:
# Do not set ABI=32 on OS X 10.6, since there everything # defaults to 64-bit: ...
comment:11 Changed 11 years ago by
I'm not sure if it helps, but on bsd.math (OS X 10.6) a non-SAGE64=yes
build gives
$ ./sage -c "print is_64_bit" True
A SAGE64=yes
build yields
$ ./sage -c "print is_64_bit" Detected SAGE64 flag Building Sage on OS X in 64-bit mode True
Is there a better way to check whether both builds are essentially the same on this machine?
comment:12 Changed 11 years ago by
I just noticed there's no link here to the ticket John created, (which fixes this problem by changing sage-check-64
, and also changes the SAGE64
mechanism).
comment:13 Changed 9 years ago by
- Component changed from doctest to build
- Owner changed from mvngu to GeorgSWeber
Is this ticket still valid?
comment:14 follow-up: ↓ 15 Changed 9 years ago by
Well, did #10303 fix this? Leif's last comment (from 2 years ago) is a little vague about exactly what was fixed.
comment:15 in reply to: ↑ 14 Changed 9 years ago by
Replying to kcrisman:
Well, did #10303 fix this? Leif's last comment (from 2 years ago) is a little vague about exactly what was fixed there.
I was referring to the useless messages ("Detected SAGE64 flag .... Building Sage on OS X in 64-bit mode") which used to disturb (not just) doctests, since they went to stdout
as well. Although #10303 bit-rottened for a while, this is meanwhile fixed.
So this ticket can IMHO be closed as fixed. (No need to change the doctest, because the failure was solely caused by that message.)
comment:16 Changed 9 years ago by
- Milestone changed from sage-5.9 to sage-duplicate/invalid/wontfix
- Status changed from new to needs_review
comment:17 Changed 9 years ago by
- Reviewers set to David Roe
- Status changed from needs_review to positive_review
comment:18 Changed 9 years ago by
- Resolution set to worksforme
- Status changed from positive_review to closed
I've many times tried building Sage 64-bit on Solaris or OpenSolaris and then set
SAGE64=yes
. I've often seen the message:when the file should have existed. There's no reason that file should ever be (re)created more than once, so something must be unnecessarily creating that file. (Either that, or something is unnecessarily deleting it, so it needs to be recreated, which seems a less likely cause).
The idea of that file is apparently to stop someone building a 64-bit version of Sage, then upgrading it a later date, but this time with SAGE64 unset, which would result in a mix of 32-bit and 64-bit objects. I can see that is sensible - for someone who has builds with SAGE64 set and builds with it unset, it is easy to get in a mess and get a mix of 32-bit ans 64-bit objects.
The opposite problem can also exist too. One can start a 32-bit build, then try to upgrade, with SAGE64=yes set by accident. The file
SAGE_LOCAL/lib/sage-64.txt
does not protect against this. One still gets a mix of 32-bit and 64-bit objects, which causes a mess.I believe an optimal solution would be he file
SAGE_LOCAL/lib/sage-64.txt
should always be created, and it have either the word "yes" or "no" in it, depending on whether SAGE64=yes when the build started or not. Then the Sage environment shouldCurrently it appears the file does not fully protect against mixing 32-bit and 64-bit objects, and for reasons unknown is being created more than once.
I think since bsd.math creates 64-bit objects by default, you will not actually see the mix of 32-bit and 64-bit objects that can happen on Solaris. But for any Solaris system, and any OS X system where the default it to build 32-bit objects, the protection system that this file is supposed to address is not addressed very well.