Closed k3w1k0d3r closed 2 years ago
Branch: u/gh-k3w1k0d3r/pyfix
Could you please open a bug report with pyenv? It's pretty wild that they install a broken python command in PATH
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?
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
Yes, we can merge a workaround in Sage. But the bug is in pyenv
Branch pushed to git repo; I updated commit sha1. New commits:
4421ac2 | removed bash-only syntax |
great thank you
Branch pushed to git repo; I updated commit sha1. New commits:
50d9272 | += gave strange not found errors |
+paths=$(echo $SAGE_ORIG_PATH | tr ":" "\n")
+NEW_PATH=""
+for path in $paths
Loops like this are better done with IFS
Branch pushed to git repo; I updated commit sha1. New commits:
cd20870 | now works in dash shell |
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
Branch pushed to git repo; I updated commit sha1. New commits:
15cd6fe | loop is pure sh with IFS instead of using tr |
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.
it seems that the issue with pyenv is most likely a misconfiguration on my end of some sort
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 https://github.com/sagemath/sage-prod/issues/29285
+ * ) NEW_PATH="$NEW_PATH$path:";;
shouldn't the colon be in a different place?
I think ":$NEW_PATH$path" also works if that's what you mean.
I think the constructed path should neither begin nor end with :
because that would mean that the current directory is included.
okay I'll get rid of the extra :
Also could you make the pattern *pyenv*
a bit more specific?
Branch pushed to git repo; I updated commit sha1. New commits:
60a30a6 | changes to $PATH construction |
+ '/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
Branch pushed to git repo; I updated commit sha1. New commits:
ec49dd2 | less specific pyenv pattern |
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
+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.
Branch pushed to git repo; I updated commit sha1. New commits:
dedc9c2 | restore default IFS behavior |
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
The startup time test is safe to ignore. It is much too optimistic regarding the precision of its measurements.
Thanks for this contribution.
Reviewer: Matthias Koeppe
Changed branch from u/gh-k3w1k0d3r/pyfix to dedc9c2
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