Opened 11 years ago

Last modified 9 years ago

#10303 closed defect

clean up sage-check-64 and use of SAGE64 — at Version 31

Reported by: jhpalmieri Owned by: GeorgSWeber
Priority: major Milestone: sage-duplicate/invalid/wontfix
Component: build Keywords: 64
Cc: drkirkby, leif, mpatel Merged in:
Authors: John Palmieri Reviewers: David Kirkby, Leif Leonhardy
Report Upstream: N/A Work issues:
Branch: Commit:
Dependencies: Stopgaps:

Status badges

Description (last modified by jhpalmieri)

There are some problems with SAGE64 and sage-check-64. First, sage-check-64 gets run whenever sage-env gets run, which is way too often. As a consequence, if SAGE64 is set, I get extra messages printed whenever I run anything like "sage -hg":

$ sage -hg status
Detected SAGE64 flag
Building Sage on OS X in 64-bit mode

This is annoying on its own, but it also produces incorrect information when running "sage -pkg": the extra messages make hg think that there are unchecked in changes.

Also, in several files in the scripts repo, comments say that SAGE64 is only for use on OS X or Solaris, but there are no such distinctions in various spkg-install files that I've looked at: if SAGE64 is set, then it tries to build in 64-bit mode, regardless of the platform.

In contrast, the file sage-check-64 only prints messages about building 64-bit if on OS X or Solaris.

Finally, the script sage-check-64 creates a file SAGE_LOCAL/lib/sage-64.txt if SAGE64 is set to "yes", to record the fact that this is a 64-bit build. This makes sense if SAGE64 is unset, but if the user sets SAGE64 to "no", then this file should be ignored, and in fact deleted.

The attached patch gets rid of the distinction between OS X, Solaris, and everything else. It deletes sage-64.txt if SAGE64 is "no". It also only runs sage-check-64 from sage-spkg (which is what calls spkg-install) and sage-build (which may not be strictly necessary, but it was there already). It no longer runs it from sage-env. Since it gets run by sage-spkg, it is now included in spkg/base.


I've asked some questions about this on sage-devel. If the answers are not what I expect, then this may need to be rethought.


Instructions:

  • apply trac_10303-scripts-SAGE64.v3.patch.
  • copy trac_10303-install to SAGE_ROOT/spkg/install, or apply the corresponding patch for that file.

Change History (35)

Changed 11 years ago by jhpalmieri

scripts repo

comment:1 Changed 11 years ago by jhpalmieri

  • Description modified (diff)
  • Status changed from new to needs_review

comment:2 Changed 11 years ago by jhpalmieri

Here is another choice, the "v2" patch. This one writes either "yes" or "no" to the file sage-64.txt. If SAGE64 is unset, then it gets its value from this file. If it is set and the setting is not compatible with the file, then a warning is printed and the user is asked if they want to continue. This requires modifying the file spkg/install, so that it can run this check before attempting to install any spkgs. (Otherwise, it behaves badly with parallel building.) So to use v2, apply that patch and also patch the file spkg/install with the attached patch.

comment:3 follow-up: Changed 11 years ago by leif

Well, not my style... ;-) (which is a bit more compact)

I wonder if the sanity check on SAGE64's setting should be in sage-env. (I assume it's still there, not visible in the patch.)

Nobody should call sage-check-64 directly, but therefore I would

  • test if SAGE64 is either yes, no or empty there (otherwise one could call / source it with e.g. SAGE64=foo),
  • make sure that SAGE_LOCAL is defined,
  • prevent running it directly by substituting
    #!/usr/bin/env bash
    
    by
    #!/this/file/should/be/sourced
    
    (and/or perhaps chmod -x).

I think we could drop "uname-specific" messages (and it's unlikely that SAGE64 will work on 32-bit hosts).

Note that SAGE64=yes is (currently) not supported (or doesn't make sense) on anything but MacOS X and [Open]Solaris; this isn't clear from the echo in sage-env (which is "commented out" IIRC, but people could read it; also, "Creating SAGE_LOCAL/lib/sage-64.txt since it does not exist." might be equally confusing to people building on e.g. 32-bit Linux).


The name of the variable (and the "flag" file) is odd anyway (it should IMHO be SAGE_FORCE_64BIT_BUILD), and there's [currently] no (analogous) way to force a 32-bit build on systems that default to 64-bit...

comment:4 in reply to: ↑ 3 ; follow-up: Changed 11 years ago by jhpalmieri

Replying to leif:

Well, not my style... ;-) (which is a bit more compact)

I wonder if the sanity check on SAGE64's setting should be in sage-env. (I assume it's still there, not visible in the patch.)

It's still there, but it makes sense to move it to sage-check-64.

Nobody should call sage-check-64 directly, but therefore I would

  • test if SAGE64 is either yes, no or empty there (otherwise one could call / source it with e.g. SAGE64=foo),
  • make sure that SAGE_LOCAL is defined,
    • prevent running it directly by substituting
#!/usr/bin/env bash

by

#!/this/file/should/be/sourced

(and/or perhaps chmod -x).

Okay, we can do these.

I think we could drop "uname-specific" messages (and it's unlikely that SAGE64 will work on 32-bit hosts).

Note that SAGE64=yes is (currently) not supported (or doesn't make sense) on anything but MacOS X and [Open]Solaris;

If SAGE64 is set to "yes", then the compiler flag "-m64" is added by certain spkgs with no check on the platform. Is it the case this flag has no effect except on OS X and Solaris?

The name of the variable (and the "flag" file) is odd anyway (it should IMHO be SAGE_FORCE_64BIT_BUILD), and there's [currently] no (analogous) way to force a 32-bit build on systems that default to 64-bit...

I don't think we can change the name of the variable, but the flag file could be renamed, since it's only used in sage-check-64. Any suggestions?

comment:5 in reply to: ↑ 4 Changed 11 years ago by leif

Replying to jhpalmieri:

Replying to leif:

I think we could drop "uname-specific" messages (and it's unlikely that SAGE64 will work on 32-bit hosts).

Note that SAGE64=yes is (currently) not supported (or doesn't make sense) on anything but MacOS X and [Open]Solaris;

If SAGE64 is set to "yes", then the compiler flag "-m64" is added by certain spkgs with no check on the platform. Is it the case this flag has no effect except on OS X and Solaris?

No. I think we should check the platform once, in sage-check-64. While -m64 is superfluous on systems defaulting to 64-bit builds (with gcc), it's illegal on typical 32-bit systems.

SAGE64=yes should simply only be used on 64-bit MacOS X and [Open]Solaris; we could give an error message on 32-bit systems, I think no need to give a warning on 64-bit Linuces.


The name of the variable (and the "flag" file) is odd anyway (it should IMHO be SAGE_FORCE_64BIT_BUILD), and there's [currently] no (analogous) way to force a 32-bit build on systems that default to 64-bit...

I don't think we can change the name of the variable, but the flag file could be renamed, since it's only used in sage-check-64. Any suggestions?

Well, we could use a new variable name but keep "backwards compatibility".

sage-build-arch.txt (or -abi) is perhaps inappropriate for a file containing "yes" or "no"... ;-)

Just omit the actual filename from the message? ("Recording build ABI...") At least if SAGE64=no?

Changing the contents of the file (e.g. "32"/"64" instead of "no"/"yes") is of course another option, but would currently only complicate the tests. (Would make sense if we introduce SAGE_BUILD_ABI and set SAGE64 accordingly, or the other way around if SAGE64 is set instead of that. Then we could rename the script to e.g. sage-check-abi as well...)

Changed 11 years ago by jhpalmieri

the file spkg/install -- for use with the 'v2' patch

Changed 11 years ago by jhpalmieri

diff between old spkg/install and new one

comment:6 follow-up: Changed 11 years ago by jhpalmieri

Here's a new version of the "v2" patch. I haven't changed the variable or the file name, but this does check whether SAGE64 is set to anything except "no" on a non-OS X or Solaris machine, and gives an error in that case. I'm checking the return code for sage-check-64 where it's called.

SAGE64=yes should simply only be used on 64-bit MacOS X and [Open]Solaris; we could give an error message on 32-bit systems, I think no need to give a warning on 64-bit Linuces.

I don't know how to check for 32-bit systems in a shell script, so I'm not doing this, just checking for "Darwin" or "SunOS".

comment:7 in reply to: ↑ 6 Changed 11 years ago by leif

Replying to jhpalmieri:

Here's a new version of the "v2" patch.

Hmmm, now we get "32-bit build, so writing 'no' to SAGE_LOCAL/lib/sage-64.txt." on every non-OS X / non-[Open]Solaris system...

(And if SAGE64 is not set, we usually read the flag file twice.)


SAGE64=yes should simply only be used on 64-bit MacOS X and [Open]Solaris; we could give an error message on 32-bit systems, I think no need to give a warning on 64-bit Linuces.

I don't know how to check for 32-bit systems in a shell script, so I'm not doing this, just checking for "Darwin" or "SunOS".

You could check if ... `uname -m` in i[3456]86|ppc) ..., Dave will know what in addition to look for on [Open]Solaris (e.g. "x86") and especially SPARC.

(On Darwin PPCs, uname -m might report more weird "processor names" like "Power Macintosh", but that's IMHO a minor issue since the build would fail later on 32-bit OSs anyway if we pass -m64. We do not support 64-bit builds on PowerPCs at all, so looking for i[3456]86|ppc*|[Pp]ower* would be ok, too.)


If you edit sage-check-64 again, you could remove the persistent superfluous semicolon from echo "SAGE_LOCAL undefined ... exiting"; ;-) and replace `uname` by "$UNAME".

comment:8 Changed 11 years ago by leif

P.S.:

What about

    ...
    case "$UNAME" in
        Darwin|SunOS)
            # Do the right thing...

            ;;
        *)
            if [ -n "$SAGE64" ]; then
                echo "Warning: Ignoring setting of SAGE64 (=$SAGE64) [since it is only used on MacOS X and Solaris]..."
            fi
            # We don't care if there's a sage-64.txt file
            return 0
    esac

?

Changed 11 years ago by jhpalmieri

scripts repo

comment:9 Changed 11 years ago by jhpalmieri

Okay, this version of the patch still doesn't check for 32-bit. Doing that right may involve more testing than I have time to do right now. (Is there any authoritative listing of the possible outputs for "uname -m" anywhere, with information about each platform, like whether it's 64-bit or not?) This version does check for the platform; if not OS X or Solaris (or OpenSolaris), if SAGE64 is not "no" or unset, there is an error. If SAGE64 is okay, it just exits immediately and silently.

comment:10 follow-up: Changed 11 years ago by leif

Seems you really like negation... ;-) (perhaps because proofs by contradiction are usually easier?)

Looks ok to me, though still a bit complicated.

Giving an (early) error if someone tries to build a 64-bit version on a 32-bit platform is IMHO less important, and there are still ticket numbers available.


Regarding uname, unfortunately every OS implementor is free to let the uname() system call return whatever he/she likes (i.e. almost prose, and no official standards); I now recall Solaris returns e.g. "i86pc", not "i386" or alike, and the uname command just returns what it gets from that call.

For the purpose of Sage, the list of possible values shouldn't be that long though, but I'm not aware of a compilation, and just testing uname -m might not even be sufficient. A good resource are autotools sources (i.e. config.guess and configure scripts); our Skynet machines wiki page also contains entries for uname -a IIRC.

Another, safer way to detect if 64-bit builds are supported is to use getconf, e.g.

    if [ `getconf _POSIX_V6_LP64_OFF64` -eq 1 ]; then
        # 64-bit builds supported
        echo "Max. unsigned long: `getconf -v POSIX_V6_LP64_OFF64 ULONG_MAX`"
        # should give 2^64-1
    fi

comment:11 follow-up: Changed 11 years ago by drkirkby

Like Peter on sage-devel, I think it's bad to restrict this SAGE64 to OS X and Solaris, as these two systems are certainly not the only ones which default to 32-bit, but can build 64-bit.

comment:12 in reply to: ↑ 10 ; follow-up: Changed 11 years ago by drkirkby

Replying to leif:

Another, safer way to detect if 64-bit builds are supported is to use getconf, e.g.

    if [ `getconf _POSIX_V6_LP64_OFF64` -eq 1 ]; then
        # 64-bit builds supported
        echo "Max. unsigned long: `getconf -v POSIX_V6_LP64_OFF64 ULONG_MAX`"
        # should give 2^64-1
    fi

That's not true. Read the section on "Exit Status" on the OpenGroup? web site.

Dave

comment:13 in reply to: ↑ 11 ; follow-up: Changed 11 years ago by jhpalmieri

Replying to drkirkby:

Like Peter on sage-devel, I think it's bad to restrict this SAGE64 to OS X and Solaris, as these two systems are certainly not the only ones which default to 32-bit, but can build 64-bit.

I don't have a strong opinion about this, but I lean slightly toward agreeing with you. At least allowing SAGE64=yes on any system maintains the status quo, more or less.

Do you think we should print a warning saying that it might not be fully supported, or just accept SAGE64=yes silently on platforms other than OS X and Solaris?

comment:14 in reply to: ↑ 13 ; follow-up: Changed 11 years ago by drkirkby

Replying to jhpalmieri:

Replying to drkirkby:

Like Peter on sage-devel, I think it's bad to restrict this SAGE64 to OS X and Solaris, as these two systems are certainly not the only ones which default to 32-bit, but can build 64-bit.

I don't have a strong opinion about this, but I lean slightly toward agreeing with you. At least allowing SAGE64=yes on any system maintains the status quo, more or less.

It's basically what in the installation guide says:

http://www.sagemath.org/doc/installation/source.html#environment-variables says:

Do you think we should print a warning saying that it might not be fully supported, or just accept SAGE64=yes silently on platforms other than OS X and Solaris?

I don't feel the need to print any message it's unsupported.

Anyone using using something other than Linux, Solaris or OS X gets a message that the system in unsupported. They have to set SAGE_PORT to something non-empty. I don't feel the need to hold their hand any more.

comment:15 follow-up: Changed 11 years ago by jhpalmieri

Okay, here's a "v3" patch which allows the use of SAGE64 on any platform. It also requires replacing spkg/install with the attached version (that's the same for v2 and v3). I'm leaving v2 up here in case there is more debate about which approach is better: to allow SAGE64 on any system or not.

comment:16 Changed 11 years ago by jhpalmieri

  • Description modified (diff)

comment:17 in reply to: ↑ 12 Changed 11 years ago by leif

Replying to drkirkby:

Replying to leif:

Another, safer way to detect if 64-bit builds are supported is to use getconf, e.g.

    if [ `getconf _POSIX_V6_LP64_OFF64` -eq 1 ]; then
        # 64-bit builds supported
        echo "Max. unsigned long: `getconf -v POSIX_V6_LP64_OFF64 ULONG_MAX`"
        # should give 2^64-1
    fi

That's not true. Read the section on "Exit Status" on the OpenGroup? web site.

That was just an example. I know one shouldn't test for equality (-eq 1), but check that the result (output) is something else than "-1" and "undefined", but that's unrelated to its exit status.

But using getconf is safe and IMHO the right direction.

comment:18 in reply to: ↑ 14 ; follow-up: Changed 11 years ago by leif

Replying to drkirkby:

Replying to jhpalmieri:

Replying to drkirkby:

Like Peter on sage-devel, I think it's bad to restrict this SAGE64 to OS X and Solaris, as these two systems are certainly not the only ones which default to 32-bit, but can build 64-bit.

I don't have a strong opinion about this, but I lean slightly toward agreeing with you. At least allowing SAGE64=yes on any system maintains the status quo, more or less.

It's basically what in the installation guide says:

... where operating systems other than MacOS X, Solaris and OpenSolaris means Linux, since we currently don't support any other. And it doesn't make any sense to mess with SAGE64 and flag files on systems not defaulting to 64-bit builds, unless or until we also support the opposite, 32-bit builds on systems that default to 64-bit.

As we support more OSs, it's trivial to add them to sage-check-64.


Do you think we should print a warning saying that it might not be fully supported, or just accept SAGE64=yes silently on platforms other than OS X and Solaris?

I don't feel the need to print any message it's unsupported.

Because of its bad name, people erroneously set SAGE64 in the past.

Anyone using using something other than Linux, Solaris or OS X gets a message that the system in unsupported.

Which is pretty correct.

They have to set SAGE_PORT to something non-empty. I don't feel the need to hold their hand any more.


Btw, one can configure GCC to default to 32-bit builds on (almost) any system. (And one could configure GCC to default to 64-bit builds on e.g. Solaris as well.)

comment:19 in reply to: ↑ 18 Changed 11 years ago by leif

Replying to leif:

And it doesn't make any sense to mess with SAGE64 and flag files on systems not defaulting to 64-bit builds, unless or until we also support the opposite, 32-bit builds on systems that default to 64-bit.

Should of course read "on systems defaulting to 64-bit builds" (or "not defaulting to 32-bit builds").


Do you think we should print a warning saying that it might not be fully supported, or just accept SAGE64=yes silently on platforms other than OS X and Solaris?

I don't feel the need to print any message it's unsupported.

Because of its bad name, people erroneously set SAGE64 in the past.

Note that there are (still) spkgs around that do more than just adding e.g. -m64 if SAGE64=yes, i.e. setting it at least currently has other side effects.

So giving an error message on unexpected (and therefore unsupported) situations makes sense to me.

We could suppress such messages if SAGE_PORT is non-empty.

comment:20 in reply to: ↑ 15 ; follow-up: Changed 11 years ago by leif

Replying to jhpalmieri:

Okay, here's a "v3" patch which allows the use of SAGE64 on any platform. It also requires replacing spkg/install with the attached version (that's the same for v2 and v3). I'm leaving v2 up here in case there is more debate about which approach is better: to allow SAGE64 on any system or not.

I don't know if that's intentional, but to me the "v3" is simply a regression.

  • The information on SAGE64 in sage-env is again misleading.
  • If SAGE64 is not set, the message when creating sage-64.txt is simply wrong on systems defaulting to 64-bit builds.

If SAGE64 is not set, we still read the flag file twice if it exists (which is the usual case).

comment:21 in reply to: ↑ 20 Changed 11 years ago by jhpalmieri

Here's a new version of the v3 patch.

comment:22 follow-up: Changed 11 years ago by drkirkby

I'm a bit puzzled. Is the file trac_10303-install, which is said to be for version 2, supposed to work with version 3 too?

I thought I'd create a new standard package

sage-4.6.1.alpha2/spkg/standard/sage_scripts-4.6.1.alpha2.spkg 

with the patches applied before running make for the first time. But that does not work, as then I get a failure as

sage-4.6.1.alpha2/local/bin/sage-check-64

does not exist.

If I'm not mistaken, some of the files in the sage-scripts package also need to be copied to $SAGE_ROOT/spkg/base too, which is rather confusing.They then get copied to $SAGE_ROOT/local/bin which add further to the confusion.

Am I mistaken?

I was hoping to test out on my laptop, which I just stuck Solaris 11 Express on it. I need to install the developers tools first on this, though a sage binary created on 'hawk' works OK.

Perhaps you can clarify matters. By which time I should have ehough tools on here to build Sage on the laptop and test the patch more thoroughly

BTW, I would avoid using source and simply replace it with a . - i.e. just a dot. It is more portable, and will work on shells which don't support source.

Dave

comment:23 in reply to: ↑ 22 Changed 11 years ago by jhpalmieri

Replying to drkirkby:

I'm a bit puzzled. Is the file trac_10303-install, which is said to be for version 2, supposed to work with version 3 too?

Yes. See the instructions in the ticket description.

I thought I'd create a new standard package...

I think it's best to take a working version of Sage, apply the patch to the scripts repo (by cd'ing to SAGE_ROOT/local/bin and applying it), and then run "sage -sdist ...". (I guess it doesn't have to be a "working version", but enough things have to be installed for "sage -sdist ..." to work.) This will copy the appropriate files, which now include sage-check-64, to spkg/base.

BTW, I would avoid using source and simply replace it with a . - i.e. just a dot. It is more portable, and will work on shells which don't support source.

Are there any differences between . and source? That is, if I make this change, does it require other changes (do I still use "return" rather than "exit" in sage-check-64, for instance)?

comment:24 Changed 11 years ago by jhpalmieri

By the way:

$ csh
(...local/bin) [4:51pm]>> . sage-check-64
.: Permission denied.
(...local/bin) [4:51pm]>> source sage-check-64
SAGE_LOCAL: Undefined variable.

So . doesn't work, and source doesn't behave the way I want. In bash:

$ source sage-check-64

SAGE_LOCAL undefined ... exiting
Do not call sage-check-64 directly!

comment:25 Changed 11 years ago by drkirkby

To my knowledge, source and dot are the same for Bourne type shells, which includes /bin/sh and /bin/bash as well as other shells like the Korn shell.

However csh and tcsh are different animals. It not really a good idea to use them for scripts - see

http://www.faqs.org/faqs/unix-faq/shell/csh-whynot/

For interactive use, csh/tcsh have their own syntax which is quite different from sh/bash/ksh etc.

Assuming you have forced your scripts to use bash, then using source is acceptable. But I think it's better to get into a habit of using more portable constructs, avoiding bashisms.

There were (perhaps still are) some error messages generated in Sage when building on Solaris due to the use of source. They might have all gone now though, if a shell now uses bash rather than /bin/sh.

Note using bash for scripts is in general not a great idea, as its size makes it slow. That's why some Linux distributions like Ubunta & Debian link /bin/sh to /bin/dash and use the lighter-weight dash shell. But if you use #!/bin/sh at the top, you need to be more careful about what you write if you want it to be portable.

If you source a file, no matter how you do it, then if it exits you will still be back in script you sourced it from. So what you see is to be expected.

If the script is only called from one place, you might consider just writing it inline, and not sourcing the file at all.

Dave

comment:26 Changed 11 years ago by drkirkby

John, do you want to make any changes? If not, I'm happy to give this a positive review now.

It is a definite improvement over the current system. It works as expected, from a fresh build on my laptop, which runs Solaris 11 Express.

Dave

comment:27 follow-up: Changed 11 years ago by leif

  • Reviewers set to David Kirkby, Leif Leonhardy
  • Status changed from needs_review to positive_review
echo "WARNING: At least some of the following information is outdated."
echo "See the Sage installation guide for more up-to-date information" 
echo "about environment variables related to Sage."

So still needs work? ;-) Funny I never noticed the enviroment[sic]s.

Looks better, though I still don't like messing around with SAGE64 and the flag file at all on systems that don't use / require it. I personally would make a distinction on the OS and, to not annoy some developers, SAGE_PORT (i.e., only take care of SAGE64 on MacOS X 10.4 and 10.5, Solaris and OpenSolaris, or if SAGE_PORT is non-empty. Allowing SAGE64=ignore would be another option, perhaps automatically setting this on other platforms.)

Does the Sage Installation Guide mention how to "cure" an installation which erroneously defaults to SAGE64=no because the user forgot to set it in the first place?

[Want to get rid of this ticket, so positive review. I assume Dave agrees.]

comment:28 Changed 11 years ago by leif

P.S.:

#!/this/file/must/be/sourced

flagfile=... # whatever its name / location is

# Set SAGE64 to "ignore" on everything but MacOS X / [Open]Solaris unless SAGE_PORT is set:
test -z "$SAGE_PORT" && 
    case "$UNAME" in
        Darwin|SunOS) # Add other operating systems later if appropriate.
            # Do nothing here.
            ;;
        *)
            export SAGE64=ignore
    esac

test -z "$SAGE64" && export SAGE64=no # default to no if not set

case "$SAGE64" in
    yes|no)
        # Also make sure SAGE_LOCAL is defined if it is used; here or above.

        if [ -f "$flagfile" ]; then
            saved64=`cat "$flagfile"`
            if [ "$saved64" != "$SAGE64" ]; then
                # inconsistent -> error
            fi
        else
            echo $SAGE64 > "$flagfile"
        fi
       ;;
    ignore) # do nothing
       ;;
    *)
        # invalid value -> error
esac

# And no messages, only errors to stderr

A bit shorter, isn't it?

comment:29 Changed 11 years ago by leif

Perhaps

test -z "$SAGE64" && export SAGE64=no # default to no if not set

should be

test -z "$SAGE64" && export SAGE64=`cat "$flagfile" || echo no`

comment:30 in reply to: ↑ 27 Changed 11 years ago by leif

Replying to leif:

Funny I never noticed the enviroment[sic]s.

Ggle has many hits on it, this one is nice.

comment:31 Changed 11 years ago by jhpalmieri

  • Description modified (diff)
Note: See TracTickets for help on using tickets.