Opened 4 years ago

Closed 4 years ago

#26301 closed defect (fixed)

Python 3 bug in zipfile

Reported by: embray Owned by:
Priority: minor Milestone: sage-8.4
Component: python3 Keywords:
Cc: Merged in:
Authors: Frédéric Chapoton Reviewers: Erik Bray
Report Upstream: Reported upstream. No feedback yet. Work issues:
Branch: 60fb5bf (Commits, GitHub, GitLab) Commit: 60fb5bfb67446bb3ac87400157c3faa0e1d4a4bf
Dependencies: Stopgaps:

Status badges

Description (last modified by embray)

There's a doctest that invokes this bug in Python 3.6's zipfile module:

sage: from sage.plot.plot3d.shapes import Cylinder
sage: C = Cylinder(3, 1, closed=False)
sage: C.jmol_repr(C.testing_render_params())
Exception ignored in: <bound method ZipFile.__del__ of <zipfile.ZipFile [closed]>>
Traceback (most recent call last):
  File "/home/embray/src/sagemath/sage-python3/local/lib/python3.6/", line 1663, in __del__
  File "/home/embray/src/sagemath/sage-python3/local/lib/python3.6/", line 1681, in close
  File "/home/embray/src/sagemath/sage-python3/local/lib/python3.6/", line 1783, in _write_end_record
    centDirSize, centDirOffset, len(self._comment))
struct.error: argument out of range
['pmesh obj_1 "obj_1.pmesh"\ncolor pmesh  [102,102,255]']

The Graphics3d.testing_render_params() opens a ZipFile in write mode to /dev/null, so the bug seems to have to do with writing a zipfile to /dev/null (probably to do with a tell() call somewhere).

For the purposes of testing there's no reason this needs to go to '/dev/null'. It can just as well use a temp file, so we can implement that workaround. But we should also open an upstream bug report for the zipfile problem, if it hasn't already been reported.

Upstream bug report:

Change History (6)

comment:1 Changed 4 years ago by chapoton

  • Authors set to Frédéric Chapoton
  • Branch set to u/chapoton/26301
  • Status changed from new to needs_review

comment:2 Changed 4 years ago by git

  • Commit set to 60fb5bfb67446bb3ac87400157c3faa0e1d4a4bf

Branch pushed to git repo; I updated commit sha1. New commits:

60fb5bftrac 26301 replace dev/null by temporary file

comment:3 Changed 4 years ago by embray

Sure, looks good to me. I still need to make sure to make an upstream bug report (but this is pretty minor, and after fixing it in Sage I doubt we'll revisit the issue).

comment:4 Changed 4 years ago by embray

Interestingly, I can't reproduce the bug on Cygwin, only on Linux. I was trying to see if it was still broken in master, and at first I thought maybe it was fixed. But no, it just doesn't happen on Cygwin that I can tell. Maybe some difference in how /dev/null works; not sure.

comment:5 Changed 4 years ago by embray

  • Description modified (diff)
  • Report Upstream changed from Not yet reported upstream; Will do shortly. to Reported upstream. No feedback yet.
  • Reviewers set to Erik Bray
  • Status changed from needs_review to positive_review

Ok, upstream bug report submitted. LGTM.

comment:6 Changed 4 years ago by vbraun

  • Branch changed from u/chapoton/26301 to 60fb5bfb67446bb3ac87400157c3faa0e1d4a4bf
  • Resolution set to fixed
  • Status changed from positive_review to closed
Note: See TracTickets for help on using tickets.