Opened 12 years ago
Closed 10 years ago
#9668 closed defect (fixed)
Fix hardcoding of paths in R binary
Reported by: | Karl-Dieter Crisman | Owned by: | Leif Leonhardy |
---|---|---|---|
Priority: | major | Milestone: | sage-5.9 |
Component: | packages: standard | Keywords: | R spkg R.sh.in libR.pc pkg-config hard-coded package installation R_HOME_DIR sd32 r-project |
Cc: | Leif Leonhardy, John Palmieri | Merged in: | sage-5.9.rc0 |
Authors: | John Palmieri | Reviewers: | Karl-Dieter Crisman, Jeroen Demeyer |
Report Upstream: | N/A | Work issues: | |
Branch: | Commit: | ||
Dependencies: | Stopgaps: |
Description (last modified by )
See this thread on sage-support.
Here is how I got the optional package automap to install into a binary sage R. Go into the sage directory and edit the following files: local/bin/R and local/lib/R/bin/R and change all the hard-set user variables "/scratch/...." to the true locations of R_HOME_DIR, R_HOME, R_INCLUDE_DIR, R_SHARE_DIR and for good measure, R_DOC_DIR. Replace the default string EVERYWHERE in the file. I then exported SAGE_HOME as well (Not sure that this is needed.), and run local/bin/R Inside R, install.packages("automap") No more build errors, and when I restart R, automap loads using library. Just have to try it out from sage now. Any chance there's a script to find all of these hard-set strings and change them to correct values?
Related (R package):
New spkg: http://boxen.math.washington.edu/home/palmieri/SPKG/r-2.15.2.p2.spkg
Attachments (3)
Change History (53)
comment:1 Changed 12 years ago by
Cc: | Leif Leonhardy added |
---|
comment:2 Changed 11 years ago by
Description: | modified (diff) |
---|
comment:3 follow-up: 4 Changed 11 years ago by
comment:4 follow-up: 5 Changed 11 years ago by
Replying to kcrisman:
But what about #9668 (this ticket) itself? Does the stuff that is "almost" ready at #9906 address that?
Not yet, i.e. not all of it. See this comment there.
comment:5 follow-up: 6 Changed 11 years ago by
Replying to leif:
Replying to kcrisman:
But what about #9668 (this ticket) itself? Does the stuff that is "almost" ready at #9906 address that?
Not yet, i.e. not all of it. See this comment there.
Ok, a bit more subtle than I expected (R is quite weird, failed to build inbetween), but I now have a p6 that fixes both the R
scripts and the pkg-config
file (libR.pc
).
There was another issue with R someone reported on sage-devel which was caused by a "complicated" setting of an environment variable, in this case PAGER
, but that's certainly an upstream problem and rather exotic.
comment:6 Changed 11 years ago by
Replying to leif:
Replying to leif:
Replying to kcrisman:
But what about #9668 (this ticket) itself? Does the stuff that is "almost" ready at #9906 address that?
Not yet, i.e. not all of it. See this comment there.
Ok, a bit more subtle than I expected (R is quite weird, failed to build inbetween), but I now have a p6 that fixes both the
R
scripts and thepkg-config
file (libR.pc
).
Okay, I'll watch this space.
There was another issue with R someone reported on sage-devel which was caused by a "complicated" setting of an environment variable, in this case
PAGER
, but that's certainly an upstream problem and rather exotic.
Agreed.
comment:7 follow-up: 8 Changed 11 years ago by
Description: | modified (diff) |
---|
See also #10967. We should probably either incorporate that here, or make a followup spkg there.
comment:8 follow-up: 9 Changed 11 years ago by
Replying to kcrisman:
See also #10967. We should probably either incorporate that here, or make a followup spkg there.
Almost certainly the former.
[Note to myself:]
The problem is that removing any reference to SAGE_ROOT
and SAGE_LOCAL
disables the script being "automatically" relocated in the first place.
I'll then have to guess SAGE_ROOT
if none of Sage's variables are defined. We also have two copies of the R script, one in local/bin/
, and one in local/lib/R/bin/
.
comment:9 follow-ups: 10 19 Changed 11 years ago by
Dependencies: | → #9906 |
---|---|
Keywords: | R spkg R.sh.in libR.pc pkg-config hard-coded package installation R_HOME_DIR added |
Owner: | changed from tbd to Leif Leonhardy |
Work issues: | → Provide an R 2.10.1.p6 spkg. |
Replying to leif:
[Note to myself:]
The problem is that removing any reference to
SAGE_ROOT
andSAGE_LOCAL
disables the script being "automatically" relocated in the first place.I'll then have to guess
SAGE_ROOT
if none of Sage's variables are defined. We also have two copies of the R script, one inlocal/bin/
, and one inlocal/lib/R/bin/
.
Guessing SAGE_ROOT
there is simply bullshit. Simply add a sanity check, pointing to sage --R
, install_scripts()
and perhaps sage --sh
in case SAGE_LOCAL
isn't defined.
Furthermore:
- Address / check the following (copied from ticket:10967:14):
The problem was with hard-coded paths, not the permissions. Anyway, the fix was easy. I opened all the files listed above by George: sage/local/lib/R/bin/R sage/local/lib/R/bin/libtool sage/local/lib/R/etc/Makeconf sage/local/lib/R/etc/ldpath sage/local/lib/R/etc/Renviron sage/local/bin/R and edited obvious lines containing hardcoded paths (using find- replace-all at once).
- Change
libR.pc
to use either${pc_top_builddir}
or$${SAGE_ROOT}
. (In the former case, perhaps also setPC_TOP_BUILD_DIR
to$SAGE_ROOT
, or prepend$SAGE_ROOT:
, unless it is already contained.)
comment:10 Changed 11 years ago by
Replying to leif:
- Change
libR.pc
to use either${pc_top_builddir}
or$${SAGE_ROOT}
. (In the former case, perhaps also setPC_TOP_BUILD_DIR
to$SAGE_ROOT
, or prepend$SAGE_ROOT:
, unless it is already contained.)
Should read:
- Change
libR.pc
to use either${pc_top_builddir}
or$${SAGE_ROOT}
. (In the former case, setPKG_CONFIG_TOP_BUILD_DIR
to$SAGE_ROOT
unless it is already, perhaps issuing a warning in case it isn't, or especially if its setting differs.)
Also prepend$SAGE_ROOT/local/lib/pkgconfig
toPKG_CONFIG_PATH
, unless it is already contained. Warn ifPKG_CONFIG_PATH
is empty, then set it to$SAGE_ROOT/local/lib/pkgconfig
.
comment:11 Changed 11 years ago by
Keywords: | sd32 added |
---|
comment:12 Changed 11 years ago by
Keywords: | r-project added |
---|
comment:13 Changed 10 years ago by
Cc: | John Palmieri added |
---|
comment:14 Changed 10 years ago by
This problem prevents Sage from being relocatable on Solaris, or at least on the skynet machines mark and mark2: if I build Sage, then move the entire Sage directory, then run sage
so the relocation scripts get executed, and then run sage -R
, I get an error:
$ ./sage -R ld.so.1: R: fatal: libgcc_s.so.1: version `GCC_4.3.0' not found (required by file /usr/local/gcc-4.7.0/sparc-SunOS-ultrasparc3/lib/libgomp.so.1) ld.so.1: R: fatal: libgcc_s.so.1: open failed: No such file or directory /home/palmieri/mark2/sage-5.4.rc2-7797/spkg/bin/sage: line 457: 28710 Killed "$SAGE_LOCAL/bin/R" "$@"
I tried just modifying local/bin/R and local/lib/R/bin/R, replacing the hard-coded paths with $SAGE_ROOT
, but I still got an error.
comment:15 Changed 10 years ago by
I doubt this is related to the original topic at all. The setup on skynet's Solaris machines is (or used to be) quite broken, as there are outdated versions of shared libraries left around in typical paths, and some R scripts insist on messing up your paths such that the former gets relevant.
Unfortunately I don't recall what the exact problem was, and how I managed to work around it, just that I had to [and somehow successfully did]; I think this problem appeared with, or was related to, [the installation of] GCC 4.7.0. In case I am right, changing PATH
and/or LD_LIBRARY_PATH
should "solve" the issue.
Sorry for not being of much help here, at least right now... ;-)
comment:16 Changed 10 years ago by
Well, if the problems on mark are not related, then for this ticket, we could just add some lines at the end of spkg-install:
# Make R relocatable by using "$SAGE_ROOT" instead of the hardcoded path. sed -e "s|$SAGE_ROOT|\$SAGE_ROOT/|" "$SAGE_LOCAL/bin/R" > "$SAGE_LOCAL/bin/R.tmp" && mv "$SAGE_LOCAL/bin/R.tmp" "$SAGE_LOCAL/bin/R" && chmod a+x "$SAGE_LOCAL/bin/R" sed -e "s|$SAGE_ROOT|\$SAGE_ROOT/|" "$SAGE_LOCAL/lib/R/bin/R" > "$SAGE_LOCAL/lib/R/bin/R.tmp" && mv "$SAGE_LOCAL/lib/R/bin/R.tmp" "$SAGE_LOCAL/lib/R/bin/R" && chmod a+x "$SAGE_LOCAL/lib/R/bin/R"
(It's too bad that the -i
flag for sed is not portable.) libR.pc
already uses SAGE_ROOT. What else needs to be done?
comment:17 Changed 10 years ago by
Authors: | → John Palmieri |
---|---|
Description: | modified (diff) |
Status: | new → needs_review |
I've posted a new spkg, along with the corresponding patch.
comment:18 Changed 10 years ago by
Work issues: | Provide an R 2.10.1.p6 spkg. |
---|
comment:19 Changed 10 years ago by
Replying to leif:
- Address / check the following (copied from ticket:10967:14):
The problem was with hard-coded paths, not the permissions. Anyway, the fix was easy. I opened all the files listed above by George: sage/local/lib/R/bin/R sage/local/lib/R/bin/libtool sage/local/lib/R/etc/Makeconf sage/local/lib/R/etc/ldpath sage/local/lib/R/etc/Renviron sage/local/bin/R and edited obvious lines containing hardcoded paths (using find- replace-all at once).
It's easy enough to add these to the "for" loop that I added to spkg-install, so I might as well do that. For what it's worth, the binary files Rscript
in both local/bin and local/lib/R/bin also have the path hard-coded in them, but I don't know how to fix this, or whether it's important. When are those files used?
comment:20 Changed 10 years ago by
Dependencies: | #9906 |
---|
This patch for the spkg makes sense to me, anyway.
Did you end up changing the libR.pc stuff in comment:10? Or was that done elsewhere?
comment:21 Changed 10 years ago by
The ".pc" files are handled by the sage-location
script, I think. In any case, I believe that if you run make build
on a fresh Sage tarball, then libR.pc should be in good shape.
comment:22 Changed 10 years ago by
Reviewers: | → Karl-Dieter Crisman |
---|---|
Status: | needs_review → positive_review |
This looks good, and I now recall the sage-location discussion. I think this is ready to go; it behaves as advertised. I can't check whether this fixes the problems on Solaris directly, but it should fix the problem in the original post too - well, if such a person were to upgrade the spkg without just downloading a new Sage.
comment:23 Changed 10 years ago by
A slight correction to what I said before: just doing make build
doesn't fix libR.pc, but running Sage once takes care of it.
comment:24 Changed 10 years ago by
Or anything else that triggers sage-location
(nowadays, any spkg upgrade even), yes.
comment:25 follow-up: 26 Changed 10 years ago by
Status: | positive_review → needs_work |
---|
I think using sed
for this is very fragile, as any special characters in $SAGE_ROOT
might cause this script to malfunction. A better solution would be to patch the R sources. This is already partially done (see R.sh.patch
in the R sources) to allow for basic relocation of R, but apparently installing R packages doesn't work.
comment:26 follow-up: 27 Changed 10 years ago by
Replying to jdemeyer:
I think using
sed
for this is very fragile, as any special characters in$SAGE_ROOT
might cause this script to malfunction. A better
Out of curiosity, do we have any current restrictions on the characters in $SAGE_ROOT
? I feel like we already don't allow whitespace, though that might be an old memory. Does Sage work in non-ASCII paths as well, say with Cyrillic characters?
comment:27 follow-up: 28 Changed 10 years ago by
Replying to kcrisman:
Out of curiosity, do we have any current restrictions on the characters in
$SAGE_ROOT
?
We certainly do, although only "no spaces" is documented.
But I doubt Sage will work correctly with a directory like /home/jdemeyer/.#|"+*=@$'
(which is a valid UNIX filename)
comment:28 Changed 10 years ago by
Out of curiosity, do we have any current restrictions on the characters in
$SAGE_ROOT
?We certainly do, although only "no spaces" is documented.
But I doubt Sage will work correctly with a directory like
/home/jdemeyer/.#|"+*=@$'
(which is a valid UNIX filename)
You sound like you're arguing that we might as well use sed
:-) If I wasn't lazy I'd try to grep through other spkgs to see if it's used elsewhere - I'm almost sure I've seen it in use before...
comment:29 Changed 10 years ago by
Status: | needs_work → positive_review |
---|
Here is another attempt at fixing this.
comment:30 Changed 10 years ago by
Status: | positive_review → needs_work |
---|
comment:31 Changed 10 years ago by
Status: | needs_work → needs_review |
---|
Wishful thinking: I accidentally set this to "positive review". Oops.
I think the changes in trac_9668-r.v2.patch could be polished a bit, but I'm not going to bother until people agree with the general approach.
comment:32 follow-up: 33 Changed 10 years ago by
I don't like this at all:
./configure --prefix="\$SAGE_LOCAL"
If it works, you're probably relying on a bug. Why is it needed?
Also, the first hunk in Makefile.in.patch
is not needed.
In any case, this needs to be rebased as R has been upgraded.
comment:33 follow-up: 34 Changed 10 years ago by
Replying to jdemeyer:
I don't like this at all:
./configure --prefix="\$SAGE_LOCAL"If it works, you're probably relying on a bug. Why is it needed?
If I remember correctly, the prefix directory gets written into various of the files in SAGE_LOCAL/lib/R/bin/
, so using \$SAGE_LOCAL
instead of $SAGE_LOCAL
means that "$SAGE_LOCAL" gets written verbatim to the file, instead of expanded first and then written. This makes those files relocatable.
Why don't you like it?
comment:34 Changed 10 years ago by
Replying to jhpalmieri:
Why don't you like it?
Because the fact that it works looks like a bug rather than a feature.
comment:35 follow-up: 36 Changed 10 years ago by
If you look at src/scripts/R.sh.in
, it has lines like
R_HOME_DIR="@prefix@/${libnn}/R"
My understanding is that prefix
gets set by the configure script, stored in Makefile.conf
, and then read by src/scripts/Makefile
so this variable's value can get used when making the R script. In particular, the R people are deliberately using the prefix
variable here. So it looks like their design decision, not a bug. But I'm not sure I understand your point.
comment:36 Changed 10 years ago by
Replying to jhpalmieri:
If you look at
src/scripts/R.sh.in
, it has lines likeR_HOME_DIR="@prefix@/${libnn}/R"
This line is completely irrelevant, as we already patch R.sh.in
to overwrite R_HOME_DIR
.
Changed 10 years ago by
Attachment: | trac_9668-r.v3.patch added |
---|
comment:37 follow-up: 39 Changed 10 years ago by
I'll post a new version of the spkg when sage.math is back up. I've attached the patch. This has the effect of changing lines in local/bin/R
(and the corresponding file local/lib/R/bin/R
) from
R_SHARE_DIR=.../sage-5.8.beta4/local/lib/R/share export R_SHARE_DIR R_INCLUDE_DIR=.../sage-5.8.beta4/local/lib/R/include export R_INCLUDE_DIR R_DOC_DIR=.../sage-5.8.beta4/local/lib/R/doc export R_DOC_DIR
to
R_SHARE_DIR="${R_HOME_DIR}/share" export R_SHARE_DIR R_INCLUDE_DIR="${R_HOME_DIR}/include" export R_INCLUDE_DIR R_DOC_DIR="${R_HOME_DIR}/doc" export R_DOC_DIR
With this change, running sage -R
and then install.packages("automap")
seems to work.
comment:39 follow-up: 40 Changed 10 years ago by
Replying to jhpalmieri:
I'll post a new version of the spkg when sage.math is back up.
Replying to jhpalmieri:
New spkg posted.
I still cannot log into sage.math... ;-)
comment:40 Changed 10 years ago by
Replying to leif:
Replying to jhpalmieri:
I'll post a new version of the spkg when sage.math is back up.
Replying to jhpalmieri:
New spkg posted.
I still cannot log into sage.math... ;-)
I did an scp
to boxen. If you want, I can try to keep my promise by posting the new version (again) when sage.math comes back up...
comment:41 Changed 10 years ago by
For what it's worth, someone reported on ask.sagemath.org that this spkg fixed their problem with installing an R package.
comment:42 Changed 10 years ago by
I think that sounds like a positive review, combined with Jeroen's good comments... What do you think?
comment:43 follow-up: 44 Changed 10 years ago by
I think that someone should confirm that with the new spkg, in the script local/bin/R
, the variables R_SHARE_DIR
, R_INCLUDE_DIR
, and R_DOC_DIR
are now defined in terms of R_HOME_DIR
rather than being hard-coded paths as they are with the current spkg.
comment:44 Changed 10 years ago by
I think that someone should confirm that with the new spkg, in the script
local/bin/R
, the variablesR_SHARE_DIR
,R_INCLUDE_DIR
, andR_DOC_DIR
are now defined in terms ofR_HOME_DIR
rather than being hard-coded paths as they are with the current spkg.
I can do this.
comment:45 Changed 10 years ago by
Reviewers: | Karl-Dieter Crisman → Karl-Dieter Crisman, Jeroen Demeyer |
---|---|
Status: | needs_review → positive_review |
I confirmed that moving a Sage install (not just binary) caused installing an R package to fail with precisely the problems one would expect if these were incorrectly defined (e.g.
Warning: R include directory is empty -- perhaps need to install R-devel.rpm or similar
with appropriate nonexistent directory referenced). The script local/bin/R
had the (now incorrect) paths.
Then installing this spkg and retrying caused success, and local/bin/R
looks right now too. Nice work!
comment:46 Changed 10 years ago by
Status: | positive_review → needs_info |
---|
One question, though... when I move Sage back and run Sage, it doesn't change R_HOME_DIR
back.
R_HOME_DIR=/Users/.../Downloads/tempR/sage-5.9.beta5/local/lib/R
when it should be
R_HOME_DIR=/Users/.../Downloads/sage-5.9.beta5/local/lib/R
So this was changed, presumably, when I reinstalled the spkg. It doesn't impact installing new R packages, by the way, nor functionality of R.
In particular, moving a different Sage installation and starting Sage changes some things, but doesn't change the location of R_HOME_DIR
in local/bin/R
. I'm not sure why that doesn't affect functionality, but presumably this should somehow be taken care of. On this ticket?
comment:47 Changed 10 years ago by
It took me a little while to understand this, too. Right before the lines defining R_SHARE_DIR
, etc., there are lines
if test x$SAGE_BUILDING_R = x; then R_HOME_DIR="$SAGE_LOCAL/lib/R/" fi
If you're not building the R spkg, then this will be executed, overriding the hard-coded path earlier in the script, and setting R_HOME_DIR
to the desired portable setting. I don't know R, so I don't know how to test this: if you run sage -R
, can you execute some R command to tell you the current setting of R_HOME_DIR
?
comment:48 Changed 10 years ago by
Status: | needs_info → needs_review |
---|
Well, that is a Sage-specific thing, I think, but we have
R_HOME="${R_HOME_DIR}"
later on and also
$ ./sage -R RHOME /Users/.../Downloads/sage-5.9.beta5/local/lib/R/
So all is well, I think.
comment:49 Changed 10 years ago by
Status: | needs_review → positive_review |
---|
comment:50 Changed 10 years ago by
Merged in: | → sage-5.9.rc0 |
---|---|
Resolution: | → fixed |
Status: | positive_review → closed |
IIRC I have some "almost" ready R spkg for #9906, which also does a lot of clean-up in
spkg-install
...