-
-
Notifications
You must be signed in to change notification settings - Fork 10.8k
BUG, TST: Fix python3-dbg bug in Travis script #8066
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Conversation
With USE_DEBUG=1, the wrong python was being used to create the virtualenv, meaning that installed packages (e.g. Cython) were being installed to the wrong location.
@charris : Any updates / comments on this one? This fix does seem straightforward IMHO. |
The actual change makes sense to me. I found the refactoring a bit confusing, though, because of how context is/isn't propagated between different shell scripts -- so e.g. there's this pushd/popd stuff copied into the new before-install script, but it isn't really needed, because working directory changes don't propagate beyond single script, and then there's the need to remember to re-activate the virtualenv each time. I think it would be simplest to just merge the stuff that's currently in |
@njsmith : Makes sense to me. I made a separate commit to test what happens on Travis. |
else | ||
virtualenv --python=python builds/venv | ||
fi | ||
|
||
set -ex |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
set -ex
needs to be the first thing in the file.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Done.
@@ -11,15 +33,12 @@ if [ -r /usr/lib/libeatmydata/libeatmydata.so ]; then | |||
export LD_PRELOAD=/usr/lib/libeatmydata/libeatmydata.so | |||
fi | |||
|
|||
source builds/venv/bin/activate | |||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Let's put this right next to where we create the venv.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Done.
Travis failed, not sure why. |
@njsmith : I forgot to copy over the pip installs. 😄 |
Changes look good to me, but Travis is still failing. Maybe send a ping when you've got it passing? |
@njsmith : Will do. In the meantime, do you think you could cancel #170595892? It's an old build. Thanks! |
That failure can be fixed by just telling the shell that we're okay with the information gathering steps at the beginning failing... something like
A bigger question is whether it's useful to be running the virtualenv stuff inside a chroot. Potentially the answer is yes, this is actually better... but the chroot handling is confusing enough that I'm not 100% sure :-/. I guess we could make the simple change above and see what happens. |
@njsmith : Didn't work either. TBH, I would rather push the patch in first, as I don't see any easy way right now to integrate the two together since the |
Ugh. Well, I guess this is so confusing that we're not making it any worse. (What could possibly go wrong.) Thanks for sticking with it... |
With USE_DEBUG=1, the wrong python was being used to create the
virtualenv
, meaning that installedpackages (e.g.
Cython
) were being installed to the wrong location. This was discovered in #6938.