Opened 4 years ago

Closed 19 months ago

#26116 closed enhancement (duplicate)

Simplify sage-starts logic

Reported by: saraedum Owned by:
Priority: major Milestone: sage-duplicate/invalid/wontfix
Component: build Keywords:
Cc: jdemeyer, arojas, gh-timokau, fbissey, slelievre Merged in:
Authors: Reviewers: François Bissey
Report Upstream: N/A Work issues:
Branch: u/saraedum/26116 (Commits, GitHub, GitLab) Commit: 9a48301769c040bbabb6399e3967ff5e5d3e6182
Dependencies: Stopgaps:

Status badges

Description (last modified by slelievre)

Distributions don't (and won't) install /etc/sage-started.txt. However, one doctest relies on the existence of SAGE_LOCAL/etc/sage-started.txt. So this fails everywhere/has to be patched away I guess.

I do not see why this stamp file needs any content at all. And so as I understand this the function _write_started_file() can go away and just be a touch. If somebody wonders what this file is about, they should just grep the source code and look at its timestamp.

Or am I missing something?

Change History (23)

comment:1 Changed 4 years ago by saraedum

Branch: u/saraedum/26116

comment:2 Changed 4 years ago by saraedum

Commit: 9a48301769c040bbabb6399e3967ff5e5d3e6182
Description: modified (diff)

New commits:

9a48301Simplify sage-starts logic

comment:3 Changed 4 years ago by saraedum

Cc: jdemeyer added

CCing the original author of this function.

comment:4 Changed 4 years ago by saraedum

Cc: arojas gh-timokau fbissey added

cc: the usual packaging suspects.

comment:5 Changed 4 years ago by fbissey

As I remember, Jeroen added that, when he was the release manager, has a way of proving that sage was properly built and functioning at the end of the building process.

Distro very much prefer to do it by running the test suite before the final install of course.

comment:6 Changed 4 years ago by saraedum

Status: newneeds_review

comment:7 Changed 4 years ago by gh-timokau

When is sage-starts executed? Will the removal of the doctest touching the file mean that I won't have to worry about it anymore? Currently I cheat by just doing touch <sage-prefix>/etc/sage-started.txt in the build script.

comment:8 Changed 4 years ago by slelievre

Cc: slelievre added
Description: modified (diff)

comment:9 Changed 4 years ago by saraedum

Thanks for fixing the typos.

sage-starts is executed when the sage-started.txt target needs to be rebuilt. This target is called by all-start and build-start and it depends on STANDARD_PACKAGE_INSTS. So, the idea in SageMath the distribution is that sage gets started after an SPKG has been installed so that the relocation scripts run (I guess.) The sage-started.txt file is not used anywhere else but in the Makefile as a timestamp for these *-start targets. For downstream distributions it has no meaning.

comment:10 Changed 4 years ago by gh-timokau

This change LGTM, but we should probably give Jeroen a chance to comment on this before positive_review'ing. An alternative would be to change the doctest of sage-starts to make it run without actually needing the /etc file.

comment:11 Changed 4 years ago by jdemeyer

It's not entirely clear which problem this ticket is supposed to fix.

If you're bothered by the doctest, I'm fine with just removing it.

comment:12 in reply to:  5 Changed 4 years ago by jdemeyer

Status: needs_reviewneeds_work

If you're going to make sage-starts less useful, why keep it? I'm fine with completely removing it. I don't think that there are compelling reasons to keep it.

comment:13 Changed 4 years ago by jdemeyer

Removing sage-starts would simplify the build system also.

comment:14 Changed 4 years ago by saraedum

jdemeyer: please see my comment:9 above. If what's written there is not true anymore and sage-starts is simply obsolete, I am happy to remove it completely.

comment:15 Changed 4 years ago by saraedum

Status: needs_workneeds_review

comment:16 Changed 4 years ago by jdemeyer

The "relocation script" sage-location is obsolete. I'd rather get rid of it completely, but it still exists for now.

comment:17 Changed 4 years ago by saraedum

I am not sure I understand what "obsolete" means here. More work would be necessary to get rid of sage-location/sage-starts, right? Or can we just take it out?

comment:18 in reply to:  17 Changed 4 years ago by fbissey

Replying to saraedum:

I am not sure I understand what "obsolete" means here. More work would be necessary to get rid of sage-location/sage-starts, right? Or can we just take it out?

Well sage-location was supposed to be run when you moved the sage tree. Basically sage doesn't support that anymore. I think the binaries do something else now. So sage-location has outlived its use and it had a host of issues about getting the right behavior on various platforms. So I guess that's why it is "obsolete", you shouldn't use it.

Going back to sage-starts I think it is perfectly fine to just remove the file and the doctest (unsurprisingly, I don't ship the file and I have a very similar looking patch for all.py - the only difference is a line saying # Don't do that on Gentoo.). I am fairly sure I whined to Jeroen when he introduced this. So it would make me quite happy if you just removed the file as well as the doctest. I cannot find sage-starts in the doc so nothing else should be removed.

comment:19 Changed 4 years ago by fbissey

Milestone: sage-8.4sage-8.5
Status: needs_reviewneeds_work

comment:20 Changed 4 years ago by fbissey

I looked at the doc but of course I forgot that it is run from the makefile. So build/make/deps needs touching as well if we are to remove sage-starts.

comment:21 Changed 20 months ago by mkoeppe

Authors: Julian Rüth
Milestone: sage-8.5sage-duplicate/invalid/wontfix
Status: needs_workneeds_review

sage-started.txt removed in #32036

comment:22 Changed 19 months ago by fbissey

Reviewers: François Bissey
Status: needs_reviewpositive_review

comment:23 Changed 19 months ago by chapoton

Resolution: duplicate
Status: positive_reviewclosed
Note: See TracTickets for help on using tickets.