Opened 10 years ago

Closed 9 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:

Status badges

Description (last modified by jdemeyer)

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)

11704.patch (4.2 KB) - added by jdemeyer 9 years ago.
Patch to the SCRIPTS repository (local/bin)

Download all attachments as: .zip

Change History (23)

comment:1 Changed 10 years ago by jdemeyer

  • Status changed from new to needs_review

comment:2 Changed 10 years ago by jdemeyer

  • Status changed from needs_review to needs_work

comment:3 Changed 10 years ago by leif

  • Cc leif added

comment:4 Changed 10 years ago by jdemeyer

  • Description modified (diff)

comment:5 Changed 10 years ago by jdemeyer

  • Description modified (diff)

comment:6 Changed 10 years ago by jdemeyer

  • Dependencies set to #5852

comment:7 Changed 10 years ago by jdemeyer

  • Status changed from needs_work to needs_review

comment:8 Changed 10 years ago by jdemeyer

  • Dependencies #5852 deleted

comment:9 Changed 10 years ago by jhpalmieri

Note also #11924: currently if DOTSAGE doesn't end with a slash, then there are unintended consequences. (#11924 should be an easy review.)

comment:10 Changed 10 years ago by jdemeyer

  • Dependencies set to #11924

comment:11 follow-up: Changed 10 years ago by 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). As such, this seems related to #11073.

comment:12 in reply to: ↑ 11 Changed 10 years ago by jdemeyer

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

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 10 years ago by jdemeyer

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 10 years ago by jhpalmieri

  • Reviewers set to John Palmieri
  • Status changed from needs_review to positive_review

comment:16 Changed 9 years ago by jdemeyer

  • 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 9 years ago by jdemeyer

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

Changed 9 years ago by jdemeyer

Patch to the SCRIPTS repository (local/bin)

comment:18 Changed 9 years ago by jdemeyer

  • 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 9 years ago by jdemeyer

  • Status changed from needs_work to needs_review

comment:20 Changed 9 years ago by jdemeyer

John, can you please review this new revised patch?

comment:21 Changed 9 years ago by jhpalmieri

  • 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 9 years ago by jdemeyer

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