Opened 7 years ago

Closed 7 years ago

#7876 defect closed fixed (fixed)

setup.py on Python 3 does not install data files that the tests need

Reported by: hawkowl Owned by: hawkowl
Priority: normal Milestone:
Component: core Keywords:
Cc: radix Branch: branches/externaltestfiles-py3-7876
branch-diff, diff-cov, branch-cov, buildbot
Author: hawkowl

Description (last modified by hawkowl)

setup.py install does not install all of the correct data files into a Python 3 install, despite them being included in the tarball. This should be fixed.

This makes the tests pass in a Py3 venv, of which I will make a builder for soon.

Change History (11)

comment:1 Changed 7 years ago by DefaultCC Plugin

Cc: radix added

comment:2 Changed 7 years ago by hawkowl

Author: hawkowl
Branch: branches/sdist-7876

(In [44619]) Branching to sdist-7876.

comment:3 Changed 7 years ago by Jean-Paul Calderone

Why?

comment:4 Changed 7 years ago by Jean-Paul Calderone

And what is "The Right Thing"?

comment:5 Changed 7 years ago by hawkowl

Description: modified (diff)
Summary: setup.py sdist should do The Right Thing™twisted.python._release should use setup.py sdist for generating tarballs

It turns out that some things that I thought it was not doing were actually the python 3 setup.py not installing files (rather than sdist not adding them) so rather than the ticket this would unblock, I'll make it the ticket that it should be.

comment:6 Changed 7 years ago by hawkowl

Branch: branches/sdist-7876branches/externaltestfiles-py3-7876

(In [44852]) Branching to externaltestfiles-py3-7876.

comment:7 Changed 7 years ago by hawkowl

Component: release managementcore
Description: modified (diff)
Summary: twisted.python._release should use setup.py sdist for generating tarballssetup.py on Python 3 does not install data files that the tests need
Type: enhancementdefect

Okay, so -- sdist generates a correct tarball, this is good. However, the missing piece isn't quite sdist, but that setup.py on Python 3 doesn't actually install all the files.

So this is now *not* sdist, but setup.py. (I am seemingly not very good at debugging).

comment:8 Changed 7 years ago by hawkowl

Keywords: review added

setup.py in this branch installs the required data files on Python 3. There's also direct tests for the parts that process the file list for distutils.

Builders spun, please review.

comment:9 Changed 7 years ago by Adi Roiban

Keywords: review removed
Owner: set to hawkowl

Changes look good.

Maybe we can start with adding the virtual env buildslave so that we can test these changes on a slave

I will be happy to review/help the changes required to create the new py3 venv builder


I am not happy with keeping another manual list of files for python3, but I think that this can be done in a separate ticket.

I think that the best practice is to not have these files at all, but rather include them in the test_.py module and write them into temporary files at test runtime.

Maybe we can put all test data files in the same folder and update setup.py to automatically copy at install time all files from that folder.

What do you say?

Thanks!

comment:10 Changed 7 years ago by hawkowl

I'm going to do a builder for this soon, so that this works and keeps working in the future.

The manual list is unfortunately I think the best solution right now -- eventually this will ~all go away~. Eventually.

Will merge.

comment:11 Changed 7 years ago by hawkowl

Resolution: fixed
Status: newclosed

(In [44887]) Merge externaltestfiles-py3-7876: setup.py now installs the data files that the tests need on Py3

Author: hawkowl Reviewer: adiroiban Fixes: #7876

Note: See TracTickets for help on using tickets.