Opened 10 years ago

Closed 9 years ago

#11073 closed enhancement (fixed)

remove the spkg/base repo!

Reported by: jhpalmieri Owned by: jdemeyer
Priority: blocker Milestone: sage-5.0
Component: build Keywords: scripts base hg
Cc: jdemeyer, kini, leif, was Merged in: sage-5.0.beta0
Authors: Volker Braun, Jeroen Demeyer Reviewers: John Palmieri, William Stein
Report Upstream: N/A Work issues:
Branch: Commit:
Dependencies: sage-4.8.alpha6 Stopgaps:

Status badges

Description (last modified by jdemeyer)

Given the creation of the Sage root repo (see #9433), the repository in SAGE_ROOT/spkg/base should be combined with it.

1) apply 11073_root_hgignore.patch to the root repo

2)

cd $SAGE_ROOT/spkg/base
rm -rf .hg* stdint.h_Solaris9 sage-*
hg add *-install README.txt
mkdir ../bin
mv *.py sage-* *.sh text-* ../bin
cd ../bin
cp -p $SAGE_ROOT/local/bin/sage-env .
cp -p $SAGE_ROOT/local/bin/sage-spkg .
cp -p $SAGE_ROOT/local/bin/sage-sage sage
hg add . *
hg commit -m "Trac #11073: Move base repository to root repository"

4) apply 11073_root_after.patch and 11073_prereq_install.patch to the root repository.

5)

cd $SAGE_ROOT/local/bin
hg remove sage-env sage-spkg sage-sage
hg commit -m "Trac #11073: Move various scripts to spkg/bin"

5) apply 11073_scripts.patch to the scripts repo

6) apply 11073_sagelib_misc.patch to the main Sage repo.

7) apply 11073_extcode.patch to data/extcode.

The file stdint.h_Solaris9 is not used and therefore deleted. sage-env, sage-spkg and sage-sage (renamed to sage) are moved to spkg/bin, sage-make_relative to scripts (local/bin).

The script sage-sage is renamed to spkg/bin/sage and $SAGE_ROOT is no longer put in the $PATH, such that running "sage" after sage-env will run $SAGE_ROOT/spkg/bin/sage.

Fixes #10970 (do not generate pipestatus), see also #12102 (make bzip2 a standard package) and #12206 (install sage_scripts earlier), #12311 (stop copying testcc.sh and testcxx.sh).

Attachments (9)

trac_11073_stubs_for_base.patch (29.7 KB) - added by vbraun 10 years ago.
Updated patch
trac_11073_stop_copying_stuff.patch (1.6 KB) - added by vbraun 10 years ago.
Updated patch
trac_11073_remove_base_repo.patch (54.9 KB) - added by vbraun 10 years ago.
Updated patch
11073_prereq_install.patch (1.9 KB) - added by jdemeyer 9 years ago.
11073_root_hgignore.patch (651 bytes) - added by jdemeyer 9 years ago.
11073_sagelib_misc.patch (1.3 KB) - added by jdemeyer 9 years ago.
11073_root_after.patch (20.1 KB) - added by jdemeyer 9 years ago.
11073_scripts.patch (8.9 KB) - added by jdemeyer 9 years ago.
11073_extcode.patch (1.4 KB) - added by jdemeyer 9 years ago.

Download all attachments as: .zip

Change History (114)

comment:1 Changed 10 years ago by jdemeyer

I will take care of removing spkg/base/sage-check-64 in sage-4.7.alpha3.

comment:2 Changed 10 years ago by drkirkby

I never realised this was this duplication - it is clearly very stupid. If nothing else, copies could be replaced with links, but it would be better to do away with the repository if possible.

If the repositories can be merged, then I think that's a good idea. I don't like the idea of having a repository as a suppository of another - I think it would just add unnecessary complications. From reading about the subrepositories, I don't feel they are designed to address a need we have.

comment:3 Changed 10 years ago by kini

  • Cc kini added

From what I understand of such things, using subrepositories is a huge mess. The "proper" usage case of a subrepository is basically what the src/ directory inside an spkg is - i.e. a repository that is bolted on but comes from some "upstream" place. When you commit changes to your repository, if you've modified anything in the subrepository, it automatically commits to the subrepository too (or at least tortoisehg does on windows, if I recall correctly from when I was messing with this stuff a year ago or so), allowing you to send changesets upstream with hg push or what-have-you, and when someone pulls from your repository, their mercurial pulls the latest version of its subrepository from the upstream URL rather than from your copy of it, I think. Though we sage developers have a pretty non-standard mercurial workflow - nobody ever pulls from anywhere, really - so I don't know how much this matters. Still, as drkirkby says, it would just needlessly complicate matters.

tl;dr I too support merging the repositories or getting rid of one of them over maintaining one as a subrepository of the other.

comment:4 Changed 10 years ago by jdemeyer

  • Milestone changed from sage-4.7 to sage-4.7.1

comment:5 Changed 10 years ago by jdemeyer

  • Priority changed from minor to blocker

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

Dave: in the base repository, can you explain what the files stdint.h_Solaris9, testcc.sh, and testcxx.sh are for? I know what the last two do, but are they actually used anywhere? I can't find any reference to them in SAGE_ROOT/local/bin or SAGE_ROOT/spkg. Are they called by various spkg-install files?

Right now, testcc.sh and testcxx.sh are copied (by the file spkg/install) to SAGE_ROOT/local/bin. If we're doing away with the base repository, should they be tracked in the Sage root repository or the scripts repository? Since they're scripts which might be useful in general, the scripts repo might make sense. But if they're only used in the installation process -- I don't know where they're used -- maybe the root repo makes sense. Any opinions?

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

Replying to jhpalmieri:

Dave: in the base repository, can you explain what the files stdint.h_Solaris9, testcc.sh, and testcxx.sh are for? I know what the last two do, but are they actually used anywhere? I can't find any reference to them in SAGE_ROOT/local/bin or SAGE_ROOT/spkg. Are they called by various spkg-install files?

I'm not aware of what stdint.h_Solaris9 was supposed to do. I think Micheal added that. It's pretty dumb, as it forces a long to be 4 bytes with:

typedef unsigned long       int_fast32_t;

which is obviously invalid on 64-bit builds. I recall having to create a ticket to stop including that file, as it was screwing up attempts to build Sage 64-bit on Solaris.

Given Solaris 10 came out in 2005, I don't think there's much point in worrying about Solaris 9 anyway.

As such, I think that stdint.h_Solaris9 should be deleted and if there is any issues it creates, we fix them. But I don't think it will create any problems.

The testcc.sh and testcxx.sh scripts are used in a few .spkg files. There's several .spkg files which have tests for the compilers, which would be much simplified if replaced by those scripts.

Right now, testcc.sh and testcxx.sh are copied (by the file spkg/install) to SAGE_ROOT/local/bin. If we're doing away with the base repository, should they be tracked in the Sage root repository or the scripts repository? Since they're scripts which might be useful in general, the scripts repo might make sense. But if they're only used in the installation process -- I don't know where they're used -- maybe the root repo makes sense. Any opinions?

I think the scrips repository will be ok. If nothing else, it is consistent with much of the rest of Sage. I don't think either of those scripts gets used before the sage-scripts package is installed, but I'm not 100% sure on that.

comment:8 in reply to: ↑ 7 ; follow-up: Changed 10 years ago by jdemeyer

Replying to drkirkby:

It's pretty dumb, as it forces a long to be 4 bytes with:

typedef unsigned long       int_fast32_t;

which is obviously invalid on 64-bit builds.

It is invalid, but not for the reason you state.

The problem is that int_fast32_t is supposed to be signed, while unsigned long is unsigned. As for size, the type int_fast32_t is required to be at least 32 bits, not exactly 32 bits, so the following would be correct, both on 32-bit and 64-bit machines:

typedef long            int_fast32_t;

In fact, on my 64-bit Linux laptop, the types int_fast16_t and int_fast32_t are typedefed to be 64-bit longs.

comment:9 follow-up: Changed 10 years ago by jhpalmieri

I tried grepping through all of the spkgs and the rest of Sage. I find no mention of stdint.h_Solaris9, so I think we should delete it. testcxx.sh doesn't get used anywhere, but it could be useful, so we'll keep it. testcc.sh gets used in the spkg-install file for libm4ri, so if we add testcc.sh to the scripts repo, then we need to make sure that the scripts spkg is a prerequisite for libm4ri.

comment:10 follow-up: Changed 10 years ago by jhpalmieri

Regarding the files sage-env, sage-make_relative, and sage-spkg: these are currently in the scripts repo (and it makes a lot of sense for them to be there), but they get used in the Sage build process right from the start. (There have been complaints about the role of sage-make_relative, and those might have some validity to them, but this is not the ticket to deal with that.) Right now, the script sage-sdist copies them from the scripts repo to the base directory so they're in the right place in a new source distribution. One of the first things that spkg/install does is to copy them back to local/bin/ so they're in the right place to be executed.

So: what should be done with these? Move them to root repo and put links to them in local/bin? That seems to make a lot of sense, but it doesn't feel right for some reason. Any other ideas?

comment:11 Changed 10 years ago by kini

Assuming I understand correctly what is going on here, why don't we keep them in local/bin and copy them to spkg/base during the -sdist process without putting the copies under revision control? spkg/install will copy them back to local/bin when building a new sage installation from source, but once we install the scripts repo, they will sync with the copy we keep under revision control in local/bin anyway. If we take hg merge out of local/bin/sage-spkg-install and put in hg update -C instead, which I think is a good idea, then we have even less to worry about.

comment:12 in reply to: ↑ 10 Changed 10 years ago by jdemeyer

Replying to jhpalmieri:

Move them to root repo and put links to them in local/bin? That seems to make a lot of sense, but it doesn't feel right for some reason.

If feels right to me...

comment:13 in reply to: ↑ 8 Changed 10 years ago by drkirkby

Replying to jdemeyer:

Replying to drkirkby:

It's pretty dumb, as it forces a long to be 4 bytes with:

typedef unsigned long       int_fast32_t;

which is obviously invalid on 64-bit builds.

It is invalid, but not for the reason you state.

The problem is that int_fast32_t is supposed to be signed, while unsigned long is unsigned. As for size, the type int_fast32_t is required to be at least 32 bits, not exactly 32 bits, so the following would be correct, both on 32-bit and 64-bit machines:

typedef long            int_fast32_t;

On Solaris the int_fast_32 remains at 32-bits, even on 64-bit code, so what actually caused Sage to fail to compile was certainly the size rather than the sign.

That little bit of header file was included in Sage well after Sage built reliably 32-bit. It was only when I tried a 64-bit build of Sage that this became a problem. (I think a 64-bit Sage on Solaris is looking further away now, as we don't have a good solution for making sure modules are initialized properly before anything is imported from them.)

Test program

drkirkby@laptop:~$ cat test.c
#include <stdio.h>
#include <stdlib.h>
#include <sys/int_types.h>

int main() {
  printf("sizeof(int)=%d\n",sizeof(int));
  printf("sizeof(long)=%d\n",sizeof(long));
  printf("sizeof(int_fast32_t)=%d\n",sizeof(int_fast32_t));
  exit(0);
}

Create a 32-bit executable and execute it

drkirkby@laptop:~$ gcc  test.c
drkirkby@laptop:~$ ./a.out
sizeof(int)=4
sizeof(long)=4
sizeof(int_fast32_t)=4

Create a 64-bit executable and execute it

drkirkby@laptop:~$ gcc  -m64 test.c
drkirkby@laptop:~$ ./a.out
sizeof(int)=4
sizeof(long)=8
sizeof(int_fast32_t)=4

In fact, on my 64-bit Linux laptop, the types int_fast16_t and int_fast32_t are typedefed to be 64-bit longs.

As you can see, that's not so on Solaris, int_fast32_t remains 32-bits, even in 64-bit code.

I recall using a Cray Y-MP with Unicos, where

sizeof(char)=1
sizeof(short)=8
sizeof(int)=8
sizeof(long)=8

The Cray is the only system I've come across where shorts are 64-bit. That tripped me up, as some code I'd written assumed shorts were 2 bytes.

Dave

comment:14 in reply to: ↑ 9 Changed 10 years ago by drkirkby

Replying to jhpalmieri:

testcc.sh gets used in the spkg-install file for libm4ri, so if we add testcc.sh to the scripts repo, then we need to make sure that the scripts spkg is a prerequisite for libm4ri.

Having thought about this more, I think it would be better if they were in the root repository.

There are lots of bits of code in Sage which are testing if the compilers are Sun or GNU. Those bits could be dramatically simplified by using the testcc.sh and testcxx.sh scripts. Look in rubiks for example. There are probably 20 or more spkg-install scripts which are using some test for the Sun compiler in one form or another. I've just never got around to re-writing them to use those scripts, but long-term it would be more sensible to remove a lot of things from numerous spkg-install scripts and use the more portable and robust scripts.

As such, I think it might be better if those scripts were available early on, which would be achieved by having them in the root repository.

comment:15 follow-up: Changed 10 years ago by vbraun

  • Authors set to Volker Braun

Copying files between root and scripts repositories is horrible. We can't make relative symlinks from $SAGE_LOCAL to $SAGE_ROOT since they might be unrelated directories. I think the best solution is to have small shell scripts in the scripts repository that call the script in the sage root repository. It seems that this is only sage-env and the testcc.sh, testcxx.sh scripts.

Patch instructions:

1) remove everything but the tar archives from spkg/base

cd $SAGE_ROOT/spkg/base
rm -rf .hg *-install README.txt sage-* test*sh stdint.h_Solaris9

2) apply trac_11073_remove_base_repo.patch to the root repo

3) apply trac_11073_stop_copying_stuff.patch to the root repo

4) apply trac_11073_stubs_for_base.patch to the scripts repo

Some notes about files formerly in the spkg/base repository:

  • stdint.h_Solaris9: deleted
  • sage-spkg: deleted from sage_scripts, should live in sage_root only.
  • sage-env: replaced by stub in sage_scripts.
  • testcc.sh, testcxx.sh: replaced by stub in sage_scripts
  • sage-make_relative: was a Python script, but base doesn't have Python yet ?!?. Deleted.

comment:16 Changed 10 years ago by vbraun

  • Status changed from new to needs_review

comment:17 Changed 10 years ago by vbraun

  • Description modified (diff)
  • Summary changed from remove the spkg/base repo? to remove the spkg/base repo!

Forgot to delete SAGE_ROOT/spkg/base/.hgignore, instructions moved to ticket description so they can be edited.

Changed 10 years ago by vbraun

Updated patch

Changed 10 years ago by vbraun

Updated patch

Changed 10 years ago by vbraun

Updated patch

comment:18 Changed 10 years ago by vbraun

Updated for Sage-4.7.1.alpha1

comment:19 Changed 10 years ago by vbraun

  • Status changed from needs_review to needs_work

Upgrading doesn't work, the sage_root repository is upgraded near the end of the upgrade process since the spkg depends on mercurial...

comment:20 Changed 10 years ago by jdemeyer

  • Priority changed from blocker to major

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

Replying to vbraun:

  • sage-make_relative: was a Python script, but base doesn't have Python yet ?!?. Deleted.

For the record: This funny [...] script is called from sage-spkg:

...
echo "Making Sage/Python scripts relocatable..."

cd "$SAGE_LOCAL"/bin
./sage-make_relative

echo "Finished installing $PKG_NAME.spkg" 

# It's OK if the above fails -- in fact it will until Python
# itself gets installed. That's fine. 
exit 0   

comment:22 Changed 10 years ago by kini

Any more news here? I'm about to submit a patch to #3052 that adds files to the spkg/base repo :(

comment:23 Changed 10 years ago by leif

  • Cc leif added

comment:24 Changed 10 years ago by jhpalmieri

See #10970 for a somewhat related ticket (at least the discussion there touches on similar issues with the Sage root repo).

comment:25 in reply to: ↑ 21 ; follow-up: Changed 10 years ago by leif

Replying to leif:

Replying to vbraun:

  • sage-make_relative: was a Python script, but base doesn't have Python yet ?!?. Deleted.

For the record: This funny [...] script is called from sage-spkg:

...
echo "Making Sage/Python scripts relocatable..."

cd "$SAGE_LOCAL"/bin
./sage-make_relative

echo "Finished installing $PKG_NAME.spkg" 

# It's OK if the above fails -- in fact it will until Python
# itself gets installed. That's fine. 
exit 0   

I recall having changed this (such that sage-make_relative only gets called if Sage's Python is already installed), but it's not yet in 4.7.2, and I don't recall the ticket.

Anyway, sage-make_relative itself should be present from the beginning, although we could do the same thing (and a bit smarter) directly in sage-spkg, AFAIK the only script from where it's used.

comment:26 in reply to: ↑ 25 Changed 10 years ago by leif

Replying to leif:

I recall having changed this (such that sage-make_relative only gets called if Sage's Python is already installed), but it's not yet in 4.7.2, and I don't recall the ticket.

Ah, #9992, merged into Sage 4.8.alpha0, formerly known as 4.7.3.alpha0.

comment:27 Changed 9 years ago by jdemeyer

  • Description modified (diff)

Simpler solution instead of making stubs in SAGE_ROOT/local/bin:

  1. Create a new directory SAGE_ROOT/spkg/bin
  2. Put sage-env, sage-spkg,... there and not in sage_scripts, nor stubs in sage_scripts
  3. In sage-env, add SAGE_ROOT/spkg/bin to the path
  4. Make sage-sage and sage-spkg find the new sage-env

comment:28 Changed 9 years ago by jdemeyer

  • Description modified (diff)

comment:29 Changed 9 years ago by jdemeyer

  • Description modified (diff)

comment:30 Changed 9 years ago by jdemeyer

  • Description modified (diff)

comment:31 Changed 9 years ago by jdemeyer

  • Description modified (diff)

comment:32 Changed 9 years ago by jdemeyer

  • Description modified (diff)
  • Milestone changed from sage-4.8 to sage-5.0
  • Owner changed from GeorgSWeber to jdemeyer
  • Priority changed from major to blocker

comment:33 Changed 9 years ago by jdemeyer

  • Description modified (diff)

comment:34 Changed 9 years ago by jdemeyer

  • Authors changed from Volker Braun to Volker Braun, Jeroen Demeyer
  • Work issues set to Fix sage -sdist to copy spkg/base/*.tar*

comment:35 Changed 9 years ago by jdemeyer

  • Dependencies set to #12057

comment:36 Changed 9 years ago by jdemeyer

  • Description modified (diff)
  • Work issues changed from Fix sage -sdist to copy spkg/base/*.tar* to Fix upgrading

comment:37 Changed 9 years ago by jdemeyer

  • Description modified (diff)

comment:38 follow-up: Changed 9 years ago by jhpalmieri

In root-spkg-install, line 18, from 11073_root_after.patch, should the last character be deleted?

rm -rf .hg* json_bundle.py stdint.h_Solaris9 sage-* text-* *.sh ) 

comment:39 in reply to: ↑ 38 Changed 9 years ago by jdemeyer

Replying to jhpalmieri:

In root-spkg-install, line 18, from 11073_root_after.patch, should the last character be deleted?

rm -rf .hg* json_bundle.py stdint.h_Solaris9 sage-* text-* *.sh ) 

Yes, thanks.

I believe this ticket is almost ready, it builds and tests fine, sdist and bdist work. The main thing still left to do is to test upgrading.

comment:40 Changed 9 years ago by jdemeyer

Can anybody here please review #12122: it adds a short doctest that sage-make_relative is functional. I created it while working on this ticket because at some point sage-make_relative broke without me realizing it.

comment:41 Changed 9 years ago by jdemeyer

Upgrading to sage-4.7.2 failed because of trying to "hg merge" on top of a hg-untracked file:

****************************************************
nothing changed
pulling from /mnt/usb1/scratch/jdemeyer/sage-4.7.2/spkg/build/sage_root-4.8.alpha3-remove-base
searching for changes
adding changesets
adding manifests
adding file changes
added 17 changesets with 40 changes to 21 files (+1 heads)
(run 'hg heads' to see heads, 'hg merge' to merge)
abort: untracked file in working directory differs from file in requested revision: 'spkg/bin/sage-env'
nothing changed
abort: crosses branches (merge branches or use --check to force update)
'hg update' failed

real    0m0.922s
user    0m0.610s
sys     0m0.260s
sage: An error occurred while installing sage_root-4.8.alpha3-remove-base

comment:42 Changed 9 years ago by jdemeyer

  • Description modified (diff)

comment:43 Changed 9 years ago by jdemeyer

  • Dependencies changed from #12057 to #12057, #12128

comment:44 follow-up: Changed 9 years ago by jdemeyer

Wow, I just realized that the base repo was never actually updated on upgrade. We cannot retro-actively fix the sage-update script of previous Sage versions to fix this. So this is going to remain.

Changed 9 years ago by jdemeyer

comment:45 Changed 9 years ago by jdemeyer

  • Description modified (diff)

comment:46 in reply to: ↑ 44 Changed 9 years ago by jhpalmieri

Replying to jdemeyer:

Wow, I just realized that the base repo was never actually updated on upgrade. We cannot retro-actively fix the sage-update script of previous Sage versions to fix this. So this is going to remain.

I'm not sure what you mean. We can modify the sage-update script: it gets run twice during the upgrade process, so it will be modified the second time through, so we might be able to make relevant changes there. But upgrading seems complicated to me, and I've made mistakes trying to modify the update script before.

comment:47 Changed 9 years ago by jdemeyer

There is no compelling need to upgrade the base packages: an old bzip2 and old prereq (which only does some checks AFAIK) should do just fine.

But yes, we could do it, I didn't realize that sage-upgrade was run twice.

comment:48 Changed 9 years ago by jdemeyer

  • Status changed from needs_work to needs_review

I succesfully:

  1. built from scratch, testlong
  2. create sdist, build from that, testlong
  3. create bdist, testlong
  4. upgrade from sage-4.5, testlong
  5. upgrade from sage-4.7.2, testlong

Everything worked.

comment:49 Changed 9 years ago by jdemeyer

  • Description modified (diff)

comment:50 Changed 9 years ago by jdemeyer

  • Status changed from needs_review to needs_work
  • Work issues changed from Fix upgrading to Rebase to sage-4.8.alpha4

comment:51 Changed 9 years ago by jdemeyer

  • Work issues changed from Rebase to sage-4.8.alpha4 to Rebase to sage-4.8.alpha5

comment:52 Changed 9 years ago by jdemeyer

  • Work issues changed from Rebase to sage-4.8.alpha5 to Rebase to #12016

I see no reason to keep sage-make_relative in the root repo. It would be perfectly fine to keep it only in the scripts repo.

This would break relocating a Sage install during installation, but I don't think that's a problem...

comment:53 Changed 9 years ago by jdemeyer

  • Description modified (diff)

comment:54 Changed 9 years ago by jdemeyer

  • Description modified (diff)

comment:55 Changed 9 years ago by jdemeyer

  • Dependencies changed from #12057, #12128 to sage-4.8.alpha4 + #12016
  • Description modified (diff)
  • Status changed from needs_work to needs_review
  • Work issues Rebase to #12016 deleted

comment:56 Changed 9 years ago by jdemeyer

  • Description modified (diff)

comment:57 Changed 9 years ago by jdemeyer

Because of the discussion at #12206: any objections to moving $SAGE_LOCAL/bin/sage-sage to $SAGE_ROOT/spkg/bin/sage (or some other file in the root repo)?

comment:58 Changed 9 years ago by jdemeyer

  • Description modified (diff)
  • Status changed from needs_review to needs_work
  • Work issues set to move sage-sage

comment:59 Changed 9 years ago by jdemeyer

  • Dependencies changed from sage-4.8.alpha4 + #12016 to sage-4.8.alpha4 + #12016 + #11704

comment:60 Changed 9 years ago by jdemeyer

  • Description modified (diff)

comment:61 Changed 9 years ago by jdemeyer

  • Description modified (diff)

comment:62 Changed 9 years ago by jdemeyer

  • Cc was added
  • Dependencies changed from sage-4.8.alpha4 + #12016 + #11704 to sage-4.8.alpha5 + #11704
  • Status changed from needs_work to needs_review
  • Work issues move sage-sage deleted

Now that sage-sage is moved to spkg/bin/sage, the commands sage -sh and sage -i ... work immediately, directly from the extracted source tarball. So this makes #12206 obsolete.

comment:63 Changed 9 years ago by jdemeyer

  • Status changed from needs_review to needs_work

comment:64 Changed 9 years ago by jdemeyer

  • Dependencies changed from sage-4.8.alpha5 + #11704 to sage-4.8.alpha6

comment:65 Changed 9 years ago by jdemeyer

  • Status changed from needs_work to needs_review

Needs review (again).

Changed 9 years ago by jdemeyer

comment:66 Changed 9 years ago by jdemeyer

  • Description modified (diff)

Add spkg/base/prereq-0.7-install and spkg/base/prereq-0.8-install to .hgignore for upgrades from old Sage versions.

comment:67 Changed 9 years ago by jdemeyer

  • Status changed from needs_review to needs_work

Because of #12311, we should still copy testcc.sh and testcxx.sh to $SAGE_LOCAL/bin. This can then truly be fixed in #12311.

comment:68 Changed 9 years ago by jdemeyer

  • Description modified (diff)

comment:69 Changed 9 years ago by jdemeyer

  • Status changed from needs_work to needs_review

comment:70 Changed 9 years ago by jdemeyer

Upgrading from sage-4.4.4 failed gracefully, as expected.

Doubly-upgrading sage-4.4.4 -> sage-4.8.rc0 -> sage-5.0.prealpha2 worked fine.

comment:71 Changed 9 years ago by jhpalmieri

Needs work: as pointed out by was on sage-devel, the Makefile still refers to local/bin/sage-env in a few places.

comment:72 Changed 9 years ago by jdemeyer

  • Status changed from needs_review to needs_work

I couldn't find that sage-devel post, but you're right. How could I miss that?

comment:73 follow-up: Changed 9 years ago by jdemeyer

Okay, I did a grep of sage-env, sage-sage and sage-spkg in the whole Sage tree, including spkgs. The following are fixed now:

  • Top-level Makefile
  • data/extcode/sage/ext/mac-app/start-sage.sh
  • devel/sage/doc/en/developer/coding_in_python.rst

There is a remaining reference in a comment in the cliquer spkg (can be fixed in #12311) and in comments in the R spkg (should be taken of in #9906).

Changed 9 years ago by jdemeyer

comment:74 Changed 9 years ago by jdemeyer

  • Description modified (diff)

comment:75 Changed 9 years ago by jdemeyer

  • Status changed from needs_work to needs_review

comment:76 in reply to: ↑ 73 Changed 9 years ago by kcrisman

Replying to jdemeyer:

Okay, I did a grep of sage-env, sage-sage and sage-spkg in the whole Sage tree, including spkgs. The following are fixed now:

  • Top-level Makefile
  • data/extcode/sage/ext/mac-app/start-sage.sh

So for positive review, I guess someone would have to build the Mac app and make sure it functions properly.

comment:77 Changed 9 years ago by jdemeyer

I will have the buildbot build a "Mac app" for the unreleased sage-5.0.beta1, which includes the latest version of this ticket.

comment:78 follow-ups: Changed 9 years ago by was

  • Status changed from needs_review to needs_work

COMMENTS:

  1. In this code (and anything similar?):
            17	cd "$SAGE_ROOT/spkg/base" 
     	18	rm -rf .hg* json_bundle.py stdint.h_Solaris9 sage-* text-* *.sh 
    

can we have an error check after the cd, before "rm -rf", since if something went wrong (even a control-c at the wrong moment, or the spkg/base directory is already gone!), then the "rm -rf" *will* cause some serious pain, e.g., deleting the .hg repo from SAGE_ROOT. Any time I see an "rm -rf" in a script I'm very paranoid. This is the sort of situation where Python would be better.

  1. Many people -- e.g., me on the seemingly dozens of machines I have Sage installed on -- will have a "sage" script that is from before this patch. That script will *not* be upgraded when we upgrade our Sage install, since that script is in local/bin/ (say), not in SAGE_ROOT (and it need not be a symlink). That older script explicitly calls "local/bin/sage-sage", so it will now be totally broken, since this ticket deletes sage-sage. For this reason, I think it is absolutely essential that sage-sage not be deleted. The sage-sage script needs to remain, but could either be a 1-liner that calls $SAGE_ROOT/spkg/bin/sage *or* it could produce a nice useful error message explaining that the user must update their sage script by copying over the one from SAGE_ROOT, and edit the path, or create a symbolic link.
  1. As far as I can tell, I'm happy with everything else about the new scripts added 33 hours ago. They fix the issue I reported on sage-devel.

comment:79 in reply to: ↑ 78 Changed 9 years ago by jdemeyer

Replying to was:

COMMENTS:

  1. In this code (and anything similar?):
            17	cd "$SAGE_ROOT/spkg/base" 
     	18	rm -rf .hg* json_bundle.py stdint.h_Solaris9 sage-* text-* *.sh 
    

can we have an error check after the cd

Good point!

comment:80 in reply to: ↑ 78 Changed 9 years ago by jdemeyer

Replying to was:

For this reason, I think it is absolutely essential that sage-sage not be deleted. The sage-sage script needs to remain, but could either be a 1-liner that calls $SAGE_ROOT/spkg/bin/sage *or* it could produce a nice useful error message explaining that the user must update their sage script by copying over the one from SAGE_ROOT, and edit the path, or create a symbolic link.

Also a good point. I would go for a warning with explanation and then

exec "$SAGE_ROOT/spkg/bin/sage" "$@"

comment:81 Changed 9 years ago by jdemeyer

William or somebody else with OS X 10.6: please test the Mac App at http://boxen.math.washington.edu/home/buildbot/binaries/sage/sage-5.0.beta1-OSX-64bit-10.6-x86_64-Darwin-app.dmg The point is really to test whether the "App" part works, not to "make (p)test(long)" or anything.

comment:82 Changed 9 years ago by was

I'm testing the app bundle right now...

comment:83 in reply to: ↑ 78 Changed 9 years ago by jdemeyer

Replying to was:

or the spkg/base directory is already gone!

Just to clarify: we never remove the spkg/base directory. Things like prereq-0.9-install would still live in that directory, but now as part of the root repository.

This is the sort of situation where Python would be better.

Or set -e, see my sage-devel post.

Changed 9 years ago by jdemeyer

comment:84 Changed 9 years ago by jdemeyer

  • Status changed from needs_work to needs_review

William, I implemented the changes you asked. I solved the "cd" followed by "rm -rf" problem by using "cd && rm". I also made this change in an unrelated part of sage-bdist.

Changed 9 years ago by jdemeyer

comment:85 follow-ups: Changed 9 years ago by was

The app bundle *almost* works (on OS X 10.7 even), but does not quite. Basically it goes to this URL:

http://localhost:0/new_worksheet

and

http://localhost:0

This of course does not work at all. Changing the 0 to 8000 works. Everything else seems to work fine though.

I'm happy with your change for "rm -rf". If the port got fixed (to 8000), I think this should get a positive review.

comment:86 Changed 9 years ago by jhpalmieri

I think it looks good, too.

comment:87 in reply to: ↑ 85 Changed 9 years ago by jdemeyer

Replying to was:

The app bundle *almost* works (on OS X 10.7 even), but does not quite. Basically it goes to this URL:

http://localhost:0/new_worksheet

and

http://localhost:0

This of course does not work at all.

Is this a regression due to this ticket? Looks unlikely to me.

comment:88 in reply to: ↑ 85 Changed 9 years ago by jdemeyer

Replying to was:

The app bundle *almost* works (on OS X 10.7 even), but does not quite. Basically it goes to this URL:

http://localhost:0/new_worksheet

and

http://localhost:0

This of course does not work at all.

Does sage -n from the command-line work?

comment:90 Changed 9 years ago by was

Does sage -n from the command-line work?

Yes, perfectly.

comment:91 in reply to: ↑ 89 Changed 9 years ago by was

Replying to jdemeyer:

Does the 4.8.rc0 Mac App work: http://boxen.math.washington.edu/home/buildbot/binaries/sage/sage-4.8.rc0-OSX-64bit-10.6-x86_64-Darwin-app.dmg

Yes, it appears to work perfectly.

comment:92 in reply to: ↑ 85 ; follow-up: Changed 9 years ago by jhpalmieri

Replying to was:

The app bundle *almost* works (on OS X 10.7 even), but does not quite.

I'd like to test this out, too. Can you explain what you mean by the app bundle? Do you mean "sage --bdist ..." with "SAGE_APP_BUNDLE=yes" on a Mac?

comment:93 in reply to: ↑ 92 Changed 9 years ago by kcrisman

I'd like to test this out, too. Can you explain what you mean by the app bundle? Do you mean "sage --bdist ..." with "SAGE_APP_BUNDLE=yes" on a Mac?

Yes.

comment:94 follow-up: Changed 9 years ago by jhpalmieri

  • Status changed from needs_review to needs_work

When I use the app bundle, I see this in the log file:

/Users/palmieri/Desktop/Sage-5.0.pa1.app/Contents/Resources/start-sage.sh: line 41: ./local/bin/sage-env: No such file or directory

I wonder if there should be a place-holder script local/bin/sage-env which runs spkg/bin/sage-env. Otherwise, the file SAGE_ROOT/data/extcode/sage/ext/mac-app/start-sage.sh will have to be changed.

I do not see the wrong port, though: I see 8000, not 0.

comment:95 in reply to: ↑ 94 Changed 9 years ago by jdemeyer

  • Status changed from needs_work to needs_review

Replying to jhpalmieri:

The file SAGE_ROOT/data/extcode/sage/ext/mac-app/start-sage.sh will have to be changed.

I fixed precisely that error in 11073_extcode.patch. I assume you have an older version without that patch?

comment:96 follow-up: Changed 9 years ago by jdemeyer

Mac App testers: please test http://boxen.math.washington.edu/home/buildbot/binaries/sage/sage-5.0.prealpha3-OSX-64bit-10.6-x86_64-Darwin-app.dmg

This includes the latest version of this ticket.

comment:97 Changed 9 years ago by jdemeyer

  • Reviewers set to John Palmieri, William Stein

If it's just the Mac App holding us back, I would prefer to have this ticket set to positive_review. The best way to further test this ticket is to merge it in sage-5.0.beta0. Any small remaining issues can still be fixed in later betas.

comment:98 in reply to: ↑ 96 ; follow-up: Changed 9 years ago by iandrus

Replying to jdemeyer:

Mac App testers: please test http://boxen.math.washington.edu/home/buildbot/binaries/sage/sage-5.0.prealpha3-OSX-64bit-10.6-x86_64-Darwin-app.dmg

This includes the latest version of this ticket.

It worked for me, but only after I realized that I could no longer use an older external sage distribution because of the local/bin/sage-env to spkg/bin/sage-env change. I think we should load spkg/bin/sage-env or if it doesn't exist then source local/bin/sage-env. Of course I'm probably the only one who uses external sage distributions so I won't insist. :-)

comment:99 in reply to: ↑ 98 ; follow-up: Changed 9 years ago by jdemeyer

Replying to iandrus:

It worked for me, but only after I realized that I could no longer use an older external sage distribution

What's an "external sage distribution"?

comment:100 in reply to: ↑ 99 Changed 9 years ago by iandrus

Replying to jdemeyer:

Replying to iandrus:

It worked for me, but only after I realized that I could no longer use an older external sage distribution

What's an "external sage distribution"?

Sorry for being unclear. You can point the app at any sage directory you want (SAGE_ROOT) instead of the one built in. I do this since when I build a new app for development I don't want to copy sage into it. It also allows me to upgrade sage independently of Sage.app.

So I had told it to use an unpatched sage 4.7.2 which won't work.

comment:101 Changed 9 years ago by jdemeyer

iandrus: If that's what you do, I don't find it a problem that the Apps for sage-4.x and sage-5.x are incompatible.

comment:102 Changed 9 years ago by jhpalmieri

I think it's very nice to be able to point the app at any Sage directory. It seems easy to avoid breaking that feature, so if not here, then we should have a follow-up ticket which tries spkg/bin/sage-env and if that doesn't exist, then it uses local/bin/sage-env. Can we just do something like this?

  • sage/ext/mac-app/start-sage.sh

    diff --git a/sage/ext/mac-app/start-sage.sh b/sage/ext/mac-app/start-sage.sh
    a b cd /tmp/sage-mac-app || exit 1 
    3838
    3939# Set (i.e. "source") SAGE_ROOT and all the other environment settings
    4040echo Setting environment variables >> "$SAGE_LOG"
    41 . ./spkg/bin/sage-env >> "$SAGE_LOG" 2>> "$SAGE_LOG"
     41. ./spkg/bin/sage-env >> "$SAGE_LOG" 2>> "$SAGE_LOG" || \
     42. ./local/bin/sage-env >> "$SAGE_LOG" 2>> "$SAGE_LOG"
    4243export SAGE_ROOT
    4344
    4445# Mac OS X app bundles are *intended* to be moved around, and/or given away

Should we fix this here or on a followup?

jdemeyer: you're right, I missed that patch. I read it and thought it made sense, but then forgot about it and didn't apply it.

comment:103 Changed 9 years ago by jdemeyer

OKay, new extcode patch for the Mac App, totally untested (I don't have a Mac).

Changed 9 years ago by jdemeyer

comment:104 Changed 9 years ago by jhpalmieri

This works for me. With an old version of Sage, a message about spkg/bin/sage-env not existing still shows up, but the server starts anyway, as it should.

Any objections to giving this a positive review?

comment:105 Changed 9 years ago by jdemeyer

  • Merged in set to sage-5.0.beta0
  • Resolution set to fixed
  • Status changed from needs_review to closed

Let's give this positive_review and release sage-5.0.beta0.

Note: See TracTickets for help on using tickets.