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:

Status badges

Description (last modified by ahd)

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)

trac_10967_R_sage_local_fix.patch (655 bytes) - added by ahd 10 years ago.
patch to fix
trac_10967-R.patch (1.8 KB) - added by jhpalmieri 10 years ago.
hg patch for R spkg

Download all attachments as: .zip

Change History (23)

Changed 10 years ago by ahd

patch to fix

comment:1 Changed 10 years ago by ahd

  • Description modified (diff)

comment:2 follow-up: Changed 10 years ago by mvngu

Is this ready for review?

comment:3 in reply to: ↑ 2 ; follow-up: Changed 10 years ago by ahd

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 mvngu

Which file in the Sage distribution are you patching? Provide the exact path to the file from SAGE_ROOT.

comment:5 follow-up: Changed 10 years ago by ahd

$SAGE_ROOT/local/bin/R

comment:6 in reply to: ↑ 5 Changed 10 years ago by mvngu

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: Changed 10 years ago by kcrisman

  • 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: Changed 10 years ago by 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.

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 kcrisman

  • 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 kcrisman

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 jhpalmieri

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 jhpalmieri

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.

Changed 10 years ago by jhpalmieri

hg patch for R spkg

comment:13 Changed 10 years ago by kcrisman

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 kcrisman

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 kcrisman

This may be related to #9968 and friends.

comment:16 Changed 10 years ago by kcrisman

  • Cc leif added

This should also probably have a nonzero exit code if $SAGE_LOCAL is undefined.

comment:17 follow-up: Changed 10 years ago by leif

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 leif

  • 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 its PATH, such that running e.g. (Sage's) R by typing simply R 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 or cd $SAGE_ROOT && ./sage --R).
  • Use the install_scripts() function from the sage: prompt to create light-weight "shortcut" scripts for all "stand-alone" components (programs) installed by Sage, or one can run it by for example
    $ ./sage -c "install_scripts('$HOME/bin')"
    
    or
    $ sudo ./sage -c "install_scripts('/usr/local/bin')" # needs root privileges
    
    Afterwards, provided that both an instance of $SAGE_ROOT/sage is found along and the directory passed to install_scripts() is contained in PATH, one can start (e.g.) Sage's R again by simply typing R 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:19 Changed 10 years ago by kcrisman

  • Keywords r-rproject added

See also #12057.

comment:20 Changed 10 years ago by kcrisman

  • Keywords r-project added; r-rproject removed

comment:21 Changed 10 years ago by jdemeyer

  • 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
Note: See TracTickets for help on using tickets.