Skip to content
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 · Fixed by #1046
Closed

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

suutari opened this issue Oct 8, 2018 · 11 comments · Fixed by #1046
Labels
bug:normal affects many people or has quite an impact pr-merged
Milestone

Comments

@suutari
Copy link

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

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

@gaborbernat gaborbernat added the bug:normal affects many people or has quite an impact label Oct 8, 2018
suutari-ai pushed a commit to suutari/prequ that referenced this issue Oct 8, 2018
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
@helpr helpr bot added the pr-available label 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
Copy link

@gaborbernat thanks for the quick fix!

@gaborbernat
Copy link
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
Copy link
Author

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

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

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

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

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

Great to hear, you're welcome.

@tox-dev tox-dev locked and limited conversation to collaborators Jan 14, 2021
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
bug:normal affects many people or has quite an impact pr-merged
Projects
None yet
Development

Successfully merging a pull request may close this issue.

4 participants