Opened 7 years ago

Last modified 5 years ago

#13448 needs_work task

source sage-env in spkg/install

Reported by: ohanar Owned by: jason
Priority: major Milestone: sage-6.4
Component: misc Keywords:
Cc: jdemeyer, burcin, fbissey Merged in:
Authors: R. Andrew Ohana Reviewers:
Report Upstream: N/A Work issues:
Branch: Commit:
Dependencies: #13123 Stopgaps:

Description (last modified by ohanar)

Currently sage environmental variables are set in both sage-env, and a subset are set in spkg/install. This should be refactored. Also, since sage-spkg is now always called from within the sage environment (as it should be), so it is no longer necessary to source sage-env.

Installation instructions:

Apply attachment:trac13448.patch​ to the root repo.

Attachments (1)

trac13448.patch​ (2.7 KB) - added by ohanar 7 years ago.
apply to root repo

Download all attachments as: .zip

Change History (11)

comment:1 Changed 7 years ago by ohanar

  • Authors set to R. Andrew Ohana
  • Cc jdemeyer burcin fbissey added
  • Dependencies set to #13123
  • Description modified (diff)
  • Status changed from new to needs_review

The dependency is only because I don't want to rebase #13123 on this.

Changed 7 years ago by ohanar

apply to root repo

comment:2 Changed 7 years ago by jdemeyer

This is going to break badly because sage-env is sourced only once (in this case, at the very beginning of the installation) but it needs to be re-sourced at various times during the installation because of environment variables which are conditionally set. This can be seen during upgrading for example, where sage-env is sourced by sage --upgrade, requiring various ugly work-arounds. To support upgrading, spkg/install needs to keep working with an old sage-env.

I agree there is a little bit of duplication between spkg/install and sage-env but I don't see a clean way to fix this. I'm open for suggestions though...

I propose to postpone this ticket, perhaps at a later time refactoring will be easier.

comment:3 follow-up: Changed 7 years ago by ohanar

Hmm, you are right. I'll see if I can't come up with a clean way around that, but for now we should postpone this (not really sure the best way to mark that on trac though...)

comment:4 in reply to: ↑ 3 Changed 7 years ago by jdemeyer

  • Milestone changed from sage-5.4 to sage-6.0

Replying to ohanar:

not really sure the best way to mark that on trac though...

Like this?

comment:5 Changed 7 years ago by jdemeyer

  • Status changed from needs_review to needs_work

comment:6 Changed 7 years ago by kini

Typo on line 95 of sage-spkg.

comment:7 Changed 6 years ago by vbraun_spam

  • Milestone changed from sage-6.0 to sage-6.1

comment:8 Changed 6 years ago by vbraun_spam

  • Milestone changed from sage-6.1 to sage-6.2

comment:9 Changed 6 years ago by vbraun_spam

  • Milestone changed from sage-6.2 to sage-6.3

comment:10 Changed 5 years ago by vbraun_spam

  • Milestone changed from sage-6.3 to sage-6.4
Note: See TracTickets for help on using tickets.