Skip to content

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

Merged
njsmith merged 1 commit into
numpy:masterfrom
gfyoung:python3-dbg-patch
Oct 26, 2016
Merged

njsmith merged 1 commit into
numpy:masterfrom
gfyoung:python3-dbg-patch

Conversation

@gfyoung

@gfyoung gfyoung commented Sep 20, 2016 •

Copy link
Copy Markdown
Contributor

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

gfyoung commented Sep 21, 2016

Copy link
Copy Markdown
Contributor Author

@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

gfyoung commented Oct 25, 2016

Copy link
Copy Markdown
Contributor Author

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

@njsmith

njsmith commented Oct 25, 2016

Copy link
Copy Markdown
Member

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

gfyoung commented Oct 25, 2016

Copy link
Copy Markdown
Contributor Author

@njsmith : Makes sense to me. I made a separate commit to test what happens on Travis.

Comment thread tools/travis-test.sh Outdated

Copy link
Copy Markdown
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
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Done.

Comment thread tools/travis-test.sh Outdated

Copy link
Copy Markdown
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
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Done.

@njsmith

njsmith commented Oct 25, 2016

Copy link
Copy Markdown
Member

Travis failed, not sure why.

@gfyoung

gfyoung commented Oct 25, 2016

Copy link
Copy Markdown
Contributor Author

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

@njsmith

njsmith commented Oct 25, 2016

Copy link
Copy Markdown
Member

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

@gfyoung

gfyoung commented Oct 25, 2016

Copy link
Copy Markdown
Contributor Author

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

@gfyoung

gfyoung commented Oct 25, 2016 •

Copy link
Copy Markdown
Contributor Author

@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

njsmith commented Oct 25, 2016

Copy link
Copy Markdown
Member

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

gfyoung commented Oct 26, 2016

Copy link
Copy Markdown
Contributor Author

@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

njsmith commented Oct 26, 2016

Copy link
Copy Markdown
Member

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