Opened 8 years ago

Closed 8 years ago

#6929 defect closed fixed (fixed)

setup3.py misspells and therefore doesn't install twisted.internet.utils (and doesn't notice)

Reported by: Itamar Turner-Trauring Owned by:
Priority: normal Milestone: Twisted-14.0.0
Component: core Keywords:
Cc: Branch: branches/setup3-missing-6929-2
branch-diff, diff-cov, branch-cov, buildbot
Author: itamarst, itamar

Description

twisted.internet.utils is never installed on Python 3 because setup3.py lists it as twisted.internet.util, and apparently doesn't notice it's non-existent when installing.

Change History (8)

comment:1 Changed 8 years ago by Itamar Turner-Trauring

Milestone: Python-3.xTwisted-14.0.0

Setting milestone to Twisted 14.0, since this is significant problem for Python 3.3 (e.g. I can't run tests for Crochet because of this off installed Twisted, breaking my Travis-CI setup).

comment:2 Changed 8 years ago by itamarst

Author: itamarst
Branch: branches/setup3-missing-6929

(In [41604]) Branching to 'setup3-missing-6929'

comment:3 Changed 8 years ago by Itamar Turner-Trauring

Author: itamarstitamar
Keywords: review added

http://buildbot.twistedmatrix.com/boxes-supported?branch=/branches/setup3-missing-6929

I manually tested that you can still do "python3.3 setup3.py install".

comment:4 Changed 8 years ago by Hynek Schlawack

Keywords: review removed
Owner: set to Itamar Turner-Trauring

Thanks for not giving up on Python 3. :)

  1. I’ve forced a fresh build and hand-verified that pip install . and bin/admin/build-tarballs works on Python 3.3.
  2. _dist3.py
    1. Lacks a test-case header.
    2. modulesToInstall’s docstring doesn’t really conform to our standards, it should have a general comment additionally to the @return statement. No idea though if this is relevant.
    3. Why is modulesToInstall a function in the first place? The name doesn’t sound like a function to me, please add a verb or make it just a module variable…
  3. run-python3-tests
    1. I’m not a fan of calling a file handler setup. Maybe setupFile?
  4. test_dist3.py
    1. “All modules listed in L{modulesToInstall} exist.” doesn’t make any sense if modulesToInstall is a function. Resolution depends on how you decide to approach 2.3.
    2. Are you sure about your encode('utf-8')s in test_exist? Shouldn’t either the filesystem’s encoding or ascii be used? Wouldn’t this fail on Windows if there’d be actually non-ascii chars in the paths?
    3. 43: please use format() in new code

Feel free to merge after addressing issues that seem relevant to you. 4.2 seems most important to me be shouldn’t be a reason to delay this ticket. I think I’d prefer to use ascii so the case is clear and the error messages unequivocal.

comment:5 Changed 8 years ago by Itamar Turner-Trauring

Keywords: review added
Owner: Itamar Turner-Trauring deleted

Addressed review comments, and made additional changes requiring more review:

  1. Prose docs now link to list of modules.
  2. Some changes to setup3.py to insure we always use *latest* version of modules to install list.

twistedchecker buildslave is red, but I feel that this is not a valid error.

comment:6 Changed 8 years ago by Hynek Schlawack

Keywords: review removed
  1. I have hand-verified the creation of release tarballs (we really need a buildbot for this :())
  2. When pip-installing a release tarball, I get:
     Running setup.py install for Twisted
       file twisted.py (for module twisted) not found
       file twisted/internet.py (for module twisted.internet) not found
       file twisted/internet/test.py (for module twisted.internet.test) not found
       file twisted/names.py (for module twisted.names) not found
       file twisted/names/test.py (for module twisted.names.test) not found
       file twisted/protocols.py (for module twisted.protocols) not found
       file twisted/protocols/test.py (for module twisted.protocols.test) not found
       file twisted/python.py (for module twisted.python) not found
       file twisted/python/test.py (for module twisted.python.test) not found
       file twisted/test.py (for module twisted.test) not found
       file twisted/trial.py (for module twisted.trial) not found
       file twisted/trial/test.py (for module twisted.trial.test) not found
       file twisted/web.py (for module twisted.web) not found
       file twisted/web/test.py (for module twisted.web.test) not found
       file twisted.py (for module twisted) not found
       file twisted/internet.py (for module twisted.internet) not found
       file twisted/internet/test.py (for module twisted.internet.test) not found
       file twisted/names.py (for module twisted.names) not found
       file twisted/names/test.py (for module twisted.names.test) not found
       file twisted/protocols.py (for module twisted.protocols) not found
       file twisted/protocols/test.py (for module twisted.protocols.test) not found
       file twisted/python.py (for module twisted.python) not found
       file twisted/python/test.py (for module twisted.python.test) not found
       file twisted/test.py (for module twisted.test) not found
       file twisted/trial.py (for module twisted.trial) not found
       file twisted/trial/test.py (for module twisted.trial.test) not found
       file twisted/web.py (for module twisted.web) not found
       file twisted/web/test.py (for module twisted.web.test) not found
    
       file twisted.py (for module twisted) not found
       file twisted/internet.py (for module twisted.internet) not found
       file twisted/internet/test.py (for module twisted.internet.test) not found
       file twisted/names.py (for module twisted.names) not found
       file twisted/names/test.py (for module twisted.names.test) not found
       file twisted/protocols.py (for module twisted.protocols) not found
       file twisted/protocols/test.py (for module twisted.protocols.test) not found
       file twisted/python.py (for module twisted.python) not found
       file twisted/python/test.py (for module twisted.python.test) not found
       file twisted/test.py (for module twisted.test) not found
       file twisted/trial.py (for module twisted.trial) not found
       file twisted/trial/test.py (for module twisted.trial.test) not found
       file twisted/web.py (for module twisted.web) not found
       file twisted/web/test.py (for module twisted.web.test) not found
       file twisted.py (for module twisted) not found
       file twisted/internet.py (for module twisted.internet) not found
       file twisted/internet/test.py (for module twisted.internet.test) not found
       file twisted/names.py (for module twisted.names) not found
       file twisted/names/test.py (for module twisted.names.test) not found
       file twisted/protocols.py (for module twisted.protocols) not found
       file twisted/protocols/test.py (for module twisted.protocols.test) not found
       file twisted/python.py (for module twisted.python) not found
       file twisted/python/test.py (for module twisted.python.test) not found
       file twisted/test.py (for module twisted.test) not found
       file twisted/trial.py (for module twisted.trial) not found
       file twisted/trial/test.py (for module twisted.trial.test) not found
       file twisted/web.py (for module twisted.web) not found
       file twisted/web/test.py (for module twisted.web.test) not found
    
    I guess that’s it trying to install packages as modules? TBH I can’t remember whether that happened before and I can import twisted after installation. It would be nice if we could avoid it though; it’s not the best impression to make.
  3. Yeah the twistedchecker warning is benign. You could avoid it by rephrasing it to “Tests for downloadPage aren't ported yet:” or something. Always nice to have less warnings around.
  4. I have to admit that I don’t know enough about import hacks to tell whether the related code is useful or not.
  5. Let’s call it just “Python 3” in topfiles; it should work with future Python 3s too.
  6. Making os.path cope with file system encodings is probably the best way forward. At least it’s not our fault if it explodes. :)

I can’t vouch for 4-related changes and I’m not sure about point 2. If that’s not a regression, please open a ticket for it and merge. Otherwise please fix and put out for review again. Thanks!

In closing (and not related to this ticket), couldn’t we get rid of setup3.py now? The code within could be easily put into setup.py, no?

comment:7 Changed 8 years ago by itamarst

Author: itamaritamarst, itamar
Branch: branches/setup3-missing-6929branches/setup3-missing-6929-2

(In [41836]) Branching to 'setup3-missing-6929-2'

comment:8 Changed 8 years ago by itamarst

Resolution: fixed
Status: newclosed

(In [41839]) Merge setup3-missing-6929-2. Python 3 module list now has tests to catch typos.

Author: itamar Review: hynek Fixes: #6929

Note: See TracTickets for help on using tickets.