Opened 15 years ago

Closed 15 years ago

#1952 enhancement closed fixed (fixed)

twisted.python.zippath is inconsistent in its use of path separators

Reported by: Glyph Owned by:
Priority: highest Milestone:
Component: core Keywords:
Cc: Branch:
Author:

Description

It seems like python's zipimport uses os.sep to separate internal zip paths, so twisted probably should too (except in the places where you HAVE to use "/", because that's what PKZIP uses...)

Buildbot error is here: http://twistedmatrix.com/buildbot/win32-scmikes-select/builds/55/step-default/2

Change History (6)

comment:1 Changed 15 years ago by Jean-Paul Calderone

Buildbot logs are not persistent things.

Here's the output:

[FAIL]: twisted.test.test_paths.ZipFilePathTestCase.test_walk

Traceback (most recent call last):
  File "e:\twistedbuildbot\W32-full2.4-scmikes-select\Twisted\twisted\test\test_paths.py", line 64, in test_walk
    self.assertEquals(x, self.all)
twisted.trial.unittest.FailTest: ['e:\\twistedbuildbot\\W32-full2.4-scmikes-select\\Twisted\\_trial_temp\\twisted.test.test_paths\\ZipFilePathTestCase\\test_walk\\bxkcny\\temp.zip', 'e:\\twistedbuildbot\\W32-full2.4-scmikes-select\\Twisted\\_trial_temp\\twisted.test.test_paths\\ZipFilePathTestCase\\test_walk\\bxkcny\\temp.zip/file1', 'e:\\twistedbuildbot\\W32-full2.4-scmikes-select\\Twisted\\_trial_temp\\twisted.test.test_paths\\ZipFilePathTestCase\\test_walk\\bxkcny\\temp.zip/sub1', 'e:\\twistedbuildbot\\W32-full2.4-scmikes-select\\Twisted\\_trial_temp\\twisted.test.test_paths\\ZipFilePathTestCase\\test_walk\\bxkcny\\temp.zip/sub1/file2', 'e:\\twistedbuildbot\\W32-full2.4-scmikes-select\\Twisted\\_trial_temp\\twisted.test.test_paths\\ZipFilePathTestCase\\test_walk\\bxkcny\\temp.zip/sub3', 'e:\\twistedbuildbot\\W32-full2.4-scmikes-select\\Twisted\\_trial_temp\\twisted.test.test_paths\\ZipFilePathTestCase\\test_walk\\bxkcny\\temp.zip/sub3/file3.ext1', 'e:\\twistedbuildbot\\W32-full2.4-scmikes-select\\Twisted\\_trial_temp\\twisted.test.test_paths\\ZipFilePathTestCase\\test_walk\\bxkcny\\temp.zip/sub3/file3.ext2', 'e:\\twistedbuildbot\\W32-full2.4-scmikes-select\\Twisted\\_trial_temp\\twisted.test.test_paths\\ZipFilePathTestCase\\test_walk\\bxkcny\\temp.zip/sub3/file3.ext3'] != ['e:\\twistedbuildbot\\W32-full2.4-scmikes-select\\Twisted\\_trial_temp\\twisted.test.test_paths\\ZipFilePathTestCase\\test_walk\\bxkcny\\temp.zip', 'e:\\twistedbuildbot\\W32-full2.4-scmikes-select\\Twisted\\_trial_temp\\twisted.test.test_paths\\ZipFilePathTestCase\\test_walk\\bxkcny\\temp.zip\\file1', 'e:\\twistedbuildbot\\W32-full2.4-scmikes-select\\Twisted\\_trial_temp\\twisted.test.test_paths\\ZipFilePathTestCase\\test_walk\\bxkcny\\temp.zip\\sub1', 'e:\\twistedbuildbot\\W32-full2.4-scmikes-select\\Twisted\\_trial_temp\\twisted.test.test_paths\\ZipFilePathTestCase\\test_walk\\bxkcny\\temp.zip\\sub1\\file2', 'e:\\twistedbuildbot\\W32-full2.4-scmikes-select\\Twisted\\_trial_temp\\twisted.test.test_paths\\ZipFilePathTestCase\\test_walk\\bxkcny\\temp.zip\\sub3', 'e:\\twistedbuildbot\\W32-full2.4-scmikes-select\\Twisted\\_trial_temp\\twisted.test.test_paths\\ZipFilePathTestCase\\test_walk\\bxkcny\\temp.zip\\sub3\\file3.ext1', 'e:\\twistedbuildbot\\W32-full2.4-scmikes-select\\Twisted\\_trial_temp\\twisted.test.test_paths\\ZipFilePathTestCase\\test_walk\\bxkcny\\temp.zip\\sub3\\file3.ext2', 'e:\\twistedbuildbot\\W32-full2.4-scmikes-select\\Twisted\\_trial_temp\\twisted.test.test_paths\\ZipFilePathTestCase\\test_walk\\bxkcny\\temp.zip\\sub3\\file3.ext3']

comment:2 Changed 15 years ago by Glyph

Keywords: review added

A fix for this (plus the reverted branch) is now in zippath-1952.

comment:3 Changed 15 years ago by Glyph

Owner: changed from Glyph to Jean-Paul Calderone

Please review.

comment:4 Changed 15 years ago by Stephen Thorne

Keywords: review removed
Owner: changed from Jean-Paul Calderone to Glyph

Reviewed. Looks good, test that this keeps the buildslaves green (do a branch test, especially on win32).

You've omitted docstrings. Please go through and add docstrings, especially to _ZipMapImpl.mapPath and TestLoader.loadPackage.

Once you've verified that it builds, and you're satisfied that the docstring requirement of UQDS is forfilled, please merge.

comment:5 Changed 15 years ago by Glyph

Resolution: fixed
Status: newclosed

(In [17672]) Re-merge branch for #1940, plus fix for Windows and additional docs.

Fixes #1952 Fixes #1940 Fixes #1276

Author: glyph

Reviewer: jerub

This is a do-over of r17651, hopefully this time without the test failure on the Windows buildslave.

comment:6 Changed 11 years ago by <automation>

Owner: Glyph deleted
Note: See TracTickets for help on using tickets.