Remove sage-location's "sage-force-relocate" mechanism, fix script to work without SAGE_ROOT

[mkoeppe]

#21783 removes the last productive bit of sage-location. The only thing that sage-location does now is write a warning if it notices that the tree has moved.

We want to get rid of the whole mechanism completely.... (or reuse its remains to implement #31076).

In this ticket, we remove a part of it, the "sage-force-relocate" mechanism, which used to be invoked by sage-spkg.

We also fix the script so that it works in installations without SAGE_ROOT. This is for the benefit of downstream packaging and #30913.

(When we finally get rid of sage-location, we can finally close tickets #15146, #17479, #11755.)

This is also preparation of sorts for #31076, which proposes to add a different relocation mechanism.

Depends on #21783

CC: @jhpalmieri @culler @dimpase @kiwifb @antonio-rojas @mwageringel

Component: relocation

Author: Matthias Koeppe, John Palmieri

Branch/Commit: 3035932

Reviewer: John Palmieri, Matthias Koeppe

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

[mkoeppe]

Branch: u/mkoeppe/remove-sage-location

[sagetrac-git]

Commit: 287dc70

[sagetrac-git]

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

287dc70

src/doc/it/faq/faq-usage.rst: rimuovi gli antichi consigli relativi alla posizione della salvia

[mkoeppe]

Author: Matthias Koeppe

[sagetrac-git]

Changed commit from 287dc70 to b0e4816

[sagetrac-git]

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

1dfc920	src/bin/sage-location: Do not fail if SAGE_ROOT is not set
b0e4816	src/bin/sage-location: Do not output 2 lines announcing a no-op

[jhpalmieri]

comment:7

• I don't know how to test whether this will "fix script to work without SAGE_ROOT". Maybe the distro people have to look at that part.

• The script's main purpose is to see if Sage has been relocated and then to quit with an error message if so. Do we need to do this? Can we just document that it can't be moved and let it fail however if fails if someone ignores this? That is, can we delete this entirely?

[mkoeppe]

comment:8

Replying to @jhpalmieri:

• I don't know how to test whether this will "fix script to work without SAGE_ROOT". Maybe the distro people have to look at that part.

Yes, I hope this ticket can reduce the amount of downstream patching

[jhpalmieri]

comment:9

Also: would the check

    if OLD_SAGE_ROOT != SAGE_ROOT:

be more robust as

    if not os.path.samefile(OLD_SAGE_ROOT, SAGE_ROOT):

or SAGE_ROOT = os.pathlib.Path('...') followed by SAGE_ROOT.samefile(...) or something similar? Maybe we have to handle None as a special case.

Edit: typo: should be samefile, not samepath.

[mkoeppe]

comment:10

Replying to @jhpalmieri:

• The script's main purpose is to see if Sage has been relocated and then to quit with an error message if so. Do we need to do this? Can we just document that it can't be moved and let it fail however if fails if someone ignores this? That is, can we delete this entirely?

I think there's value in giving a clear error message - removing it could cause more traffic on sage-support

[mkoeppe]

comment:11

Replying to @jhpalmieri:

Also: would the check

    if OLD_SAGE_ROOT != SAGE_ROOT:

be more robust as

    if not os.path.samepath(OLD_SAGE_ROOT, SAGE_ROOT):

or SAGE_ROOT = os.pathlib.Path('...') followed by SAGE_ROOT.samefile(...) or something similar? Maybe we have to handle None as a special case.

Yes, this sounds like a good idea.

[mkoeppe]

comment:12

But in fact to make this code really useful, it should be revised to work with SAGE_LOCAL instead of SAGE_ROOT (see discussion in #30913). So on the present ticket, I wouldn't want to make such improvements

[jhpalmieri]

comment:13

I'm not sure what you mean by that, but here is how I would change the file (without worrying about SAGE_ROOT vs. SAGE_LOCAL):

diff --git a/src/bin/sage-location b/src/bin/sage-location
index f656dd8f41..e83b06e409 100755
--- a/src/bin/sage-location
+++ b/src/bin/sage-location
@@ -97,15 +97,9 @@ if __name__ == '__main__':
     # OLD_SAGE_ROOT is None if this is a first-time install.
     OLD_SAGE_ROOT = read_location_file()
 
-    if OLD_SAGE_ROOT != SAGE_ROOT:
-        if OLD_SAGE_ROOT is None:
-            print("This looks like the first time you are running Sage.")
-        elif OLD_SAGE_ROOT != SAGE_ROOT:
-            print(RELOCATION_ERROR.format(OLD_SAGE_ROOT=OLD_SAGE_ROOT, SAGE_ROOT=SAGE_ROOT))
-            sys.exit(1)
-        else:
-            print("The Sage installation tree has moved")
-            print("from %s" % OLD_SAGE_ROOT)
-            print("  to %s" % SAGE_ROOT)
-            assert(False)
+    if OLD_SAGE_ROOT is None:
+        print("This looks like the first time you are running Sage.")
         sage_relocate()
+    elif not os.path.samefile(OLD_SAGE_ROOT, SAGE_ROOT):
+        print(RELOCATION_ERROR.format(OLD_SAGE_ROOT=OLD_SAGE_ROOT, SAGE_ROOT=SAGE_ROOT))
+        sys.exit(1)

[mkoeppe]

comment:14

This change would be fine with me. In fact, the message "This looks like the first time you are running Sage." can probably be removed. Please feel free to push to the ticket

[jhpalmieri]

Changed branch from u/mkoeppe/remove-sage-location to u/jhpalmieri/remove-sage-location

[jhpalmieri]

Changed commit from b0e4816 to 3035932

[jhpalmieri]

comment:16

Done.

---

New commits:

3035932

trac 31270: use os.path.samefile to compare paths.

[mkoeppe]

Changed author from Matthias Koeppe to Matthias Koeppe, John Palmieri

[mkoeppe]

comment:17

Positive review for your changes.

[mkoeppe]

Reviewer: John Palmieri, Matthias Koeppe

[jhpalmieri]

comment:18

I'm happy with it, too, but as I said, I can't review the parts that claim to work when SAGE_ROOT is not defined.

[mkoeppe]

Changed reviewer from John Palmieri, Matthias Koeppe to John Palmieri, Matthias Koeppe, ...

[kiwifb]

comment:20

Well, sage-on-gentoo and probably other distros don't bother installing sage-location.

But it looks safe to run on a distro installed sage since we exit straight away on SAGE_ROOT being undefined.

[jhpalmieri]

Changed reviewer from John Palmieri, Matthias Koeppe, ... to John Palmieri, Matthias Koeppe

[jhpalmieri]

comment:21

I don't speak Italian (despite the last name), but I think we can merge this.

[mkoeppe]

comment:22

Thanks!

[vbraun]

Changed branch from u/jhpalmieri/remove-sage-location to 3035932