Opened 11 years ago

Last modified 9 years ago

#10303 closed defect

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

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:
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.v2.patch or trac_10303-scripts-SAGE64.v3.patch, depending on your mood. (The first insists that SAGE64 should not be used except on OS X or Solaris; the second has no such restriction.)
  • copy trac_10303-install to SAGE_ROOT/spkg/install, or apply the corresponding patch for that file.

Change History (20)

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 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 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 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)
Note: See TracTickets for help on using tickets.