Opened 4 months ago

Closed 3 months ago

#28320 closed enhancement (fixed)

Further fixes for sage-env-less installs

Reported by: arojas Owned by:
Priority: major Milestone: sage-8.9
Component: distribution Keywords:
Cc: fbissey, jhpalmieri Merged in:
Authors: Antonio Rojas, François Bissey Reviewers: John Palmieri
Report Upstream: N/A Work issues:
Branch: 638a33b (Commits) Commit: 638a33b7f1bf871f7706dbde59734be744134f16
Dependencies: Stopgaps:

Description (last modified by arojas)

Follow-up for #28225

  • Set SAGE_DOC as fallback for SAGE_DOC_SRC when sage-env is not available
  • Fix sage-cython on python3

Change History (14)

comment:1 Changed 4 months ago by arojas

  • Branch set to u/arojas/further_fixes_for_sage_env_less_installs

comment:2 Changed 4 months ago by arojas

  • Commit set to ac8cf16c5c576d8124afd29e0eacf6797b67c056
  • Component changed from PLEASE CHANGE to distribution
  • Description modified (diff)
  • Type changed from PLEASE CHANGE to enhancement

comment:3 Changed 4 months ago by git

  • Commit changed from ac8cf16c5c576d8124afd29e0eacf6797b67c056 to ea0b5e783f2a8863c61a8e5fa740c83c5ee40b90

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

ea0b5e7Use sage-python23 in sage-cython shebang to make is work with python 3

comment:4 Changed 4 months ago by git

  • Commit changed from ea0b5e783f2a8863c61a8e5fa740c83c5ee40b90 to 5f50b05466ac3e48904a17009ad6d23ac5626a60

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

5f50b05Set SAGE_DOC as fallback for SAGE_DOC_SRC and make sure it is used when SAGE_ROOT is not defined

comment:5 Changed 4 months ago by arojas

  • Cc fbissey added
  • Description modified (diff)

François, feel free to take over the branch and add any further fixes. I'll be on vacation with no computer access for the next 3 weeks.

comment:6 Changed 4 months ago by fbissey

Have a nice one then!

comment:7 follow-up: Changed 4 months ago by fbissey

  • Branch changed from u/arojas/further_fixes_for_sage_env_less_installs to u/fbissey/further_fixes_for_sage_env_less_installs
  • Cc jhpalmieri added
  • Commit changed from 5f50b05466ac3e48904a17009ad6d23ac5626a60 to 638a33b7f1bf871f7706dbde59734be744134f16
  • Status changed from new to needs_review

While it may be overkill, does that fix your python3 problem, John? If not I move to one of the fix you suggested. Lastly, since it is deprecated, there is the option of just removing sage-cython.


New commits:

638a33bRemove some useless os.environ in favor of sage.env

comment:8 in reply to: ↑ 7 Changed 4 months ago by jhpalmieri

Replying to fbissey:

While it may be overkill, does that fix your python3 problem, John? If not I move to one of the fix you suggested. Lastly, since it is deprecated, there is the option of just removing sage-cython.

This does fix it. While it's deprecated, I'm not sure what the plans are: maybe to make sage --cython just call cython directly? Better to ask the people involved with the deprecation (#27041). I don't think we can just delete it, since sage --cython uses it right now.

comment:9 Changed 4 months ago by fbissey

Merged 6 months ago, so it is probably too early for removal anyway. I think getting it to run with whichever python sage has been built with, is the right thing to do.

If you don't have any objections or problem with the rest of the branch, could I ask for a review :)

comment:10 Changed 4 months ago by jhpalmieri

My general feeling is that we should use python rather than sage-python23 whenever possible, so I'm not delighted with this change. It is possible that someone could use the sage-cython script before Sage has been completely installed, but not with this change.

However, since the whole thing is deprecated, I'm not going to worry too much about it. Sage builds with this change, so it doesn't break anything in the build process. The other changes look safe enough. Let me run a few more tests, but it's probably ready to go.

comment:11 Changed 4 months ago by jhpalmieri

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

Okay, it builds from scratch with Python 2 and Python 3, with only the expected failures with Python 3.

comment:12 Changed 3 months ago by vbraun

  • Status changed from positive_review to needs_work

Author is missing

comment:13 Changed 3 months ago by fbissey

  • Authors set to Antonio Rojas, François Bissey
  • Status changed from needs_work to positive_review

Fixed.

comment:14 Changed 3 months ago by vbraun

  • Branch changed from u/fbissey/further_fixes_for_sage_env_less_installs to 638a33b7f1bf871f7706dbde59734be744134f16
  • Resolution set to fixed
  • Status changed from positive_review to closed
Note: See TracTickets for help on using tickets.