sage-bootstrap-python doesn't work when pyenv shadows all usable pythons in PATH

[k3w1k0d3r]

The script sage-bootstrap-python attempts to call a command like
"/path/to/python" -c "..."
Pyenv python treats "python" differently from just python, giving the output:
/home/.../.pyenv/shims/python: line 1: 2: command not found

With pyenv installed and configured normally, the script will never check a non-pyenv version of python, and thus will fail to find a suitable python. My fix was to attempt to remove pyenv from the PATH before testing for python.

I've only confirmed this behavior on arch linux

Component: build: configure

Keywords: bootstrap, python, pyenv

Author: Julien Grijalva

Branch/Commit: dedc9c2

Reviewer: Matthias Koeppe

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

[k3w1k0d3r]

Branch: u/gh-k3w1k0d3r/pyfix

[k3w1k0d3r]

Commit: ab156dd

[k3w1k0d3r]

New commits:

ab156dd

Small fix to sage-bootstrap-python

[mkoeppe]

comment:5

Could you please open a bug report with pyenv? It's pretty wild that they install a broken python command in PATH

[k3w1k0d3r]

comment:6

yeah sure

do you think I should give up on this ticket or should I still work on it to make the build not fail?

[k3w1k0d3r]

comment:7

well I'll fix it because it's not too bad, I just used bash-only syntax and for some reason when I tested it locally it still worked

[mkoeppe]

comment:8

Yes, we can merge a workaround in Sage. But the bug is in pyenv

[sagetrac-git]

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

4421ac2

removed bash-only syntax

[sagetrac-git]

Changed commit from ab156dd to 4421ac2

[k3w1k0d3r]

comment:10

great thank you

[sagetrac-git]

Changed commit from 4421ac2 to 50d9272

[sagetrac-git]

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

50d9272

+= gave strange not found errors

[mkoeppe]

comment:13

+paths=$(echo $SAGE_ORIG_PATH | tr ":" "\n")
+NEW_PATH=""
+for path in $paths

Loops like this are better done with IFS

[sagetrac-git]

Changed commit from 50d9272 to cd20870

[sagetrac-git]

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

cd20870

now works in dash shell

[k3w1k0d3r]

comment:15

Replying to @mkoeppe:

+paths=$(echo $SAGE_ORIG_PATH | tr ":" "\n")
+NEW_PATH=""
+for path in $paths

Loops like this are better done with IFS

alright I'll change it thanks

[sagetrac-git]

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

15cd6fe

loop is pure sh with IFS instead of using tr

[sagetrac-git]

Changed commit from cd20870 to 15cd6fe

[k3w1k0d3r]

comment:17

I realized my system sh was a symlink to bash instead of dash, which is why my local tests worked. I've now properly tested with dash, so hopefully now everything works.

[k3w1k0d3r]

comment:18

pyenv/pyenv#2378

[k3w1k0d3r]

comment:19

it seems that the issue with pyenv is most likely a misconfiguration on my end of some sort

[k3w1k0d3r]

comment:20

yeah the quotes issue is fixed on my system, but the old script still doesn't work for me. I assume it's because of #29285

[mkoeppe]

comment:22

+        *      ) NEW_PATH="$NEW_PATH$path:";;

shouldn't the colon be in a different place?

[k3w1k0d3r]

comment:23

I think ":$NEW_PATH$path" also works if that's what you mean.

[mkoeppe]

comment:24

I think the constructed path should neither begin nor end with : because that would mean that the current directory is included.

[k3w1k0d3r]

comment:25

okay I'll get rid of the extra :

[mkoeppe]

comment:26

Also could you make the pattern *pyenv* a bit more specific?

[sagetrac-git]

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

60a30a6

changes to $PATH construction

[sagetrac-git]

Changed commit from 15cd6fe to 60a30a6

[mkoeppe]

comment:28

+        '/home/'*'/.pyenv/shims'*);;

Well that's now a bit too specific. For example, on macOS, user home directories live in /Users. By the way, words don't need to be quoted in these patterns

[sagetrac-git]

Changed commit from 60a30a6 to ec49dd2

[sagetrac-git]

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

ec49dd2

less specific pyenv pattern

[k3w1k0d3r]

comment:30

Replying to @mkoeppe:

+        '/home/'*'/.pyenv/shims'*);;

Well that's now a bit too specific. For example, on macOS, user home directories live in /Users. By the way, words don't need to be quoted in these patterns

yeah I didn't think that through very well

[mkoeppe]

comment:31

+IFS=' '

It doesn't matter very much here, but note that this is not the default value of IFS.
It would be more idiomatic to either save the old IFS in a variable and then restore it, or to unset IFS to restore the default behavior of the shell.

[sagetrac-git]

Changed commit from ec49dd2 to dedc9c2

[sagetrac-git]

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

dedc9c2

restore default IFS behavior

[k3w1k0d3r]

comment:33

it seems odd that this failed a startup time test right? it doesn't seem like this script ever gets executed after it's already been configured

[mkoeppe]

comment:34

The startup time test is safe to ignore. It is much too optimistic regarding the precision of its measurements.

[mkoeppe]

comment:35

Thanks for this contribution.

[mkoeppe]

Reviewer: Matthias Koeppe

[vbraun]

Changed branch from u/gh-k3w1k0d3r/pyfix to dedc9c2