clean up sage-check-64 and use of SAGE64

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

---

Follow-up ticket: #11077.

---

Instructions:

• apply trac_10303-scripts-SAGE64.v5.patch​ to the scripts repo.
• apply trac_10303-root-repo.patch​ to the root repo.
• apply 10303-add-sage-check-64-dot-hgignore.patch​ to the spkg/base repo.

This depends on the patch at #9960.

[jhpalmieri]

scripts repo

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

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

[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?

[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...)

[jhpalmieri]

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

[jhpalmieri]

diff between old spkg/install and new one

[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".

[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".

[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

?

[jhpalmieri]

scripts repo

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

[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

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

[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

[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?

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

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

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

[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.)

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

[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).

[jhpalmieri]

Here's a new version of the v3 patch.

[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

[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)?

[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!

[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

[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

[leif]

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

[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?

[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`

[leif]

Replying to leif:

Funny I never noticed the enviroment[sic]s.

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

[jhpalmieri]

Replying to leif:

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? ;-)

Probably so, but not on this ticket :p Does it work to set $AR or $RANLIB or many of these other variables? We should probably delete all of this and just update the information in the installation guide. (There are also other typos in this passage of the code, but it doesn't seem worth spending more time on.)

Funny I never noticed the enviroment[sic]s.

Me neither, until the last patch.

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

I didn't see any support on sage-devel to change the status quo which allows the use of SAGE64 on any system, so I think for now, doing more documenting of what's going on is the right choice.

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?

I doubt it.

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

...

A bit shorter, isn't it?

Yes, but you left out many of the comments and messages; for example, your "# inconsistent -> error" replaced 15 lines of my version. Shortening it that way doesn't count. ;)

[leif]

Replying to jhpalmieri:

Replying to leif:

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

I didn't see any support on sage-devel to change the status quo which allows the use of SAGE64 on any system, so I think for now, doing more documenting of what's going on is the right choice.

Well, the status quo is (or was) to do nothing on any operating system other than MacOS X or SunOS/Solaris. (I.e., SAGE64 was effectively ignored w.r.t. sage-env / sage-check-64. The assumption in spkg-installs is that a non-empty SAGE64 implies MacOS X or SunOS; we previously had tests like if [ "$SAGE64" = yes -a "$UNAME" = Darwin ], until Dave began to port to Solaris, first adding a test for SunOS, then dropping the distinction on $UNAME, which as a side-effect then slightly changed the meaning or possible interpretations of the spkg-install files regarded in isolation. Of course removing the OS dependency there makes sense, as long as the overall meaning is kept.)

So this patch introduces quite different behavoir on the other platforms rather than just removing stupid messages (and fixing the previous ill and incomplete logic).

Yes, but you left out many of the comments and messages; for example, your "# inconsistent -> error" replaced 15 lines of my version. Shortening it that way doesn't count. ;)

I moved more detailed documentation to the Installation Guide... ;-)

My version also contains lots of (brief) comments and empty lines, nevertheless fits on half a page, so is easier to read and maintain. (Dave also likes saving a few bytes.) :D

The messages are a matter of taste; perhaps we should add more --verbose and --quiet options (and/or corresponding environment variables) to Sage.

[jhpalmieri]

Replying to leif:

Well, the status quo is (or was) to do nothing on any operating system other than MacOS X or SunOS/Solaris. (I.e., SAGE64 was effectively ignored w.r.t. sage-env / sage-check-64. The assumption in spkg-installs is that a non-empty SAGE64 implies MacOS X or SunOS; we previously had tests like if [ "$SAGE64" = yes -a "$UNAME" = Darwin ], until Dave began to port to Solaris, first adding a test for SunOS, then dropping the distinction on $UNAME, which as a side-effect then slightly changed the meaning or possible interpretations of the spkg-install files regarded in isolation. Of course removing the OS dependency there makes sense, as long as the overall meaning is kept.)

So this patch introduces quite different behavoir on the other platforms rather than just removing stupid messages (and fixing the previous ill and incomplete logic).

"Quite different" is not accurate. The only functional difference was that previously on non OS X or Solaris systems, if SAGE64=yes, then the file sage-64.txt would not be created or read. Now it is. Some extra messages are also printed. (Yes, previously sage-check-64 ignored SAGE64 on Linux boxes, but many spkg-install files do not.)

[drkirkby]

I'm happy with this The variable name SAGE64 implies absolutely nothing about the operating system, so I don't see any reason it should behave any differently on different operating systems. As John said, nobody on sage-devel objected to this either.

No two people are likely to want to code a script in exactly the same way, or use the exact same messages. One could spend ages worrying about such small things, and not have any significant impact on Sage.

For me personally, I try to make my scripts as portable as possible, so they don't rely on bash. Then I can bit of the scripts on other software projects, were people don't rely on bash.

Dave

[leif]

Replying to jhpalmieri:

Replying to leif:

Well, the status quo is (or was) to do nothing on any operating system other than MacOS X or SunOS/Solaris. (I.e., SAGE64 was effectively ignored w.r.t. sage-env / sage-check-64. The assumption in spkg-installs is that a non-empty SAGE64 implies MacOS X or SunOS; we previously had tests like if [ "$SAGE64" = yes -a "$UNAME" = Darwin ], until Dave began to port to Solaris, first adding a test for SunOS, then dropping the distinction on $UNAME, which as a side-effect then slightly changed the meaning or possible interpretations of the spkg-install files regarded in isolation. Of course removing the OS dependency there makes sense, as long as the overall meaning is kept.)

So this patch introduces quite different behavoir on the other platforms rather than just removing stupid messages (and fixing the previous ill and incomplete logic).

"Quite different" is not accurate. The only functional difference was that previously on non OS X or Solaris systems, if SAGE64=yes, then the file sage-64.txt would not be created or read. Now it is.

And exactly that is the "problem", besides interpreting SAGE64 not set as "use previous setting", which wasn't recorded on e.g. Linux.

If a user now erroneously sets SAGE64=yes once, potential problems persist, where previously doing unset SAGE64 (or simply leaving / changing the shell) was sufficient. This has to be documented.

(Though it's [at least meanwhile] simply wrong what some spkgs still do when SAGE64 is set [to "yes"], namely e.g. also modifying flags unrelated to building 32- or 64-bit. We've already removed lots of wrong Building a 32-bit version of ... messages.)

IMHO setting SAGE64 (to "yes" or anything else then "no", and perhaps in the future "ignore") on e.g. Linux should raise an error, nothing else, since the behavior is - at least currently - more or less "undefined" in that case.

Some extra messages are also printed.

I won't mind that.

(Yes, previously sage-check-64 ignored SAGE64 on Linux boxes, but many spkg-install files do not.)

Because SAGE64 is not expected to be set on platforms not defaulting to 32-bit builds; this should be catched in a central place, where also new OSs would and easily could be added in the first place.

[leif]

Replying to drkirkby:

The variable name SAGE64 implies absolutely nothing about the operating system,

Hence it should be named SAGE_FORCE_64_BIT_BUILD.

Note that it used to, and you're right in the sense that its documentation has now (actually recently) changed, but that doesn't really reflect the current situation in Sage. (It will hopefully in the future, but currently it's a "should" rather than "is", for developers rather than users.)

so I don't see any reason it should behave any differently on different operating systems.

Its only (intended) use is to force 64-bit builds (on operating systems that default to 32-bit, and therefore doesn't make sense on others, while it should in principle do no harm there either, or more precisely it shouldn't have any effect there).

As John said, nobody on sage-devel objected to this either.

I haven't followed that thread, but in general that doesn't mean much to me. How many people did read the thread? Two people besides you two posted; William just noted that SAGE64 was introduced solely for MacOS X, with no impact on anything else. (Of course that wasn't a very good decision, since lots of code had to be changed for Solaris in essentially the same manner again.)

May I cite Peter's statement?

Generically adding '-m64' when SAGE64 is set is definitely wrong because not all compilers will support that flag. In addition, gcc treats x86 and x86_64 as different variants of the one architecture - so gcc on a 32-bit platform can compile x86_64 code. On 32-bit x86 Linux and *BSD, using '-m64' will cause gcc to build x86_64 code - which the kernel won't be able to execute - so this is highly undesirable.

Ideally, all skpg files should inherit {C,CPP,CXX,FC,LD}FLAGS from the environment (adding spkg-specific options if required). This would allow SAGE64 to be processed in one spot fairly early on in the build -- adding '-m64' or equivalent to {C,FCC,FC,LD}FLAGS, which is then inherited by all succeeding spkg builds. This would remove a lot of special-casing from the build.

If all spkgs really contained just what John posted,

f [ -z $CFLAG64 ] ; then
   CFLAG64=-m64
fi

if [ "x$SAGE64" = xyes ]; then
   echo "64 bit build of cddlib"
   CFLAGS="$CFLAGS $CFLAG64"; export CFLAGS
fi

(which definitely isn't the case, so also the thread didn't really address the issue), the situation would be different, and as Peter noted, we could easily move that code snippet into e.g. sage-env, as well as others by the way.

Other aspects we discussed here weren't discussed there at all.

[jdemeyer]

With a build from scratch, I get

spkg/pipestatus "cd spkg && ./install all 2>&1" "tee -a ../install.log"
./install: line 77: /scratch/jdemeyer/merger/sage-4.6.1.alpha3/local/bin/sage-check-64: No such file or directory
make: *** [build] Error 1

[drkirkby]

Replying to jdemeyer:

With a build from scratch, I get

spkg/pipestatus "cd spkg && ./install all 2>&1" "tee -a ../install.log"
./install: line 77: /scratch/jdemeyer/merger/sage-4.6.1.alpha3/local/bin/sage-check-64: No such file or directory
make: *** [build] Error 1

It needs at least some of the files must be copied to $SAGE_ROOT/spkg/base too. Then they get to $SAGE_LOCAL/bin.

I can't understand why some files are in two repositories, but that seems to be the case. Some things in $SAGE_ROOT/spkg/base are also in the sage-scripts package.

[drkirkby]

At least the following are both in sage_scripts-4.6.1.alpha2.spkg and in $SAGE_ROOT/spkg/base

sage-env
sage-make_relative
sage-spkg

I think sage-check-64 needs to be put into $SAGE_ROOT/spkg/base and any of the above files which get modified by Johns patch.

[drkirkby]

To the release manager

I wish I understood the logic of this, but it appears that

sage-env
sage-make_relative
sage-spkg

in $SAGE_ROOT/spkg/base are in .hgignore. I'm adding a patch which adds sage-check-64 to $SAGE_ROOT/spkg/base/.hgignore. After that is added, the files

sage-env
sage-make_relative
sage-spkg
sage-check-64

should all be copied to $SAGE_ROOT/spkg/base

At least that is my understanding of what's needed.

[drkirkby]

Adds sage-check-64 to .hgignore

[jhpalmieri]

Positive review for Dave's patch.

Replying to jdemeyer:

With a build from scratch, I get

spkg/pipestatus "cd spkg && ./install all 2>&1" "tee -a ../install.log"
./install: line 77: /scratch/jdemeyer/merger/sage-4.6.1.alpha3/local/bin/sage-check-64: No such file or directory
make: *** [build] Error 1

How did you apply the patch? After you apply it to the scripts repo, you have to run "sage -sdist" to make sure the appropriate files are copied to SAGE_ROOT/spkg/base.

[drkirkby]

Is there any good reason for the placing of these files in the scripts repository, copying them to $SAGE_ROOT/spkg/base and then having them added to $SAGE_ROOT/spkg/base/.hgignore ? Perhaps I'm missing something, but it would seem more logical to me to:

• Remove the files from the scripts repository
• Put then in the $SAGE_ROOT/spkg/base and remove them from the $SAGE_ROOT/spkg/base/.hgignore

Perhaps on another ticket, we address this issue, if there is a more logical way of doing this, as to me at least, this is pretty illogical.

Dave

[jhpalmieri]

Replying to drkirkby:

Is there any good reason for the placing of these files in the scripts repository, copying them to $SAGE_ROOT/spkg/base and then having them added to $SAGE_ROOT/spkg/base/.hgignore ?

I don't know. These are scripts that get used both in the installation process and in day-to-day use of Sage (at least when doing things like installing optional skpgs). If you add them to base, then you need to remove them from scripts. One advantage to having them in the scripts repo is that it is an actively updated, well-documented, repo, whereas base is not. Is base even mentioned in the developer's guide?

[jdemeyer]

Dave: I agree 100% that having files in spkg/base and in the scripts repo is a mess. This has caused problems for me countless times. I will now also handle sage-check-64 specially, so it should work.

[jdemeyer]

The commit message of trac_10303-scripts-SAGE64.v3.patch​ should not be one long line. Provide a short description on the first line (this will appear in hg log). Any further info should go on following lines.

[leif]

Replying to jdemeyer:

The commit message of trac_10303-scripts-SAGE64.v3.patch​ should not be one long line. Provide a short description on the first line (this will appear in hg log). Any further info should go on following lines.

I was aware of that, but didn't want to appear to picky... ;-)

[jhpalmieri]

Seriously? You're holding up the patch because of this? The release manager can easily fix an issue like this in about 2 minutes. Instead, ten hours later, I'm going to post a new patch.

[jhpalmieri]

scripts repo

[jdemeyer]

Replying to jhpalmieri:

You're holding up the patch because of this?

No, I'm not.

Of course, the release manager can do it, but I think it is better that I point out these minor issues to the aurhors and that they fix them (does it matter if it takes 10 hours?). Imagine you have 10 tickets with various of these minor issues.

[jdemeyer]

Fixed commit message (first line was blank)

[jhpalmieri]

Replying to jdemeyer:

Replying to jhpalmieri:

You're holding up the patch because of this?

No, I'm not.

Changing it "needs work" instead of merging it seems like holding it up...

Of course, the release manager can do it, but I think it is better that I point out these minor issues to the aurhors

Sure.

and that they fix them

Why? If it's trivial to fix, why does it matter who does it? If you insist that the authors do it every time, you're adding needless bureaucracy. If anyone notices a problem that's trivial to fix, they should just do it. Sage is not run by the French government: it is not supposed to be governed by rigid bureaucratic rules, but instead by common sense whenever possible. Leif, the same goes for you: if you see a problem like missing line breaks in the commit message, and if it seems clear that other work on the ticket has stopped, why not just fix it? That seems more productive than waiting until someone else (like the release manager) notices it and then commenting "I was aware of that."

(does it matter if it takes 10 hours?).

If it's the 10 hours during which a version is released, yes. But 10 hours is just an example. What if I were on vacation or at a conference with little or no internet access, or swamped with grading or research or other issues, and couldn't get to this for a week or two? This particular ticket is not very important, so if it had missed being included in the upcoming version of Sage, no big deal, but why incur a possible long delay and require the author to fix it?

Imagine you have 10 tickets with various of these minor issues.

I don't have to imagine it, I've been release manager and fixed a number of issues like this (for example, commit messages without the ticket number). I may have then posted to the ticket what the problem was, but I took care of fixing it myself.

[leif]

Replying to jhpalmieri:

Leif, the same goes for you: if you see a problem like missing line breaks in the commit message, and if it seems clear that other work on the ticket has stopped, why not just fix it? That seems more productive than waiting until someone else (like the release manager) notices it and then commenting "I was aware of that."

As I said, I considered it a minor issue (and was pretty sure you were also aware of it), so did not even mention it.

Perhaps I should have, but it's IMHO relatively useless to upload another patch just fixing the commit message, since that usually causes more confusion. (I actually did so on other tickets, because I knew the author was busy / unavailable.)

If we had different access rights on trac (e.g. reviewers as "invited" co-authors with the ability to change attachments of others), that would be different.

[jdemeyer]

Replying to jhpalmieri:

Changing it "needs work" instead of merging it seems like holding it up...

My interpretation is: changing it to "needs_work" is a way of signaling the author, that's all...

[leif]

Apparently this doesn't work with upgrades, if the flag file already exists.

I think spkg/install should check if one with the previous semantics (present, but usually empty => SAGE64=yes) exists and then write the new one, containing "yes" or "no".

sage-env and sage-check-64 have to be the new versions immediately after this as well.

[leif]

Replying to leif:

Apparently this doesn't work with upgrades, if the flag file already exists.

I think spkg/install should check if one with the previous semantics (present, but usually empty => SAGE64=yes) exists and then write the new one, containing "yes" or "no".

sage-env and sage-check-64 have to be the new versions immediately after this as well.

See for example ​http://build.sagemath.org/sage/builders/OSX%2010.6-64%20up%20%28bsd%29/builds/8/steps/shell_2/logs/stdio .

[jdemeyer]

Trouble with sage -ba, see #10436.

[jhpalmieri]

I don't think I have any time to work on this right now. If no one else has time either, then the best solution might be to back it out; it was not very high priority to begin with.

[jdemeyer]

Replying to jhpalmieri:

I don't think I have any time to work on this right now. If no one else has time either, then the best solution might be to back it out;

Allright, current plan is to unmerge this in sage-4.6.1.rc0.

[leif]

New patches here should perhaps be based on #9960 (which currently needs review, but hopefully gets merged soon before it further rottens - just rebased it to Sage 4.6.1.rc0.)

(#9960 mainly patches sage-spkg, but also sage-sage and sage-env.)

[patioheater12]

Replying to 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.

• apply 10303-add-sage-check-64-dot-hgignore.patch to the base repo (SAGE_ROOT/spkg/base).

To produce a version with which to test from scratch, you then have to run "sage -sdist ..." to make sure that the necessary files from the scripts repo are copied to spkg/base.

[nthiery]

What's the status for this issue?

It breaks sage -combinat install on the OS X machine of a colleague, precisely due to the SAGE64 verbosity effect in sage -hg.

Thanks!

Nicolas

[jhpalmieri]

The status is, the patches here don't work with upgrades. There was also some debate about the approach in general. If someone else has the time and energy to work on it, please go ahead; it might be best to start over.

[jhpalmieri]

For future reference: if the proposed fix works like the old one -- copying sage-check-64 to spkg/base -- then you must add sage-check-64 to the .hgignore file for the base repo. See also #11073 for other issues dealing with the base repo.

[jhpalmieri]

I'm considering reviving this ticket. As it stands, this ticket deals with two separate issues:

• where sage-check-64 is called, where the variable $SAGE64 is set (in sage-env or sage-check-64), etc.
• what sage-check-64 actually does: which platforms are relevant, does it write anything to the file SAGE_ROOT/local/lib/sage-64.txt, etc.

The second of these is more controversial, and also more complicated, so I think it should go on a separate ticket. The first of these can be solved, I think, by using a slight modification of the "v4" patch. Basically, we should modify everything touched by that patch except for sage-check-64. I'll test this and see how things go; if it looks good, I'll post a patch and open up a second ticket for rewriting sage-check-64.

[drkirkby]

Replying to jhpalmieri:

I'm considering reviving this ticket.

Brave of you John!

I've just spent 15 minutes reading all the comments. This is another of the many tickets that gets out of hand, with a huge number of requested changes.

As it stands, this ticket deals with two separate issues:

They should be on separate tickets.

It might be simpler to ask the release manager to close this ticket as "duplicate", and open two new tickets which deal with two issues in isolation. That will save there being a lot of irrelevant comments on this ticket. This has become too large for comfort.

Dave

[jhpalmieri]

Here are some patches. I believe these should also fix the problem at #10436. (I think you can see that problem if you apply the scripts patch, but then revert just the file sage-build. Then build the polybori spkg without SAGE64 being set.)

[jhpalmieri]

scripts repo

[jhpalmieri]

root repo

[jhpalmieri]

By the way, I took Sage 4.7.alpha2, applied #9960 and the patches here, and created a ​tar file and an upgrade path:

• ​http://sage.math.washington.edu/home/palmieri/misc/10303/sage-4.7.10303/.

[drkirkby]

I don't know enough about the build process of Sage, i.e. the role of all these scripts, to comment much more on this ticket. It is also dependent on #9960 which is listed as needing work.

I'm happy to apply the patches and try some builds with and without setting SAGE64. But I'm not going to be the one to give this a positive review, as I don't understand it enough.

I'll also take a look at #11077 and put some comments there.

Dave

[jdemeyer]

Changing milestone to 4.7.1 since it depends on #9960.

[jdemeyer]

I think none of the issues mentioned are still relevant (some were fixed at #12789). I'm going to close, but feel to protest and disagree.