sagemath/sage

src/bin/sage-starts should be moved to build/bin

Closed this issue · 14 comments

src/bin/sage-starts uses build/bin/sage-logger (writing into $SAGE_ROOT/logs) and therefore belong to sage-the-distribution rather than sagelib. It should not be installed in $SAGE_LOCAL/bin.

(It could as well be eliminated and merged into build/make/deps (the only place from which it is called); this is where many invocations of build/bin/sage-logger happen and this one could as well, for consistency.)

Part of #21559. See also #21510.

CC: @jdemeyer @kiwifb @jhpalmieri @embray @dimpase

Component: build

Author: Matthias Koeppe

Branch/Commit: 23ffe1d

Reviewer: Dima Pasechnik

Issue created by migration from https://trac.sagemath.org/ticket/23769

comment:1

I agree. IIUC the only purpose of this file is to prevent make from building some targets if sage didn't build properly.

comment:2

Replying to @embray:

IIUC the only purpose of this file is to prevent make from building some targets if sage didn't build properly.

Not quite. I think the purpose is to do a very basic check that Sage actually works. I remember cases in the past where Sage seemed to build fine but doing ./sage would fail. These days, Sage is also used by the build system, so that is less likely. Still, it is good to leave that basic check in place.

Description changed:

--- 
+++ 
@@ -2,3 +2,4 @@
 
 (It could as well be eliminated and merged into `build/make/deps` (the only place from which it is called); this is where many invocations of `build/bin/sage-logger` happen and this one could as well, for consistency.)
 
+See also #21510.

Dependencies: #21559

New commits:

23ffe1dMove src/bin/sage-starts to build/bin/

Author: Matthias Koeppe

Commit: 23ffe1d

Changed dependencies from #21559 to none

Description changed:

--- 
+++ 
@@ -2,4 +2,4 @@
 
 (It could as well be eliminated and merged into `build/make/deps` (the only place from which it is called); this is where many invocations of `build/bin/sage-logger` happen and this one could as well, for consistency.)
 
-See also #21510.
+Part of #21559. See also #21510.
comment:9

lgtm

Reviewer: Dima Pasechnik

comment:10

Thanks!