Opened 10 years ago

Closed 8 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 10 years ago by mpatel

  • 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 10 years ago by drkirkby

I've many times tried building Sage 64-bit on Solaris or OpenSolaris and then set SAGE64=yes. I've often seen the message:

Creating SAGE_LOCAL/lib/sage-64.txt since it does not exist

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 should

  • export SAGE64=yes if the file contains the word "yes"
  • unset SAGE64 if the file contains the word "no".

Currently 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.

comment:3 Changed 10 years ago by justin

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: Changed 10 years ago by mpatel

  • 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 10 years ago by mpatel

are

comment:6 in reply to: ↑ 4 Changed 10 years ago by leif

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 10 years ago by leif

:-) 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 10 years ago by drkirkby

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 10 years ago by leif

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 10 years ago by leif

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 10 years ago by mpatel

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 10 years ago by leif

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 8 years ago by roed

  • Component changed from doctest to build
  • Owner changed from mvngu to GeorgSWeber

Is this ticket still valid?

comment:14 follow-up: Changed 8 years ago by kcrisman

Well, did #10303 fix this? Leif's last comment (from 2 years ago) is a little vague about exactly what was fixed there.

Last edited 8 years ago by kcrisman (previous) (diff)

comment:15 in reply to: ↑ 14 Changed 8 years ago by leif

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 8 years ago by leif

  • Milestone changed from sage-5.9 to sage-duplicate/invalid/wontfix
  • Status changed from new to needs_review

comment:17 Changed 8 years ago by jdemeyer

  • Reviewers set to David Roe
  • Status changed from needs_review to positive_review

comment:18 Changed 8 years ago by jdemeyer

  • Resolution set to worksforme
  • Status changed from positive_review to closed
Note: See TracTickets for help on using tickets.