Opened 11 years ago
Closed 11 years ago
#11704 closed enhancement (fixed)
Resolve symbolic links in $HOME/.sage
Reported by: | jdemeyer | Owned by: | |
---|---|---|---|
Priority: | major | Milestone: | sage-4.8 |
Component: | scripts | Keywords: | |
Cc: | leif | Merged in: | sage-4.8.alpha6 |
Authors: | Jeroen Demeyer | Reviewers: | John Palmieri |
Report Upstream: | N/A | Work issues: | |
Branch: | Commit: | ||
Dependencies: | #11924 | Stopgaps: |
Description (last modified by )
Similarly to SAGE_ROOT
, it might be good to resolve symbolic links in $HOME/.sage
when assigning DOT_SAGE
. This can be useful for example when $HOME/.sage
is a symbolic link to a non-existent directory (the directory would be created later in sage-sage
).
Probably we should use whatever solution comes from #5852.
Attachments (1)
Change History (23)
comment:1 Changed 11 years ago by
- Status changed from new to needs_review
comment:2 Changed 11 years ago by
- Status changed from needs_review to needs_work
comment:3 Changed 11 years ago by
- Cc leif added
comment:4 Changed 11 years ago by
- Description modified (diff)
comment:5 Changed 11 years ago by
- Description modified (diff)
comment:6 Changed 11 years ago by
- Dependencies set to #5852
comment:7 Changed 11 years ago by
- Status changed from needs_work to needs_review
comment:8 Changed 11 years ago by
- Dependencies #5852 deleted
comment:9 Changed 11 years ago by
comment:10 Changed 11 years ago by
- Dependencies set to #11924
comment:11 follow-up: ↓ 12 Changed 11 years ago by
comment:12 in reply to: ↑ 11 Changed 11 years ago by
Replying to jhpalmieri:
I wonder if we should put the
resolvelinks
script in a separate file in the root repo, and then call it from the top-level sage script (as in #5852) or from sage-env (for this ticket).
We cannot put it in a seperate file because $SAGE_ROOT/sage
would need to know SAGE_ROOT
in order to find this new separate file. So, in any case, $SAGE_ROOT/sage
needs the resolvelinks
script. So, we would have to source $SAGE_ROOT/sage
from sage-env
which doesn't sound like a very good idea (it could be done though).
comment:13 follow-up: ↓ 14 Changed 11 years ago by
Well, what's the purpose of canonicalizing SAGE_ROOT in the sage
script? It seems useful to have canonical names for paths, and it's important for some doctests (etc.), but do we actually need to do this for the script to function properly right at the start? If not, we could detect a preliminary value for SAGE_ROOT
first, find the resolvelinks
script, run it if it exists (e.g., check after seeing if sage-sage
exists and before exporting SAGE_ROOT
), and then find and export the permanent value for SAGE_ROOT
.
comment:14 in reply to: ↑ 13 Changed 11 years ago by
Replying to jhpalmieri:
Well, what's the purpose of canonicalizing SAGE_ROOT in the
sage
script?
It's precisely for the reason I said: we need to be able to determine a usable value of $SAGE_ROOT
before we can do anything with Sage.
It's true that it doesn't matter that it's canonical, but we still need to dereference symbolic links in $0
(the script filename) to find $SAGE_ROOT
. Since #5852, we support an installation where somebody makes a symbolic link from /usr/local/bin/sage
to /usr/local/sage/sage-4.8/sage
or whatever.
I agree the code duplication is not so nice, but I still think the patch can be reviewed as-is.
comment:15 Changed 11 years ago by
- Reviewers set to John Palmieri
- Status changed from needs_review to positive_review
comment:16 Changed 11 years ago by
- Status changed from positive_review to needs_work
Amazing, this causes
********************************************************************** File "/mnt/usb1/scratch/jdemeyer/merger/sage-4.8.alpha4-remove-base/devel/sage-main/sage/schemes/elliptic_curves/ell_rational_field.py", line 526: sage: E.conductor(algorithm="gp") Exception raised: Traceback (most recent call last): File "/mnt/usb1/scratch/jdemeyer/merger/sage-4.8.alpha4-remove-base/local/bin/ncadoctest.py", line 1231, in run_one_test self.run_one_example(test, example, filename, compileflags) File "/mnt/usb1/scratch/jdemeyer/merger/sage-4.8.alpha4-remove-base/local/bin/sagedoctest.py", line 38, in run_one_example OrigDocTestRunner.run_one_example(self, test, example, filename, compileflags) File "/mnt/usb1/scratch/jdemeyer/merger/sage-4.8.alpha4-remove-base/local/bin/ncadoctest.py", line 1172, in run_one_example compileflags, 1) in test.globs File "<doctest __main__.example_12[5]>", line 1, in <module> E.conductor(algorithm="gp")###line 526: sage: E.conductor(algorithm="gp") File "/mnt/usb1/scratch/jdemeyer/merger/sage-4.8.alpha4-remove-base/local/lib/python/site-packages/sage/schemes/elliptic_curves/ell_rational_field.py", line 552, in conductor self.__conductor_gp = Integer(gp.eval('ellglobalred(ellinit(%s,0))[1]'%list(self.a_invariants()))) File "integer.pyx", line 681, in sage.rings.integer.Integer.__init__ (sage/rings/integer.c:6786) TypeError: unable to convert x (=read("/mnt/usb1/scratch/jdemeyer/merger/sage-4.8.alpha4-remove-base/home/.sage/temp/sage.math.washington.edu/9425//interface//tmp11939") ) to an integer **********************************************************************
comment:17 Changed 11 years ago by
- Dependencies changed from #11924 to #11924, #12221
- Status changed from needs_work to positive_review
I keep positive review here, the latter problem is now #12221.
comment:18 Changed 11 years ago by
- Dependencies changed from #11924, #12221 to #11924
- Status changed from positive_review to needs_work
I changed the patch slightly to add the slash after $DOT_SAGE. I don't see how this could cause #12221, but it seems safer to keep the slash, just like it was before. Needs review please.
comment:19 Changed 11 years ago by
- Status changed from needs_work to needs_review
comment:20 Changed 11 years ago by
John, can you please review this new revised patch?
comment:21 Changed 11 years ago by
- Status changed from needs_review to positive_review
The only real change is to append the slash, right? It works for me in my testing.
comment:22 Changed 11 years ago by
- Merged in set to sage-4.8.alpha6
- Resolution set to fixed
- Status changed from positive_review to closed
Note also #11924: currently if
DOTSAGE
doesn't end with a slash, then there are unintended consequences. (#11924 should be an easy review.)