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

Testing with `--installpkg somewheel.whl` broken (Regression in 3.5.0) #1042

Closed
suutari opened this Issue Oct 8, 2018 · 11 comments

Comments

Projects
None yet
4 participants
@suutari

suutari commented Oct 8, 2018

Tox 3.5.0 seems to break my Travis tests for Prequ. I have set the Travis configuration so that it runs python setup.py bdist_wheel and then runs as tox -v --installpkg dist/*.whl. This worked fine with earlier Tox versions (<=3.4.0).

With Tox 3.5.0 my Travis job started to break with an exception raised from this line:

for i in temp_dir.listdir(fil="*-{}".format(package.basename))

The exception was AttributeError: 'str' object has no attribute 'basename'. Full trace back is available in the build log: https://travis-ci.org/suutari/prequ/jobs/438550460

I think the error happens because acquire_package returns a bare str if --instalpkg is used, but otherwise it seems to return a _path.local.LocalPath instance. The failing code in create_session_view seems to assume (since commit 6538f74) that the package is a a rich Path object rather than bare string.

@gaborbernat

This comment has been minimized.

Member

gaborbernat commented Oct 8, 2018

@suutari thanks for pointing this out I'll fix it 👍

suutari-ai pushed a commit to suutari/prequ that referenced this issue Oct 8, 2018

Travis,AppVeyour: Pin Tox version to fix the build
It seems that Tox 3.5.0 breaks the `--installpkg` option, so pin Tox
version to 3.4.0 to fix the tests and to avoid such surprises caused by
a new relesease for Tox in the future.

See the relavant Tox issue and the failing Travis build log:
tox-dev/tox#1042
https://travis-ci.org/suutari/prequ/jobs/438550460

gaborbernat added a commit that referenced this issue Oct 8, 2018

@helpr helpr bot added the pr-available label Oct 8, 2018

gaborbernat added a commit that referenced this issue Oct 8, 2018

gaborbernat added a commit that referenced this issue Oct 8, 2018

gaborbernat added a commit that referenced this issue Oct 8, 2018

gaborbernat added a commit that referenced this issue Oct 8, 2018

@helpr helpr bot added pr-merged and removed pr-available labels Oct 8, 2018

@anthrotype

This comment has been minimized.

anthrotype commented Oct 8, 2018

@gaborbernat thanks for the quick fix!

@gaborbernat

This comment has been minimized.

Member

gaborbernat commented Oct 8, 2018

@suutari https://pypi.org/project/tox/3.5.1/ has been released and should address the issue; thanks for the report and let us know if you bump into any other issues!

PyPI
virtualenv-based automation of test activities
@suutari

This comment has been minimized.

suutari commented Oct 8, 2018

@gaborbernat Thanks for the fix!

However, even Tox 3.5.1 doesn't work with my use case yet (as 3.4.0 used to). See https://travis-ci.org/suutari/prequ/jobs/438805462

I guess the problem is that the wheel file is renamed to 1-something.whl, but Pip won't install from a wheel file with that name, since the name of the package doesn't have this 1- prefix. What's the point of this prefix? Can it be removed? Or maybe a directory could be used instead, e.g. 1/something.whl, so that the wheel file name could be in correct format.

@gaborbernat gaborbernat reopened this Oct 8, 2018

@gaborbernat

This comment has been minimized.

Member

gaborbernat commented Oct 8, 2018

https://tox.readthedocs.io/en/latest/changelog.html#features details the goal of adding the hard link. We'll need to change to folder method though indeed, will try to do that tomorrow. If you're available feel free to create a PR doing that.

@ftartaggia

This comment has been minimized.

ftartaggia commented Oct 9, 2018

Please note that the new hard linking approach (package.py:72) breakes if source path and destination path are on different filesystems.

Instead of checking if hasattr(os, 'link'), what about trying to os.link and falling back to copying if an Exception is raised?

@gaborbernat

This comment has been minimized.

Member

gaborbernat commented Oct 9, 2018

In theory, both folders should be within .tox so a very edge case for them to be on a different file system. However, we can be safe by doing so.

@ftartaggia

This comment has been minimized.

ftartaggia commented Oct 9, 2018

Unfortunately it's my use case, since I'm building with Jenkins and then running tox inside a docker container with --installpkg AND --workdir /tmp/...

@gaborbernat

This comment has been minimized.

Member

gaborbernat commented Oct 9, 2018

Done via 46f4e84

@gaborbernat gaborbernat added this to the 3.5 milestone Oct 9, 2018

@helpr helpr bot removed the pr-available label Oct 9, 2018

@suutari

This comment has been minimized.

suutari commented Oct 10, 2018

Tox 3.5.2 seems to fix also the second issue with wheels (which was affecting my testing of Prequ). Thanks @gaborbernat!

@gaborbernat

This comment has been minimized.

Member

gaborbernat commented Oct 10, 2018

Great to hear, you're welcome.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment