Skip to content

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

Merged
merged 1 commit into from
Oct 26, 2016
Merged

BUG, TST: Fix python3-dbg bug in Travis script #8066

merged 1 commit into from
Oct 26, 2016

Conversation

gfyoung
Copy link
Contributor

@gfyoung gfyoung commented Sep 20, 2016

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. This was discovered in #6938.

@gfyoung
Copy link
Contributor Author

gfyoung commented Sep 21, 2016

@charris : Just a signpost for future discussion. Started in #8071 but should continue here.

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.
@gfyoung
Copy link
Contributor Author

gfyoung commented Oct 25, 2016

@charris : Any updates / comments on this one? This fix does seem straightforward IMHO.

@njsmith
Copy link
Member

njsmith commented Oct 25, 2016

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 before_install: directly into travis-test.sh, so we just have one script that does everything.

@gfyoung
Copy link
Contributor Author

gfyoung commented Oct 25, 2016

@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
Copy link
Member

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.

Copy link
Contributor Author

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

Copy link
Member

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.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Done.

@njsmith
Copy link
Member

njsmith commented Oct 25, 2016

Travis failed, not sure why.

@gfyoung
Copy link
Contributor Author

gfyoung commented Oct 25, 2016

@njsmith : I forgot to copy over the pip installs. 😄

@njsmith
Copy link
Member

njsmith commented Oct 25, 2016

Changes look good to me, but Travis is still failing. Maybe send a ping when you've got it passing?

@gfyoung
Copy link
Contributor Author

gfyoung commented Oct 25, 2016

@njsmith : Will do. In the meantime, do you think you could cancel #170595892? It's an old build. Thanks!

@gfyoung
Copy link
Contributor Author

gfyoung commented Oct 25, 2016

@njsmith : So the chroot build is failing because some of the commands from before-travis-install.sh cannot be run, as can be seen here. Unless we want to have at it with linux32, I am more inclined to keep these original commands separate. Thoughts?

@njsmith
Copy link
Member

njsmith commented Oct 25, 2016

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

set -x
# It's okay if these fail, and when we run in a chroot they do
uname -a
free -m
ulimit -a
# From here on, we want to know if anything fails
set -e
mkdir builds
...

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.

@gfyoung
Copy link
Contributor Author

gfyoung commented Oct 26, 2016

@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 chroot seems to mess everything up that before-travis-install.sh does.

@njsmith
Copy link
Member

njsmith commented Oct 26, 2016

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...

@njsmith njsmith merged commit 77ce5f4 into numpy:master Oct 26, 2016
@gfyoung gfyoung deleted the python3-dbg-patch branch October 26, 2016 01:16
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants