Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

remove the spkg/base repo! #11073

Closed
jhpalmieri opened this issue Mar 28, 2011 · 117 comments
Closed

remove the spkg/base repo! #11073

jhpalmieri opened this issue Mar 28, 2011 · 117 comments

Comments

@jhpalmieri
Copy link
Member

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

  1. apply attachment: 11073_root_hgignore.patch to the root repo

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"
  1. apply attachment: 11073_root_after.patch and attachment: 11073_prereq_install.patch to the root repository.

cd $SAGE_ROOT/local/bin
hg remove sage-env sage-spkg sage-sage
hg commit -m "Trac #11073: Move various scripts to spkg/bin"
  1. apply attachment: 11073_scripts.patch to the scripts repo

  2. apply attachment: 11073_sagelib_misc.patch to the main Sage repo.

  3. apply attachment: 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).

Dependencies: sage-4.8.alpha6

CC: @jdemeyer @kini @nexttime @williamstein

Component: build

Keywords: scripts base hg

Author: Volker Braun, Jeroen Demeyer

Reviewer: John Palmieri, William Stein

Merged: sage-5.0.beta0

Issue created by migration from https://trac.sagemath.org/ticket/11073

@jdemeyer
Copy link
Contributor

comment:1

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

@sagetrac-drkirkby
Copy link
Mannequin

sagetrac-drkirkby mannequin commented Mar 28, 2011

comment:2

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.

@kini
Copy link
Contributor

kini commented Apr 3, 2011

comment:3

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.

@jhpalmieri
Copy link
Member Author

comment:6

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?

@sagetrac-drkirkby
Copy link
Mannequin

sagetrac-drkirkby mannequin commented Apr 12, 2011

comment:7

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.

@jdemeyer
Copy link
Contributor

comment:8

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

@jhpalmieri
Copy link
Member Author

comment:9

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.

@jhpalmieri
Copy link
Member Author

comment:10

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?

@kini
Copy link
Contributor

kini commented Apr 14, 2011

comment:11

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.

@jdemeyer
Copy link
Contributor

comment:12

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

@sagetrac-drkirkby
Copy link
Mannequin

sagetrac-drkirkby mannequin commented Apr 14, 2011

comment:13

Replying to @jdemeyer:

Replying to @sagetrac-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

@sagetrac-drkirkby
Copy link
Mannequin

sagetrac-drkirkby mannequin commented Apr 14, 2011

comment:14

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.

@vbraun
Copy link
Member

vbraun commented May 30, 2011

comment:15

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
  1. apply trac_11073_remove_base_repo.patch to the root repo

  2. apply trac_11073_stop_copying_stuff.patch to the root repo

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

@vbraun
Copy link
Member

vbraun commented May 30, 2011

Author: Volker Braun

@vbraun

This comment has been minimized.

@vbraun
Copy link
Member

vbraun commented May 30, 2011

comment:17

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

@vbraun vbraun changed the title remove the spkg/base repo? remove the spkg/base repo! May 30, 2011
@vbraun
Copy link
Member

vbraun commented May 30, 2011

Attachment: trac_11073_stubs_for_base.patch.gz

Updated patch

@vbraun
Copy link
Member

vbraun commented May 30, 2011

Updated patch

@vbraun
Copy link
Member

vbraun commented May 30, 2011

Attachment: trac_11073_stop_copying_stuff.patch.gz

Attachment: trac_11073_remove_base_repo.patch.gz

Updated patch

@williamstein
Copy link
Contributor

comment:90

Does sage -n from the command-line work?

Yes, perfectly.

@williamstein
Copy link
Contributor

comment:91

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.

@jhpalmieri
Copy link
Member Author

comment:92

Replying to @williamstein:

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?

@kcrisman
Copy link
Member

comment:93

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.

@jhpalmieri
Copy link
Member Author

comment:94

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.

@jdemeyer
Copy link
Contributor

comment:95

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 attachment: 11073_extcode.patch. I assume you have an older version without that patch?

@jdemeyer
Copy link
Contributor

comment:96

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.

@jdemeyer
Copy link
Contributor

comment:97

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.

@jdemeyer
Copy link
Contributor

Reviewer: John Palmieri, William Stein

@gvol
Copy link
Contributor

gvol commented Jan 20, 2012

comment:98

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

@jdemeyer
Copy link
Contributor

comment:99

Replying to @gvol:

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

@gvol
Copy link
Contributor

gvol commented Jan 20, 2012

comment:100

Replying to @jdemeyer:

Replying to @gvol:

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.

@jdemeyer
Copy link
Contributor

comment:101

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.

@jhpalmieri
Copy link
Member Author

comment:102

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?

diff --git a/sage/ext/mac-app/start-sage.sh b/sage/ext/mac-app/start-sage.sh
--- a/sage/ext/mac-app/start-sage.sh
+++ b/sage/ext/mac-app/start-sage.sh
@@ -38,7 +38,8 @@ cd /tmp/sage-mac-app || exit 1
 
 # Set (i.e. "source") SAGE_ROOT and all the other environment settings
 echo Setting environment variables >> "$SAGE_LOG"
-. ./spkg/bin/sage-env >> "$SAGE_LOG" 2>> "$SAGE_LOG"
+. ./spkg/bin/sage-env >> "$SAGE_LOG" 2>> "$SAGE_LOG" || \
+. ./local/bin/sage-env >> "$SAGE_LOG" 2>> "$SAGE_LOG"
 export SAGE_ROOT
 
 # 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.

@jdemeyer
Copy link
Contributor

comment:103

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

@jhpalmieri
Copy link
Member Author

comment:104

Attachment: 11073_extcode.patch.gz

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?

@jdemeyer
Copy link
Contributor

Merged: sage-5.0.beta0

@jdemeyer
Copy link
Contributor

comment:105

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

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

7 participants