Ticket #9847 (closed defect: fixed)

Opened 3 years ago

Last modified 3 months ago

Handle preset R_PROFILE or R_HOME variables

Reported by: mpatel Owned by: GeorgSWeber
Priority: blocker Milestone: sage-5.9
Component: build Keywords:
Cc: ggrafendorfer, kcrisman Work issues:
Report Upstream: N/A Reviewers: Leif Leonhardy, Karl-Dieter Crisman
Authors: Jeroen Demeyer Merged in: sage-5.9.beta2
Dependencies: Stopgaps:

Description (last modified by jdemeyer) (diff)

A preset R_PROFILE variable can cause some R packages to fail to build/install and interfere with doctests.

Georg Grafendorfer  reported this problem on sage-release.

For R_HOME, this yields a doctest failure if set:

**********************************************************************
File "devel/sage/sage/misc/interpreter.py", line 187, in sage.misc.interpreter.SageInteractiveShell.system_raw
Failed example:
    shell.system_raw('R --version')
Expected:
    R version ...
Got:
    WARNING: ignoring environment value of R_HOME
    R version 2.15.2 (2012-10-26) -- "Trick or Treat"
    Copyright (C) 2012 The R Foundation for Statistical Computing
    ISBN 3-900051-07-0
    Platform: x86_64-unknown-linux-gnu (64-bit)
    <BLANKLINE>
    R is free software and comes with ABSOLUTELY NO WARRANTY.
    You are welcome to redistribute it under the terms of the
    GNU General Public License versions 2 or 3.
    For more information about these matters see
    http://www.gnu.org/licenses/.
    <BLANKLINE>
**********************************************************************

Apply 9847_unset_R.patch Download to SAGE_ROOT.

Attachments

9847_unset_R.patch Download (579 bytes) - added by jdemeyer 3 months ago.

Change History

comment:1 Changed 3 years ago by mpatel

From around line 180 of SAGE_LOCAL/bin/sage-env:

if [ -z "$RHOME" ]; then
    RHOME="$SAGE_LOCAL/lib/R" && export RHOME
fi

Should we add a similar test for R_PROFILE?

comment:2 follow-up: ↓ 4 Changed 3 years ago by leif

I think we should perhaps

unset R_PROFILE

in R's spkg-install, too.

comment:3 Changed 3 years ago by kcrisman

Apparently this might be addressed by a solution to #9906.

comment:4 in reply to: ↑ 2 Changed 23 months ago by leif

  • Status changed from new to needs_review
  • Dependencies set to #9906
  • Milestone set to sage-duplicate/invalid/wontfix

Replying to leif:

I think we should perhaps

unset R_PROFILE

in R's spkg-install, too.

#9906 (R 2.10.1.p5) does this now.

comment:5 Changed 23 months ago by kcrisman

Thanks. Trac isn't sending emails, as you know, so I just saw this. (Well, six hours isn't bad.) I'll take a look at #9906 then.

comment:6 follow-up: ↓ 7 Changed 22 months ago by kcrisman

  • Reviewers set to Leif Leonhardy, Karl-Dieter Crisman

Sorry I didn't come back to this. It is correct that the spkgs at #9906 do this in the spkg scripts.

So I can give this positive review as long as Leif promises that if I or some other reviewer find out that the fix at #9906 doesn't solve the type of problem above, that it will get addressed there and no longer here!

Sound good? ;-) But I don't think that will be necessary.

comment:7 in reply to: ↑ 6 ; follow-up: ↓ 8 Changed 22 months ago by leif

Replying to kcrisman:

Sorry I didn't come back to this. It is correct that the spkgs at #9906 do this in the spkg scripts.

So I can give this positive review as long as Leif promises that if I or some other reviewer find out that the fix at #9906 doesn't solve the type of problem above, that it will get addressed there and no longer here!

In that case, you should also give this ticket positive review, such that the release manager will close it, with "resolution: duplicate".

If issues with R_PROFILE should (re)arise unrelated to (the installation of) the R spkg, i.e., which would have to be fixed elsewhere, we can still reopen this ticket.

(IIRC some of the problems showed up during doctesting, but because R_PROFILE had been set during installation, so are now fixed by the new spkg. If there needs to be done more w.r.t. this -- which I don't think -- this can be addressed at #9906 as Karl-Dieter suggests.)

Note that this ticket already has #9906 as its dependency.

comment:8 in reply to: ↑ 7 Changed 22 months ago by kcrisman

  • Status changed from needs_review to positive_review

So I can give this positive review as long as Leif promises that if I or some other reviewer find out that the fix at #9906 doesn't solve the type of problem above, that it will get addressed there and no longer here!

In that case, you should also give this ticket positive review, such that the release manager will close it, with "resolution: duplicate".

Correct. I just wanted to make sure :) Great.

(IIRC some of the problems showed up during doctesting, but because R_PROFILE had been set during installation, so are now fixed by the new spkg. If there needs to be done more w.r.t. this -- which I don't think -- this can be addressed at #9906 as Karl-Dieter suggests.)

Yeah, I'm pretty sure that was the issue, or more precisely that R_PROFILE was not (re)set.

Note that this ticket already has #9906 as its dependency.

Well, that wouldn't be necessary with a duplicate/wontfix one.

See you at #9906, eventually (when I get time to do all the many tests on that).

comment:9 Changed 19 months ago by jdemeyer

  • Status changed from positive_review to closed
  • Resolution set to duplicate

comment:10 Changed 10 months ago by jdemeyer

  • Status changed from closed to new
  • Resolution duplicate deleted
  • Milestone changed from sage-duplicate/invalid/wontfix to sage-5.4

comment:11 Changed 10 months ago by jdemeyer

I order to finally finish #9906, I decided not to add this change anymore.

comment:12 Changed 9 months ago by kcrisman

Ok. What was the solution there - simply unset R_PROFILE in its spkg-install as suggested in comment:2, or something more involved like in comment:1 (sage-env), or both, or... after reading Leif's comments and Georg's diagnosis in the thread in the description, maybe the simplest solution will be best.

comment:13 Changed 3 months ago by jdemeyer

  • Description modified (diff)
  • Summary changed from Handle a preset R_PROFILE variable to Handle a preset R_PROFILE or R_HOME variables

comment:14 Changed 3 months ago by jdemeyer

  • Priority changed from major to blocker

Changed 3 months ago by jdemeyer

comment:15 Changed 3 months ago by jdemeyer

  • Description modified (diff)
  • Authors set to Jeroen Demeyer

comment:16 follow-up: ↓ 17 Changed 3 months ago by kcrisman

Looks good, based on the various discussions in question.

And the R_HOME and R_PROFILE will be "reset" after one exits the Sage shell, correct? I do know that some people use their own custom R packages (or try to) but I guess that isn't supported behavior, given the structure of Sage.

In that case, applies and presumably works (I certainly can't test this, but based on all the above) so we should get this in, at long last.

comment:17 in reply to: ↑ 16 Changed 3 months ago by jdemeyer

Replying to kcrisman:

And the R_HOME and R_PROFILE will be "reset" after one exits the Sage shell, correct?

Sure, although nothing is really "reset", you just get your old shell back which never saw sage-env.

comment:18 Changed 3 months ago by kcrisman

  • Status changed from new to needs_review

Fair enough. I think this is a realistic solution to this; I don't think we have to support combinations of these environments. If someone really wanted to, they could set these things in init.sage, right?

comment:19 Changed 3 months ago by kcrisman

  • Status changed from needs_review to positive_review
  • Dependencies #9906 deleted

comment:20 Changed 3 months ago by jdemeyer

  • Summary changed from Handle a preset R_PROFILE or R_HOME variables to Handle preset R_PROFILE or R_HOME variables

comment:21 Changed 3 months ago by jdemeyer

  • Status changed from positive_review to closed
  • Resolution set to fixed
  • Merged in set to sage-5.9.beta2
Note: See TracTickets for help on using tickets.