search

sagemath / sage

Main repository of SageMath
https://www.sagemath.org
Other
1.4k stars 473 forks source link

Move SAGE_DATA to SAGE_LOCAL/share #13123

Closed ohanar closed 12 years ago

ohanar commented 12 years ago

This is part of the new layout for git. Since it is not too difficult to separate off this change, I decided making this a distinct ticket was a good idea. SAGE_EXTCODE will be moved to SAGE_ROOT/devel/ext, and currently SAGE_LOCAL/share/sage/ext links to this.

New SPKGS:

Installation Steps:

Depends on #13457

CC: @kini @jdemeyer @robertwb @burcin

Component: relocation

Author: R. Andrew Ohana, Jeroen Demeyer

Reviewer: François Bissey, Jeroen Demeyer

Merged: sage-5.4.beta2

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

ohanar commented 12 years ago

Dependencies: #12205

ohanar commented 12 years ago

Author: R. Andrew Ohana

ohanar commented 12 years ago

Description changed:

--- 
+++ 
@@ -1 +1,6 @@
-This is in preparation for the transition to git. `SAGE_EXTCODE` will be moved to `SAGE_ROOT/devel/ext`
+This is in preparation for the transition to git. `SAGE_EXTCODE` will be moved to `SAGE_ROOT/devel/ext`.
+
+New SPKGS:
+[http://wstein.org/home/ohanar/spkgs/elliptic_curves-0.7.spkg](http://wstein.org/home/ohanar/spkgs/elliptic_curves-0.7.spkg)
+[http://wstein.org/home/ohanar/spkgs/polytopes_db-20100210.p2.spkg](http://wstein.org/home/ohanar/spkgs/polytopes_db-20100210.p2.spkg)
+[http://wstein.org/home/ohanar/spkgs/graphs-20120404.p4.spkg](http://wstein.org/home/ohanar/spkgs/graphs-20120404.p4.spkg)
ohanar commented 12 years ago

Description changed:

--- 
+++ 
@@ -1,6 +1,9 @@
 This is in preparation for the transition to git. `SAGE_EXTCODE` will be moved to `SAGE_ROOT/devel/ext`.

 New SPKGS:
+
 [http://wstein.org/home/ohanar/spkgs/elliptic_curves-0.7.spkg](http://wstein.org/home/ohanar/spkgs/elliptic_curves-0.7.spkg)
+
 [http://wstein.org/home/ohanar/spkgs/polytopes_db-20100210.p2.spkg](http://wstein.org/home/ohanar/spkgs/polytopes_db-20100210.p2.spkg)
+
 [http://wstein.org/home/ohanar/spkgs/graphs-20120404.p4.spkg](http://wstein.org/home/ohanar/spkgs/graphs-20120404.p4.spkg)
kini commented 12 years ago

Description changed:

--- 
+++ 
@@ -1,9 +1,6 @@
 This is in preparation for the transition to git. `SAGE_EXTCODE` will be moved to `SAGE_ROOT/devel/ext`.

 New SPKGS:
-
-[http://wstein.org/home/ohanar/spkgs/elliptic_curves-0.7.spkg](http://wstein.org/home/ohanar/spkgs/elliptic_curves-0.7.spkg)
-
-[http://wstein.org/home/ohanar/spkgs/polytopes_db-20100210.p2.spkg](http://wstein.org/home/ohanar/spkgs/polytopes_db-20100210.p2.spkg)
-
-[http://wstein.org/home/ohanar/spkgs/graphs-20120404.p4.spkg](http://wstein.org/home/ohanar/spkgs/graphs-20120404.p4.spkg)
+* [http://wstein.org/home/ohanar/spkgs/elliptic_curves-0.7.spkg](http://wstein.org/home/ohanar/spkgs/elliptic_curves-0.7.spkg)
+* [http://wstein.org/home/ohanar/spkgs/polytopes_db-20100210.p2.spkg](http://wstein.org/home/ohanar/spkgs/polytopes_db-20100210.p2.spkg)
+* [http://wstein.org/home/ohanar/spkgs/graphs-20120404.p4.spkg](http://wstein.org/home/ohanar/spkgs/graphs-20120404.p4.spkg)
ohanar commented 12 years ago

Changed dependencies from #12205 to none

ohanar commented 12 years ago

Description changed:

--- 
+++ 
@@ -4,3 +4,4 @@
 * [http://wstein.org/home/ohanar/spkgs/elliptic_curves-0.7.spkg](http://wstein.org/home/ohanar/spkgs/elliptic_curves-0.7.spkg)
 * [http://wstein.org/home/ohanar/spkgs/polytopes_db-20100210.p2.spkg](http://wstein.org/home/ohanar/spkgs/polytopes_db-20100210.p2.spkg)
 * [http://wstein.org/home/ohanar/spkgs/graphs-20120404.p4.spkg](http://wstein.org/home/ohanar/spkgs/graphs-20120404.p4.spkg)
+* [http://wstein.org/home/ohanar/spkgs/conway_polynomials-0.3.spkg](http://wstein.org/home/ohanar/spkgs/conway_polynomials-0.3.spkg)
ohanar commented 12 years ago
comment:5

Ok, everything should work now.

ohanar commented 12 years ago

Description changed:

--- 
+++ 
@@ -1,4 +1,4 @@
-This is in preparation for the transition to git. `SAGE_EXTCODE` will be moved to `SAGE_ROOT/devel/ext`.
+This is part of the new layout for git. Since it is not too difficult to separate off this change, I decided making this a distinct ticket was a good idea. `SAGE_EXTCODE` will be moved to `SAGE_ROOT/devel/ext`, and currently `SAGE_LOCAL/share/sage/ext` links to this.

 New SPKGS:
 * [http://wstein.org/home/ohanar/spkgs/elliptic_curves-0.7.spkg](http://wstein.org/home/ohanar/spkgs/elliptic_curves-0.7.spkg)
kiwifb commented 12 years ago
comment:7

That will cause me a lot of work in sage-on-gentoo but overall that will make my life easier in the long term. I cannot comment on trac13123_extcode.patch and trac13123_scripts.patch as I don't know the code in there all that well. But for the rest:

kiwifb commented 12 years ago

Reviewer: François Bissey

jdemeyer commented 12 years ago
comment:8

Please make it clear in the ticket description which patches have to be applied where.

jdemeyer commented 12 years ago
comment:9

You should use cp -R instead of cp -r (like it was before).

You should deal with upgrading (probably in the new extcode installer): if SAGE_ROOT/data exists, then move SAGE_ROOT/data/extcode to SAGE_ROOT/devel/ext-main and delete the whole directory SAGE_ROOT/data.

jdemeyer commented 12 years ago
comment:10

Why don't you

mkdir -p "$SAGE_DATA"

in spkg/install? That's easier than doing it in the various packages.

jdemeyer commented 12 years ago
comment:11

Replying to @kiwifb:

  • trac13123_library.patch : I am spotting something that I should have reported as a bug a long time ago. In sage/interfaces/gap.py GAP_STAMP is created in $SAGE_LOCAL/bin. This is a no-no if sage is installed globally and accessible by many users. That means all users would need to be able to write to $SAGE_LOCAL/bin to use gap from sage. There is no reason why GAP_STAMP has to be stored there. It should be moved to $DOT_SAGE or possibly even $GAP_DIR. There is a similar problem with qepcad but your patch doesn't touch that particular bit.

See #5155.

ohanar commented 12 years ago
comment:12

Replying to @jdemeyer:

Why don't you

mkdir -p "$SAGE_DATA"

in spkg/install? That's easier than doing it in the various packages.

Maybe easier, but since I was bumping the versions of these spkgs anyways for upgrading purposes, I figured I would have them take care of their own installation, rather than relying upon the build scripts.

ohanar commented 12 years ago
comment:13

Replying to @kiwifb:

That will cause me a lot of work in sage-on-gentoo but overall that will make my life easier in the long term.

You should expect to see a number of these types of changes from me over the next few months. It should definitely simply things for you as the environment variables SAGE_DATA and SAGE_EXTCODE will now be globally respected in Sage.

ohanar commented 12 years ago

Description changed:

--- 
+++ 
@@ -5,3 +5,10 @@
 * [http://wstein.org/home/ohanar/spkgs/polytopes_db-20100210.p2.spkg](http://wstein.org/home/ohanar/spkgs/polytopes_db-20100210.p2.spkg)
 * [http://wstein.org/home/ohanar/spkgs/graphs-20120404.p4.spkg](http://wstein.org/home/ohanar/spkgs/graphs-20120404.p4.spkg)
 * [http://wstein.org/home/ohanar/spkgs/conway_polynomials-0.3.spkg](http://wstein.org/home/ohanar/spkgs/conway_polynomials-0.3.spkg)
+
+Installation Steps:
+* Add new spkgs
+* Apply [attachment: trac13123_library.patch](https://github.com/sagemath/sage-prod/files/10655790/trac13123_library.patch.gz) to the library repo
+* Apply [attachment: trac13123_root.patch](https://github.com/sagemath/sage-prod/files/10655793/trac13123_root.patch.gz) to the root repo
+* Apply [attachment: trac13123_scripts.patch](https://github.com/sagemath/sage-prod/files/10655788/trac13123_scripts.patch.gz) to the scripts repo
+* Apply [attachment: trac13123_extcode.patch](https://github.com/sagemath/sage-prod/files/10655787/trac13123_extcode.patch.gz) to the extcode repo
ohanar commented 12 years ago
comment:15

Replying to @jdemeyer:

You should use cp -R instead of cp -r (like it was before).

You should deal with upgrading (probably in the new extcode installer): if SAGE_ROOT/data exists, then move SAGE_ROOT/data/extcode to SAGE_ROOT/devel/ext-main and delete the whole directory SAGE_ROOT/data.

Done and done.

kiwifb commented 12 years ago
comment:16

Should we deal with gap_stamp in this ticket or should I add it to #5155 and try to push its review for 5.1?

ohanar commented 12 years ago
comment:17

Replying to @kiwifb:

Should we deal with gap_stamp in this ticket or should I add it to #5155 and try to push its review for 5.1?

Since I'm already touching that line, I may as well do it here. If you do end up finishing #5155 in time for 5.1, I'll have to rebase anyway :). (This is under the assumption that this ticket doesn't make it into 5.1, which I feel like is a pretty safe assumption.)

kiwifb commented 12 years ago
comment:18

Thanks. Yes considering that Jeroen wants beta5 to be the last beta before rc our target for both tickets becomes 5.2. I think gap_stamp should go in DOT_SAGE, GAP_DIR seems to be reserved for some gap workspace and it may unwise to put it in there.

ohanar commented 12 years ago
comment:19

Actually GAP_DIR/workspace-*/ are the gap workspaces, so it should be fine to place in GAP_DIR.

kiwifb commented 12 years ago
comment:20

You are right, I missed a line or two in the original file. Looks good to me.

kiwifb commented 12 years ago
comment:21

Question, why do we sometimes get the SAGE_* variables from os.environ and sometimes from sage.misc.misc? Should we try to be uniform and do it one way only?

ohanar commented 12 years ago
comment:22

Replying to @kiwifb:

Question, why do we sometimes get the SAGE_* variables from os.environ and sometimes from sage.misc.misc? Should we try to be uniform and do it one way only?

I would have it imported from sage.misc.misc (well actually I would move it to sage.misc.env, but that currently doesn't exist). I think that I only left one instance of os.environ between SAGE_DATA and SAGE_EXTCODE with this patch (I haven't touched the other SAGE_* with this patch).

ohanar commented 12 years ago
comment:23

FYI, looking at the code there might be an issue with using a GAP_STAMP shared by multiple copies of Sage (I'm not positive, I don't really know the code). For right now I'm using the SAGE_EXTCODE directory as the GAP_STAMP.

kiwifb commented 12 years ago
comment:24

Replying to @ohanar:

FYI, looking at the code there might be an issue with using a GAP_STAMP shared by multiple copies of Sage (I'm not positive, I don't really know the code). For right now I'm using the SAGE_EXTCODE directory as the GAP_STAMP.

Putting it in SAGE_EXTCODE is about as bad as putting it in SAGE_LOCAL/bin as it was initially. If you are concerned about multiple sage session running in parallel that's would still be a problem wherever you put it. I am not sure how the file is used but we don't remove it or anything. The real question is: is it actively used by gap? If gap just check for it's presence then it is OK. If it is used then it may have to be unique for each sage session and be cleaned on exit.

ohanar commented 12 years ago
comment:25

Replying to @kiwifb:

Putting it in SAGE_EXTCODE is about as bad as putting it in SAGE_LOCAL/bin as it was initially.

Not really, since it removes yet another instance of sage trying to write to someplace it shouldn't be at runtime.

The purpose of GAP_STAMP seems to be a check on whether it needs to refresh the workspaces. Really, this will all become a moot point if/when libgap is finished.

kiwifb commented 12 years ago
comment:26

As far as I am concerned SAGE_EXTCODE is a place sage shouldn't write at runtime. On a system wide installation that would be a folder that a user is not supposed to own. If a sys-admin install sage under /usr/local/sage-xxx for example, it would resolve to /usr/local/sage-xxx/local/share/sage/ext and as a sys-admin I object that my users have to write something there. Their home folder, tmp and any scratch space set for them is where it should go.

That's my perspective.

ohanar commented 12 years ago
comment:27

Replying to @kiwifb:

As far as I am concerned SAGE_EXTCODE is a place sage shouldn't write at runtime.

I don't write to SAGE_EXTCODE, I just use its creation for the timestamp that gap wants. It used to be that when you first started sage, it would touch SAGE_LOCAL/bin/gap_stamp and then use the timestamp of that.

I agree, nothing should try to write to SAGE_LOCAL or its subdirectories in the long run.

kiwifb commented 12 years ago
comment:28

OK. If we just want a GAP_STAMP file for everyone and in SAGE_EXTCODE it should really be created by the extcode spkg. Is it a file we really need?

ohanar commented 12 years ago
comment:29

@kiwifb

Do you think you are capable of finishing your review of this? (No pressure, just want to know if you can, so that I can find someone else to review it if you aren't able to.)

kiwifb commented 12 years ago
comment:30

Yes, I will hopefully tomorrow. I was a bit down today - but this is important and we should move this.

jdemeyer commented 12 years ago
comment:31

Why local/share/sage/data and not local/share/sage or even local/share?

jdemeyer commented 12 years ago
comment:32

I would argue for local/share because the stuff in SAGE_DATA (apart from extcode) isn't part of Sage. It's part of external packages/databases. So I think it's wrong to use a sage subdirectory.

Installing databases in local/share/sage/data is like installing all binaries in local/bin/sage/binaries and we don't do that either.

For extcode, local/share/sage makes sense.

jdemeyer commented 12 years ago
comment:33

As far as I can tell, this ticket will break upgrading because an upgraded Sage will still have the SAGE_ROOT/data directory. This could be simply ignored and therefore harmless, but then it needs to be kept inside .hgignore. Alternatively, you could move SAGE_ROOT/data or even delete it.

Also, the database packages should depend on SAGE_ROOT_REPO, otherwise they won't see the new sage-env when upgrading.

kiwifb commented 12 years ago
comment:34

I can agree with the SAGE_DATA location argument of Jeroen. The only argument against is that for most of them sage is the only application I can think of that make use of it. The splitting of the data is purely a convenience because it ease maintenance. In that case local/share/sage would be most appropriate.

ohanar commented 12 years ago
comment:36

Sure, my main thing is just removing SAGE_ROOT/data and moving the databases that were there to some place in SAGE_LOCAL.

ohanar commented 12 years ago

Attachment: elliptic_curves.diff.gz

for review purposes

ohanar commented 12 years ago

Attachment: polytopes_db.diff.gz

for review purpose

ohanar commented 12 years ago

for review purposes

ohanar commented 12 years ago

Attachment: conway_polynomials.diff.gz

for review purposes

ohanar commented 12 years ago
comment:38

Attachment: graphs.diff.gz

Ok, moved everything to SAGE_LOCAL/share (added SAGE_SHARE for convenience), and removed the SAGE_DATA environment variable.

I also added a bit to SAGE_ROOT/spkg/install to handle upgrading.

jdemeyer commented 12 years ago
comment:39

Some optional/experimental packages use SAGE_DATA:

  1. cunningham_tables-1.0
  2. p_group_cohomology-2.1.2
  3. database_stein_watkins_mini.p0
  4. sage-mode-0.7
  5. database_cremona_ellcurve-20120827
  6. database_sloane_oeis-2005-12
  7. database_jones_numfield-v4
  8. soya-0.11.2.p0

Some optional packages use `$SAGE_ROOT/data':

  1. database_odlyzko_zeta
  2. database_symbolic_data-20070206
  3. database_kohel-20060803

For these reasons, I would keep SAGE_DATA as synonym of SAGE_SHARE and also make a symlink

$SAGE_ROOT/data -> local/share
kiwifb commented 12 years ago
comment:40

Keeping SAGE_DATA for a little while as a symlink definitely sound like a good idea. I don't remember explain_picklejar to be removed in trac13123_library.patch before nor do I understand the rational for the removal. Care to shade some light on that?

jdemeyer commented 12 years ago
comment:41

In the extcode patch, this comment is clearly wrong:

cp -pR * "$SAGE_ROOT/devel/ext-main/" # Creates new sagenb-main dir
jdemeyer commented 12 years ago
comment:42

Why change the logic in sage/databases/cremona.py?

jdemeyer commented 12 years ago
comment:43

In extcode spkg-install, why do

if [ -d .hg ]

You know that .hg exists (if not, that would be a bug).

jdemeyer commented 12 years ago
comment:44

spkg/install: indentation should be 4 spaces.