Fix #18691 fix to #17572 fix to R.

[EmmanuelCharpentier]

#18691 is broken on fresh install because the sage-env script is invoked at a point where $SAGE_LOCAL or $DOT_SAGE do not exist yet.

Component: packages: standard

Keywords: r-project

Author: Emmanuel Charpentier

Branch/Commit: 18a1262

Reviewer: John Palmieri, Jeroen Demeyer

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

[kcrisman]

comment:2

Changing priority, just change back if I misunderstand the severity of the problem.

[EmmanuelCharpentier]

Branch: u/charpent/fix__18691_fix_to__17572_fix_to_r_

[jdemeyer]

comment:4

Should this

if [ -d "$SAGE_LOCAL" ] ; then
    R_MAKEVARS_SITE="$SAGE_LOCAL/lib/R/share/Makevars.site" && export R_MAKEVARS_SITE

be

if [ -d "$SAGE_LOCAL/lib/R/share" ] ; then
    export R_MAKEVARS_SITE="$SAGE_LOCAL/lib/R/share/Makevars.site"

---

New commits:

9a9c4cc	Original tarball dropped in place.
769fc49	Restored spkg-src.
b46fa1a	Restored ORIGINAL non-functional spkg-src as par Nathaann Cohen's request.
85de560	Condition Makevars file creation to existence of the relevant trees.

[jdemeyer]

Commit: 85de560

[EmmanuelCharpentier]

comment:5

Jeroen : I'm afraid you mixed this ticket with #18820. I do not understand what the hell 9a9c4cc, 769fc49 and b46fa1a, which belong to #18820, are doing in the present #18835.

Please tell me how to undo this mess...

[jdemeyer]

comment:6

Replying to @EmmanuelCharpentier:

Jeroen : I'm afraid you mixed this ticket with #18820.

I didn't do anything, I just made a comment...

Please tell me how to undo this mess...

Just cherry-pick the relevant commit on top of #18691.

[jhpalmieri]

comment:7

I lowered the priority. The build is not broken, but a needless error message is (prominently) displayed when building from scratch.

[jhpalmieri]

comment:8

I know this is not marked as needing review yet, but the proposed change

if [ -d "$SAGE_LOCAL" ] ; then

is not good enough: you need to check whether the directory $SAGE_LOCAL/lib/R/share exists before doing echo foo > "$R_MAKEVARS_SITE".

[EmmanuelCharpentier]

comment:9

I managed to hose my branch. I still have to learn how to revert certain commits.

Stay tuned (but not until tomorrow or late tonight). And accept my apologies.

Emmanuel Charpentier

[kcrisman]

comment:10

I lowered the priority. The build is not broken, but a needless error message is (prominently) displayed when building from scratch.

Got it! Thanks for clarifying.

[sagetrac-git]

Branch pushed to git repo; I updated commit sha1. New commits:

94275f8	Revert "Restored ORIGINAL non-functional spkg-src as par Nathaann Cohen's request."
652a004	Revert "Restored spkg-src."
cc32c78	Revert "Original tarball dropped in place."

[sagetrac-git]

Changed commit from 85de560 to cc32c78

[sagetrac-git]

Changed commit from cc32c78 to 0bbcb05

[sagetrac-git]

Branch pushed to git repo; I updated commit sha1. New commits:

0bbcb05

Condition creating Sage-specific R files to the existence of their destination trees.

[EmmanuelCharpentier]

comment:13

Reading the git doc was ... instructive ...

This one seems good (builds from make distclean without error and passes all tests of make ptestlong ==> needs_review.

[jdemeyer]

comment:14

I think git might get confused when merging this...

Please just remove the bad commits instead of reverting them.

[EmmanuelCharpentier]

comment:15

Replying to @jdemeyer:

I think git might get confused when merging this...

Please just remove the bad commits instead of reverting them.

How do you do that ?

[jdemeyer]

comment:16

There are many possibilities, I guess

git rebase -i

would be the easiest. Then you can edit, squash, remove, reorder commits...

Of course, this qualifies as "rewriting history" which is something you normally should not do for a ticket under review. However, in this case, it is the best solution.

[sagetrac-git]

Changed commit from 0bbcb05 to 18a1262

[sagetrac-git]

Branch pushed to git repo; I updated commit sha1. This was a forced push. New commits:

18a1262

Condition Makevars file creation to existence of the relevant trees.

[EmmanuelCharpentier]

comment:18

Done. I learned something useful. needs_review, again.

However, something worries me. I've read again and again and again (in the Developer's guide, on sage-devel threads, on various ticket brawls...err...discussions) thar "rewriting history" was, a priori a bad idea. I suppose that, in the present case, it is admissible because nobody has yet merged this contribution in anything else. Is this correct ?

Update : this, on top of 6.8beta6, builds with no errors and passes testlong successfully

[EmmanuelCharpentier]

Author: Emmanuel Charpentier

[EmmanuelCharpentier]

comment:20

Update : on top of 6.8beta7, this gives one ptestlong failure :

----------------------------------------------------------------------
sage -t --long --warn-long 67.6 src/sage/structure/dynamic_class.py  # 2 doctests failed
----------------------------------------------------------------------

However, this doctest passes standalone :

charpent@asus16-ec:/usr/local/sage-6.8$ sage -t --long --warn-long 67.6 src/sage/structure/dynamic_class.py
Running doctests with ID 2015-07-03-19-21-20-3619480c.
Git branch: mabranche
Using --optional=mpir,python2,sage,scons
Doctesting 1 file.
sage -t --long --warn-long 67.6 src/sage/structure/dynamic_class.py
    [69 tests, 0.23 s]
----------------------------------------------------------------------
All tests passed!
----------------------------------------------------------------------
Total time for all tests: 0.3 seconds
    cpu time: 0.2 seconds
    cumulative wall time: 0.2 seconds

So I think it's a glitch possibly due to high load.

Still needs_review

[jhpalmieri]

comment:21

I get the same doctest failure in plain 6.8.beta7. I don't think it's related to this ticket.

[jhpalmieri]

comment:22

This looks good to me now.

[jhpalmieri]

Reviewer: John Palmieri, Jeroen Demeyer

[vbraun]

Changed branch from u/charpent/fix__18691_fix_to__17572_fix_to_r_ to 18a1262