Opened 10 years ago
Closed 10 years ago
#10967 closed defect (wontfix)
R breaks if SAGE_LOCAL undefined
Reported by: | ahd | Owned by: | tbd |
---|---|---|---|
Priority: | minor | Milestone: | sage-duplicate/invalid/wontfix |
Component: | packages: standard | Keywords: | R script SAGE_LOCAL R.sh.in spkg local/bin r-project |
Cc: | jason, was, leif | Merged in: | |
Authors: | Reviewers: | Jeroen Demeyer | |
Report Upstream: | N/A | Work issues: | |
Branch: | Commit: | ||
Dependencies: | #9668 | Stopgaps: |
Description (last modified by )
horatio@havelock ~ $ R /Users/horatio/bin/R: line 212: /lib/Retc/ldpaths: No such file or directory
This is because on line 31 of the script someone rewrote $R_HOME_DIR in terms of $SAGE_LOCAL. When SAGE_LOCAL is missing from the environment of the running shell, the R script breaks. The fix is to test for empty SAGE_LOCAL before the rewrite.
This is from Sage version 4.6.2.
Attachments (2)
Change History (23)
Changed 10 years ago by
comment:1 Changed 10 years ago by
- Description modified (diff)
comment:2 follow-up: ↓ 3 Changed 10 years ago by
Is this ready for review?
comment:3 in reply to: ↑ 2 ; follow-up: ↓ 4 Changed 10 years ago by
Replying to mvngu:
Is this ready for review?
The fix is. It's not a mercurial-generated patch and there don't seem to be any developer guidelines for shell scripts.
comment:4 in reply to: ↑ 3 Changed 10 years ago by
Which file in the Sage distribution are you patching? Provide the exact path to the file from SAGE_ROOT
.
comment:5 follow-up: ↓ 6 Changed 10 years ago by
$SAGE_ROOT/local/bin/R
comment:6 in reply to: ↑ 5 Changed 10 years ago by
Replying to ahd:
$SAGE_ROOT/local/bin/R
That's a wrong file to patch in order to get any changes into the R spkg. The file SAGE_ROOT/local/bin/R
is part of the R spkg, so you should patch the R spkg. Get the latest Sage development release, unpack it, and look under SAGE_ROOT/spkg/standard/
for the R spkg. For more information on patching an spkg, see this page in the Sage Developer's Guide.
comment:7 follow-up: ↓ 8 Changed 10 years ago by
- Component changed from statistics to packages
- Owner changed from amhou to tbd
Does this actually make the problem worse? The way I read the patch, before one did check for SAGE_LOCAL
and afterwards one doesn't. Is this patch backwards?
I can't do it right now, but I should be able to produce an updated R package within a few days if ahd isn't able to.
Changing component, since the problem isn't the statistics functionality but rather the way the package is bundled.
comment:8 in reply to: ↑ 7 ; follow-up: ↓ 9 Changed 10 years ago by
Replying to kcrisman:
Does this actually make the problem worse? The way I read the patch, before one did check for
SAGE_LOCAL
and afterwards one doesn't. Is this patch backwards?I can't do it right now, but I should be able to produce an updated R package within a few days if ahd isn't able to.
Changing component, since the problem isn't the statistics functionality but rather the way the package is bundled.
Probably backwards from your point of view; since I was only lodging this issue as a courtesy, I just did a diff -u between my version of the R script (which doesn't break when SAGE_LOCAL is undefined) and the version that is in my sage installation (which does).
So the best fix from my point of view is to define SAGE_LOCAL in my .profile and leave you all to do what you want, yes?
comment:9 in reply to: ↑ 8 Changed 10 years ago by
- Cc jason was added
Replying to ahd:
Replying to kcrisman:
Does this actually make the problem worse? The way I read the patch, before one did check for
SAGE_LOCAL
and afterwards one doesn't. Is this patch backwards?I can't do it right now, but I should be able to produce an updated R package within a few days if ahd isn't able to.
Probably backwards from your point of view; since I was only lodging this issue as a courtesy, I just did a diff -u between my version of the R script (which doesn't break when SAGE_LOCAL is undefined) and the version that is in my sage installation (which does).
Okay, that explains it. I didn't have a copy of the R script on hand.
So the best fix from my point of view is to define SAGE_LOCAL in my .profile and leave you all to do what you want, yes?
Is there a reason why sage -R
isn't as good of an option? But yes, for now that would be a workaround for you. But you are right that we should fix this; no reason for the R to be unusable outside of Sage.
cc:ing the "package maintainers" as well. R is at 2.12, nearly 2.13, in the meantime, by the way. Some changes might affect build process - now needs 'popen' and a C99 compiler - if we upgrade. Probably those are standard, but I don't know if they are or not.
comment:10 Changed 10 years ago by
Okay, it turns out that this is done in spkg-install in an interesting way - by calling a Python script called fix_hardcode
:
#!/usr/bin/env python import os def fix_hardcode(file): r = open(file).read().replace('#SAGEHACK#','R_HOME_DIR="${SAGE_LOCAL}/lib/R/"') open(file,'w').write(r) S = os.environ['SAGE_LOCAL'] fix_hardcode(S + '/bin/R') fix_hardcode(S + '/lib/R/bin/R')
Which replaces the thing which is actually patched:
# HACK for Sage to avoid hardcoding. NOthing # else has been changed in this file. #R_HOME_DIR="${SAGE_LOCAL}/lib/R/" #SAGEHACK#
Apparently one can't change this part of the file R.sh.in until after configuration and make install? I think because that file must end up in the binary ahd alludes to.
So we just need to change the hack to
r = open(file).read().replace('#SAGEHACK#','foo')
where foo is a string version of
if test -n "${SAGE_LOCAL}"; then R_HOME_DIR="${SAGE_LOCAL}/lib/R/" fi
But I don't know exactly how one puts multiple lines properly in a shell script of this kind. I guess it's a Python script, so maybe \n
would suffice for that? But of course R.sh.in is not a Python script.
comment:11 Changed 10 years ago by
You can use triple quotes in python for multiline strings, so something like this (which is untested) might work:
def fix_hardcode(file): r = open(file).read().replace('#SAGEHACK#', """if test -n "${SAGE_LOCAL}"; then R_HOME_DIR="${SAGE_LOCAL}/lib/R/" fi""") open(file,'w').write(r)
comment:12 Changed 10 years ago by
I don't know anything about R, but this looks more complicated to me. When I look at R.sh.in, right after the #SAGEHACK# part, I see
R_HOME="${R_HOME_DIR}" export R_HOME 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
When I install the spkg, every instance of R_HOME_DIR
gets replaced with the hard-coded full path to SAGE_LOCAL/lib/R, rather than being kept as R_HOME_DIR. Doesn't this defeat the purpose #SAGEHACK#? Won't things go wrong if you move the whole Sage installation?
Anyway, I see the same error "/lib/Retc/ldpaths: No such file or directory" if I run R. If I apply the patch above, it works on one machine (an Intel Mac running OS X 10.6), but on another (also a mac, same OS) I get an error
dyld: Library not loaded: /usr/local/lib/libgfortran.2.dylib Referenced from: /Applications/sage/local/lib/R/lib/libR.dylib Reason: image not found Trace/BPT trap
Indeed, libgfortran is not in /usr/local/lib, I think it's only in /Applications/sage/local/lib.
I'll post the patch anyway, in case it helps. New spkg at http://sage.math.washington.edu/home/palmieri/SPKG/r-2.10.1.p5.spkg. If this works, please mark the ticket as "needs review" and add the URL to the ticket description, so the release manager can find it.
comment:13 Changed 10 years ago by
Ok, I'll look into this eventually. I don't really understand when all this hardcoding happens, because I don't know what R.sh.in does in the build process. My hazy understanding of it would indicate you are right, though. But I guess testing is the key.
comment:14 Changed 10 years ago by
Conceivably related, from a sage-support thread where someone had trouble moving the Mac version. Just for reference.
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).
comment:15 Changed 10 years ago by
This may be related to #9968 and friends.
comment:16 Changed 10 years ago by
- Cc leif added
This should also probably have a nonzero exit code if $SAGE_LOCAL
is undefined.
comment:17 follow-up: ↓ 18 Changed 10 years ago by
IIRC I completely dropped the fix_hardcode
script, but I'm not sure about whether I'm using SAGE_LOCAL
or SAGE_ROOT
; using the latter wouldn't be much better though. ;-)
I'll take a look at it, although I think I wasn't expecting someone to run Sage's R from "outside"...
comment:18 in reply to: ↑ 17 Changed 10 years ago by
- Dependencies set to #9668
- Keywords script SAGE_LOCAL R.sh.in spkg local/bin added
- Milestone changed from sage-4.7.2 to sage-duplicate/invalid/wontfix
- Work issues set to Review as soon as #9668 is ready (new R spkg).
Replying to leif:
... although I think I wasn't expecting someone to run Sage's R from "outside"...
Thinking a bit more about this: ;-)
In general, none of the "stand-alone" programs like R, the PARI/GP interpreter, Maxima etc. as shipped with and installed by Sage (and almost none of the scripts) in $SAGE_ROOT/local/bin/
are intended to be called directly from outside the Sage environment, hence there's also no point in putting this directory into one's PATH
(i.e., one shouldn't).
Instead, one can do any of the following:
- Start a Sage subshell (
sage --sh
), which then has the full Sage environment set up, and also$SAGE_ROOT/local/bin/
in itsPATH
, such that running e.g. (Sage's) R by typing simplyR
at the command prompt works.
- Run (e.g.)
sage --R
from any shell to start Sage's R (assuming an instance of$SAGE_ROOT/sage
is found along one's "global"PATH
; otherwise/path/to/SAGE_ROOT/sage --R
orcd $SAGE_ROOT && ./sage --R
).
- Use the
install_scripts()
function from thesage:
prompt to create light-weight "shortcut" scripts for all "stand-alone" components (programs) installed by Sage, or one can run it by for exampleor$ ./sage -c "install_scripts('$HOME/bin')"
Afterwards, provided that both an instance of$ sudo ./sage -c "install_scripts('/usr/local/bin')" # needs root privileges
$SAGE_ROOT/sage
is found along and the directory passed toinstall_scripts()
is contained inPATH
, one can start (e.g.) Sage's R again by simply typingR
from any shell, i.e., also from "outside the Sage environment".
Note:
install_scripts()
currently has a flaw in that the created shortcuts do not support spaces in arguments passed to them; this has been fixed at #11602, which has been merged into Sage 4.7.2.alpha2.
Nonetheless, I agree that we should at least add a sanity check to e.g. $SAGE_ROOT/local/bin/R
, such that it exits with an appropriate error message in case SAGE_LOCAL
isn't defined (like many scripts already do). I guess such a test is currently missing just because nobody thought of someone trying to call this (and some other scripts) directly, since having them in the PATH
without a properly set up environment was never intended.
I can (and will perhaps) also make some scripts automatically execute themselves in a Sage environment, since this (to me) seems convenient for e.g. devel/sage/spkg-dist
, but most probably not scripts like R
in $SAGE_ROOT/local/bin/
, because of the alternatives already present as given above.
I'll address the specific issue with the R script(s) at #9668 (not #9968 ;-) ), so I've added that ticket as a dependency.
After I've fixed this there (by simply adding a sanity check), this ticket should IMHO be closed as duplicate (not to say "invalid/won't fix").
comment:20 Changed 10 years ago by
- Keywords r-project added; r-rproject removed
comment:21 Changed 10 years ago by
- Resolution set to wontfix
- Reviewers set to Jeroen Demeyer
- Status changed from new to closed
- Work issues Review as soon as #9668 is ready (new R spkg). deleted
patch to fix