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:  sage5.0 
Component:  build  Keywords:  scripts base hg 
Cc:  jdemeyer, kini, leif, was  Merged in:  sage5.0.beta0 
Authors:  Volker Braun, Jeroen Demeyer  Reviewers:  John Palmieri, William Stein 
Report Upstream:  N/A  Work issues:  
Branch:  Commit:  
Dependencies:  sage4.8.alpha6  Stopgaps: 
Description (last modified by )
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/sageenv . cp p $SAGE_ROOT/local/bin/sagespkg . cp p $SAGE_ROOT/local/bin/sagesage 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 sageenv sagespkg sagesage 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. sageenv
, sagespkg
and sagesage
(renamed to sage
) are moved to spkg/bin
, sagemake_relative
to scripts (local/bin
).
The script sagesage
is renamed to spkg/bin/sage
and $SAGE_ROOT
is no longer put in the $PATH, such that running "sage" after sageenv
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)
Change History (114)
comment:1 Changed 10 years ago by
comment:2 Changed 10 years ago by
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
 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 whathaveyou, 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 nonstandard 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
 Milestone changed from sage4.7 to sage4.7.1
comment:5 Changed 10 years ago by
 Priority changed from minor to blocker
comment:6 followup: ↓ 7 Changed 10 years ago by
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 spkginstall 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 ; followup: ↓ 8 Changed 10 years ago by
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 spkginstall 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 64bit builds. I recall having to create a ticket to stop including that file, as it was screwing up attempts to build Sage 64bit 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 sagescripts package is installed, but I'm not 100% sure on that.
comment:8 in reply to: ↑ 7 ; followup: ↓ 13 Changed 10 years ago by
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 64bit 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 32bit and 64bit machines:
typedef long int_fast32_t;
In fact, on my 64bit Linux laptop, the types int_fast16_t
and int_fast32_t
are typedef
ed to be 64bit long
s.
comment:9 followup: ↓ 14 Changed 10 years ago by
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 spkginstall 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 followup: ↓ 12 Changed 10 years ago by
Regarding the files sageenv, sagemake_relative, and sagespkg: 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 sagemake_relative, and those might have some validity to them, but this is not the ticket to deal with that.) Right now, the script sagesdist 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
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/sagespkginstall 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
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
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 64bit builds.
It is invalid, but not for the reason you state.
The problem is that
int_fast32_t
is supposed to be signed, whileunsigned long
is unsigned. As for size, the typeint_fast32_t
is required to be at least 32 bits, not exactly 32 bits, so the following would be correct, both on 32bit and 64bit machines:typedef long int_fast32_t;
On Solaris the int_fast_32 remains at 32bits, even on 64bit 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 32bit. It was only when I tried a 64bit build of Sage that this became a problem. (I think a 64bit 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 32bit 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 64bit 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 64bit Linux laptop, the types
int_fast16_t
andint_fast32_t
aretypedef
ed to be 64bitlong
s.
As you can see, that's not so on Solaris, int_fast32_t remains 32bits, even in 64bit code.
I recall using a Cray YMP 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 64bit. 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
Replying to jhpalmieri:
testcc.sh gets used in the spkginstall 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 spkginstall scripts which are using some test for the Sun compiler in one form or another. I've just never got around to rewriting them to use those scripts, but longterm it would be more sensible to remove a lot of things from numerous spkginstall 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 followup: ↓ 21 Changed 10 years ago by
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 sageenv
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
: deletedsagespkg
: deleted from sage_scripts, should live in sage_root only.sageenv
: replaced by stub in sage_scripts.testcc.sh
,testcxx.sh
: replaced by stub in sage_scriptssagemake_relative
: was a Python script, but base doesn't have Python yet ?!?. Deleted.
comment:16 Changed 10 years ago by
 Status changed from new to needs_review
comment:17 Changed 10 years ago by
 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.
comment:18 Changed 10 years ago by
Updated for Sage4.7.1.alpha1
comment:19 Changed 10 years ago by
 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
 Priority changed from blocker to major
comment:21 in reply to: ↑ 15 ; followup: ↓ 25 Changed 10 years ago by
Replying to vbraun:
sagemake_relative
: was a Python script, but base doesn't have Python yet ?!?. Deleted.
For the record: This funny [...] script is called from sagespkg
:
... echo "Making Sage/Python scripts relocatable..." cd "$SAGE_LOCAL"/bin ./sagemake_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 9 years ago by
Any more news here? I'm about to submit a patch to #3052 that adds files to the spkg/base repo :(
comment:23 Changed 9 years ago by
 Cc leif added
comment:24 Changed 9 years ago by
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 ; followup: ↓ 26 Changed 9 years ago by
Replying to leif:
Replying to vbraun:
sagemake_relative
: was a Python script, but base doesn't have Python yet ?!?. Deleted.For the record: This funny [...] script is called from
sagespkg
:
... echo "Making Sage/Python scripts relocatable..." cd "$SAGE_LOCAL"/bin ./sagemake_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 sagemake_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, sagemake_relative
itself should be present from the beginning, although we could do the same thing (and a bit smarter) directly in sagespkg
, AFAIK the only script from where it's used.
comment:26 in reply to: ↑ 25 Changed 9 years ago by
comment:27 Changed 9 years ago by
 Description modified (diff)
Simpler solution instead of making stubs in SAGE_ROOT/local/bin
:
 Create a new directory
SAGE_ROOT/spkg/bin
 Put
sageenv
,sagespkg
,... there and not insage_scripts
, nor stubs insage_scripts
 In
sageenv
, addSAGE_ROOT/spkg/bin
to the path  Make
sagesage
andsagespkg
find the newsageenv
comment:28 Changed 9 years ago by
 Description modified (diff)
comment:29 Changed 9 years ago by
 Description modified (diff)
comment:30 Changed 9 years ago by
 Description modified (diff)
comment:31 Changed 9 years ago by
 Description modified (diff)
comment:32 Changed 9 years ago by
 Description modified (diff)
 Milestone changed from sage4.8 to sage5.0
 Owner changed from GeorgSWeber to jdemeyer
 Priority changed from major to blocker
comment:33 Changed 9 years ago by
 Description modified (diff)
comment:34 Changed 9 years ago by
 Work issues set to Fix sage sdist to copy spkg/base/*.tar*
comment:35 Changed 9 years ago by
 Dependencies set to #12057
comment:36 Changed 9 years ago by
 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
 Description modified (diff)
comment:38 followup: ↓ 39 Changed 9 years ago by
In rootspkginstall, 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
Replying to jhpalmieri:
In rootspkginstall, 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
Can anybody here please review #12122: it adds a short doctest that sagemake_relative
is functional. I created it while working on this ticket because at some point sagemake_relative
broke without me realizing it.
comment:41 Changed 9 years ago by
Upgrading to sage4.7.2 failed because of trying to "hg merge" on top of a hguntracked file:
**************************************************** nothing changed pulling from /mnt/usb1/scratch/jdemeyer/sage4.7.2/spkg/build/sage_root4.8.alpha3removebase 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/sageenv' 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_root4.8.alpha3removebase
comment:42 Changed 9 years ago by
 Description modified (diff)
comment:43 Changed 9 years ago by
 Dependencies changed from #12057 to #12057, #12128
comment:44 followup: ↓ 46 Changed 9 years ago by
Wow, I just realized that the base repo was never actually updated on upgrade. We cannot retroactively fix the sageupdate
script of previous Sage versions to fix this. So this is going to remain.
Changed 9 years ago by
comment:45 Changed 9 years ago by
 Description modified (diff)
comment:46 in reply to: ↑ 44 Changed 9 years ago by
Replying to jdemeyer:
Wow, I just realized that the base repo was never actually updated on upgrade. We cannot retroactively fix the
sageupdate
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 sageupdate
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
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 sageupgrade
was run twice.
comment:48 Changed 9 years ago by
 Status changed from needs_work to needs_review
I succesfully:
 built from scratch, testlong
 create sdist, build from that, testlong
 create bdist, testlong
 upgrade from sage4.5, testlong
 upgrade from sage4.7.2, testlong
Everything worked.
comment:49 Changed 9 years ago by
 Description modified (diff)
comment:50 Changed 9 years ago by
 Status changed from needs_review to needs_work
 Work issues changed from Fix upgrading to Rebase to sage4.8.alpha4
comment:51 Changed 9 years ago by
 Work issues changed from Rebase to sage4.8.alpha4 to Rebase to sage4.8.alpha5
comment:52 Changed 9 years ago by
 Work issues changed from Rebase to sage4.8.alpha5 to Rebase to #12016
I see no reason to keep sagemake_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
 Description modified (diff)
comment:54 Changed 9 years ago by
 Description modified (diff)
comment:55 Changed 9 years ago by
 Dependencies changed from #12057, #12128 to sage4.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
 Description modified (diff)
comment:57 Changed 9 years ago by
Because of the discussion at #12206: any objections to moving $SAGE_LOCAL/bin/sagesage
to $SAGE_ROOT/spkg/bin/sage
(or some other file in the root repo)?
comment:58 Changed 9 years ago by
 Description modified (diff)
 Status changed from needs_review to needs_work
 Work issues set to move sagesage
comment:59 Changed 9 years ago by
 Dependencies changed from sage4.8.alpha4 + #12016 to sage4.8.alpha4 + #12016 + #11704
comment:60 Changed 9 years ago by
 Description modified (diff)
comment:61 Changed 9 years ago by
 Description modified (diff)
comment:62 Changed 9 years ago by
 Cc was added
 Dependencies changed from sage4.8.alpha4 + #12016 + #11704 to sage4.8.alpha5 + #11704
 Status changed from needs_work to needs_review
 Work issues move sagesage deleted
Now that sagesage
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
 Status changed from needs_review to needs_work
comment:64 Changed 9 years ago by
 Dependencies changed from sage4.8.alpha5 + #11704 to sage4.8.alpha6
comment:65 Changed 9 years ago by
 Status changed from needs_work to needs_review
Needs review (again).
Changed 9 years ago by
comment:66 Changed 9 years ago by
 Description modified (diff)
Add spkg/base/prereq0.7install
and spkg/base/prereq0.8install
to .hgignore
for upgrades from old Sage versions.
comment:67 Changed 9 years ago by
 Status changed from needs_review to needs_work
comment:68 Changed 9 years ago by
 Description modified (diff)
comment:69 Changed 9 years ago by
 Status changed from needs_work to needs_review
comment:70 Changed 9 years ago by
Upgrading from sage4.4.4 failed gracefully, as expected.
Doublyupgrading sage4.4.4 > sage4.8.rc0 > sage5.0.prealpha2 worked fine.
comment:71 Changed 9 years ago by
Needs work: as pointed out by was on sagedevel, the Makefile still refers to local/bin/sageenv in a few places.
comment:72 Changed 9 years ago by
 Status changed from needs_review to needs_work
I couldn't find that sagedevel post, but you're right. How could I miss that?
comment:73 followup: ↓ 76 Changed 9 years ago by
Okay, I did a grep of sageenv
, sagesage
and sagespkg
in the whole Sage tree, including spkgs. The following are fixed now:
 Toplevel
Makefile
data/extcode/sage/ext/macapp/startsage.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
comment:74 Changed 9 years ago by
 Description modified (diff)
comment:75 Changed 9 years ago by
 Status changed from needs_work to needs_review
comment:76 in reply to: ↑ 73 Changed 9 years ago by
Replying to jdemeyer:
Okay, I did a grep of
sageenv
,sagesage
andsagespkg
in the whole Sage tree, including spkgs. The following are fixed now:
 Toplevel
Makefile
data/extcode/sage/ext/macapp/startsage.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
I will have the buildbot build a "Mac app" for the unreleased sage5.0.beta1, which includes the latest version of this ticket.
comment:78 followups: ↓ 79 ↓ 80 ↓ 83 Changed 9 years ago by
 Status changed from needs_review to needs_work
COMMENTS:
 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 controlc 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.
 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/sagesage", so it will now be totally broken, since this ticket deletes sagesage. For this reason, I think it is absolutely essential that sagesage not be deleted. The sagesage script needs to remain, but could either be a 1liner 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.
 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 sagedevel.
comment:79 in reply to: ↑ 78 Changed 9 years ago by
Replying to was:
COMMENTS:
 In this code (and anything similar?):
17 cd "$SAGE_ROOT/spkg/base" 18 rm rf .hg* json_bundle.py stdint.h_Solaris9 sage* text* *.shcan we have an error check after the cd
Good point!
comment:80 in reply to: ↑ 78 Changed 9 years ago by
Replying to was:
For this reason, I think it is absolutely essential that sagesage not be deleted. The sagesage script needs to remain, but could either be a 1liner 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
William or somebody else with OS X 10.6: please test the Mac App at http://boxen.math.washington.edu/home/buildbot/binaries/sage/sage5.0.beta1OSX64bit10.6x86_64Darwinapp.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
I'm testing the app bundle right now...
comment:83 in reply to: ↑ 78 Changed 9 years ago by
Replying to was:
or the spkg/base directory is already gone!
Just to clarify: we never remove the spkg/base
directory. Things like prereq0.9install
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 sagedevel post.
Changed 9 years ago by
comment:84 Changed 9 years ago by
 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 sagebdist
.
Changed 9 years ago by
comment:85 followups: ↓ 87 ↓ 88 ↓ 92 Changed 9 years ago by
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
I think it looks good, too.
comment:87 in reply to: ↑ 85 Changed 9 years ago by
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_worksheetand
http://localhost:0This 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
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_worksheetand
http://localhost:0This of course does not work at all.
Does sage n
from the commandline work?
comment:89 followup: ↓ 91 Changed 9 years ago by
Does the 4.8.rc0 Mac App work: http://boxen.math.washington.edu/home/buildbot/binaries/sage/sage4.8.rc0OSX64bit10.6x86_64Darwinapp.dmg
comment:90 Changed 9 years ago by
Does sage n from the commandline work?
Yes, perfectly.
comment:91 in reply to: ↑ 89 Changed 9 years ago by
Replying to jdemeyer:
Does the 4.8.rc0 Mac App work: http://boxen.math.washington.edu/home/buildbot/binaries/sage/sage4.8.rc0OSX64bit10.6x86_64Darwinapp.dmg
Yes, it appears to work perfectly.
comment:92 in reply to: ↑ 85 ; followup: ↓ 93 Changed 9 years ago by
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
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 followup: ↓ 95 Changed 9 years ago by
 Status changed from needs_review to needs_work
When I use the app bundle, I see this in the log file:
/Users/palmieri/Desktop/Sage5.0.pa1.app/Contents/Resources/startsage.sh: line 41: ./local/bin/sageenv: No such file or directory
I wonder if there should be a placeholder script local/bin/sageenv which runs spkg/bin/sageenv. Otherwise, the file SAGE_ROOT/data/extcode/sage/ext/macapp/startsage.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
 Status changed from needs_work to needs_review
Replying to jhpalmieri:
The file
SAGE_ROOT/data/extcode/sage/ext/macapp/startsage.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 followup: ↓ 98 Changed 9 years ago by
Mac App testers: please test http://boxen.math.washington.edu/home/buildbot/binaries/sage/sage5.0.prealpha3OSX64bit10.6x86_64Darwinapp.dmg
This includes the latest version of this ticket.
comment:97 Changed 9 years ago by
 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 sage5.0.beta0. Any small remaining issues can still be fixed in later betas.
comment:98 in reply to: ↑ 96 ; followup: ↓ 99 Changed 9 years ago by
Replying to jdemeyer:
Mac App testers: please test http://boxen.math.washington.edu/home/buildbot/binaries/sage/sage5.0.prealpha3OSX64bit10.6x86_64Darwinapp.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/sageenv
to spkg/bin/sageenv
change. I think we should load spkg/bin/sageenv
or if it doesn't exist then source local/bin/sageenv
. Of course I'm probably the only one who uses external sage distributions so I won't insist. :)
comment:99 in reply to: ↑ 98 ; followup: ↓ 100 Changed 9 years ago by
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
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
iandrus: If that's what you do, I don't find it a problem that the Apps for sage4.x and sage5.x are incompatible.
comment:102 Changed 9 years ago by
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 followup ticket which tries spkg/bin/sageenv
and if that doesn't exist, then it uses local/bin/sageenv
. Can we just do something like this?

sage/ext/macapp/startsage.sh
diff git a/sage/ext/macapp/startsage.sh b/sage/ext/macapp/startsage.sh
a b cd /tmp/sagemacapp  exit 1 38 38 39 39 # Set (i.e. "source") SAGE_ROOT and all the other environment settings 40 40 echo Setting environment variables >> "$SAGE_LOG" 41 . ./spkg/bin/sageenv >> "$SAGE_LOG" 2>> "$SAGE_LOG" 41 . ./spkg/bin/sageenv >> "$SAGE_LOG" 2>> "$SAGE_LOG"  \ 42 . ./local/bin/sageenv >> "$SAGE_LOG" 2>> "$SAGE_LOG" 42 43 export SAGE_ROOT 43 44 44 45 # 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
OKay, new extcode patch for the Mac App, totally untested (I don't have a Mac).
Changed 9 years ago by
comment:104 Changed 9 years ago by
This works for me. With an old version of Sage, a message about spkg/bin/sageenv 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
 Merged in set to sage5.0.beta0
 Resolution set to fixed
 Status changed from needs_review to closed
Let's give this positive_review and release sage5.0.beta0.
I will take care of removing
spkg/base/sagecheck64
in sage4.7.alpha3.