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: |
Description (last modified by )
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
Branch: | → u/saraedum/26116 |
---|
comment:2 Changed 4 years ago by
Commit: | → 9a48301769c040bbabb6399e3967ff5e5d3e6182 |
---|---|
Description: | modified (diff) |
comment:4 Changed 4 years ago by
Cc: | arojas gh-timokau fbissey added |
---|
cc: the usual packaging suspects.
comment:5 follow-up: 12 Changed 4 years ago by
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
Status: | new → needs_review |
---|
comment:7 Changed 4 years ago by
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
Cc: | slelievre added |
---|---|
Description: | modified (diff) |
comment:9 Changed 4 years ago by
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
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
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 Changed 4 years ago by
Status: | needs_review → needs_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:14 Changed 4 years ago by
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
Status: | needs_work → needs_review |
---|
comment:16 Changed 4 years ago by
The "relocation script" sage-location
is obsolete. I'd rather get rid of it completely, but it still exists for now.
comment:17 follow-up: 18 Changed 4 years ago by
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 Changed 4 years ago by
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
Milestone: | sage-8.4 → sage-8.5 |
---|---|
Status: | needs_review → needs_work |
comment:20 Changed 4 years ago by
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
Authors: | Julian Rüth |
---|---|
Milestone: | sage-8.5 → sage-duplicate/invalid/wontfix |
Status: | needs_work → needs_review |
sage-started.txt
removed in #32036
comment:22 Changed 19 months ago by
Reviewers: | → François Bissey |
---|---|
Status: | needs_review → positive_review |
comment:23 Changed 19 months ago by
Resolution: | → duplicate |
---|---|
Status: | positive_review → closed |
New commits:
Simplify sage-starts logic