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

R's spkg-install calls "sage -f ..." to install the contained Rpy spkg #10016

Closed
nexttime mannequin opened this issue Sep 25, 2010 · 17 comments
Closed

R's spkg-install calls "sage -f ..." to install the contained Rpy spkg #10016

nexttime mannequin opened this issue Sep 25, 2010 · 17 comments

Comments

@nexttime
Copy link
Mannequin

nexttime mannequin commented Sep 25, 2010

This already caused trouble when the Sage scripts spkg wasn't yet installed, and making all standard packages depend on the complete set of Sage scripts (which had actually then been done) is an overkill.

There's a lot wrong with this spkg, and the Rpy spkg should be moved out of R's (#9906), but as a short-term solution, I decided to just replace the call to $SAGE_ROOT/sage (which requires sage-sage) by a direct one to $SAGE_ROOT/local/bin/sage-spkg, which should be present in any case, s.t. we don't have to make the R spkg depend on the Sage scripts spkg.

The new p4 spkg does not address any other issues.


r-2.10.1.p4 (Leif Leonhardy, September 25th 2010)


New spkg: http://spkg-upload.googlecode.com/files/r-2.10.1.p4.spkg

md5sum: afb2987172a2d740227bd24227e6546a

CC: @qed777

Component: packages: standard

Keywords: sage-spkg deps scripts dependency

Author: Leif Leonhardy

Reviewer: John Palmieri

Merged: sage-4.6.alpha2

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

@nexttime nexttime mannequin added this to the sage-4.6 milestone Sep 25, 2010
@nexttime nexttime mannequin self-assigned this Sep 25, 2010
@nexttime

This comment has been minimized.

@nexttime nexttime mannequin added the s: needs review label Sep 25, 2010
@nexttime
Copy link
Mannequin Author

nexttime mannequin commented Sep 25, 2010

Attachment: trac_10016-r-2.10.1.p3-p4.spkg.patch.gz

Diff between the p3 and p4 spkg (actually a Mercurial patch). For reference/review.

@kcrisman
Copy link
Member

comment:2
# sage -f "$RPY_VER".spkg # EVIL

I think you have different standards of evil than I do :)

But this sounds like a good intermediate step here. What I understand of the change in the script makes sense. I don't know how tee works, and whether pipestatus really is in the path or what it does, so I sadly can't review this. Maybe you should cc: the maintainers? (Which I think are jason and was.)

@nexttime
Copy link
Mannequin Author

nexttime mannequin commented Sep 29, 2010

comment:3

I just noticed this doesn't fully work with spaces in the path, but since spaces aren't supported yet anyway (and most probably won't be soon), and this is just a temporary solution (cf. #9906), I'm not going to change the fairly large spkg again here...

@nexttime
Copy link
Mannequin Author

nexttime mannequin commented Sep 29, 2010

comment:4

Replying to @nexttime:

[...] I'm not going to change the fairly large spkg again here...

... unless somebody finds other things that have to be fixed here.

@jhpalmieri
Copy link
Member

comment:5

So because of the tee command, is the install log for rpy going to appear in both r-2.10.1.p4.log and rpy2-2.0.8.log?

I suppose I should try it and see. Should I use the "deps" file from #9896?

@jhpalmieri
Copy link
Member

comment:6

Just to experiment, I took deps from #9896, took out the dependency of R on sage-scripts, and put in a dependency of sage-scripts on R.

@nexttime
Copy link
Mannequin Author

nexttime mannequin commented Sep 29, 2010

comment:7

Trac...

Replying to @jhpalmieri:

So because of the tee command, is the install log for rpy going to appear in both r-2.10.1.p4.log and rpy2-2.0.8.log?

I will appear in both. Note that previously the ./sage -f ... directly wrote (appended) to $SAGE_ROOT/install.log as well, which I consider also a bug.

I suppose I should try it and see. Should I use the "deps" file from #9896?

This makes no difference, since R still depends on the scripts in both.
(But you could remove the dependency of R on sage_scripts in the deps.v2b at #9896.)

@nexttime
Copy link
Mannequin Author

nexttime mannequin commented Sep 29, 2010

comment:8

LOL, s/I will/It will/.

@nexttime
Copy link
Mannequin Author

nexttime mannequin commented Sep 29, 2010

comment:9

Replying to @jhpalmieri:

Just to experiment, I took deps from #9896, took out the dependency of R on sage-scripts, and put in a dependency of sage-scripts on R.

Good idea. But you should (at least) also remove local/bin/sage-sage to make such a test effective. In principle, you could remove everything from local/bin except sage-spkg and sage-env IIRC, but you'd have to install the new R spkg by copying it to spkg/ and then running make (or just make build).

Or rebuild Sage from scratch with the new R spkg and such deps, as I did...

@nexttime
Copy link
Mannequin Author

nexttime mannequin commented Sep 29, 2010

comment:10

Replying to @nexttime:

Replying to @jhpalmieri:

Just to experiment, I took deps from #9896, took out the dependency of R on sage-scripts, and put in a dependency of sage-scripts on R.

Good idea. But you should (at least) also remove local/bin/sage-sage to make such a test effective. In principle, you could remove everything from local/bin except sage-spkg and sage-env IIRC, but you'd have to install the new R spkg by copying it to spkg/ and then running make (or just make build).

And of course remove spkg/installed/sage_scripts-* before running make.

Or rebuild Sage from scratch with the new R spkg and such deps, as I did...

@nexttime
Copy link
Mannequin Author

nexttime mannequin commented Sep 29, 2010

comment:11

Argh, "copying it to spkg/" should read "copying it to spkg/standard/".

@jhpalmieri
Copy link
Member

comment:12

I wasn't clear: I'm building from scratch with the modified deps and the new R spkg, so I didn't need to remove anything. (I had a few false starts, but "make distclean" fixed those.)

@jhpalmieri
Copy link
Member

Reviewer: John Palmieri

@jhpalmieri
Copy link
Member

comment:13

This looks good to me. With my purely experimental version of deps, it emphasizes the problem on #9434: since sage-scripts is installed very late, there are many warning messages about being unable to find sage-banner.

@nexttime
Copy link
Mannequin Author

nexttime mannequin commented Sep 29, 2010

comment:14

Thanks for reviewing this.

Replying to @jhpalmieri:

[...] With my purely experimental version of deps, it emphasizes the problem on #9434: since sage-scripts is installed very late, there are many warning messages about being unable to find sage-banner.

:D

@qed777
Copy link
Mannequin

qed777 mannequin commented Sep 29, 2010

Merged: sage-4.6.alpha2

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

2 participants