905264 - Allow FileAvoidWrite to write in binary mode

Status: RESOLVED FIXED

(Firefox Build System :: General, defect)

Description

[Gregory Szorc [:gps]]

Currently FileAvoidWrite writes in text mode. It should support writing in binary mode. As part of this, we should fix https://hg.mozilla.org/integration/mozilla-inbound/rev/1fa1f28c6edf.

Comment 1

[Gregory Szorc [:gps]]

After 10 minutes on this patch, it looks like that solving this (at least properly) will require making FileAvoidWrite, MockedFile, and MockedOpen unicode aware / Python 3 compatible. Maybe I'll get lucky and find a simple solution. But there are definitely plenty of Python 3 time bombs in this code.

Comment 2

[Gregory Szorc [:gps]]

I ran into text/binary issues with FileAvoidWrite as part of bug 687388. I worked around it by not using FileAvoidWrite and used open() directly.

Having a few months to think about this patch, I think it might be easier to create binary versions of FileAvoidWrite, MockedFile, and MockedFile that utilize io.BytesIO under the hood rather than try to hack mixed mode support into the existing classes.

For Python 3, we can have a class that swaps out an io.BytesIO/io.StringIO instance depending on the file mode. But we don't want to do that today because io.StringIO (unlike StringIO.StringIO) enforces types consistency: if you pass a str to io.BytesIO, no automatic coercion is attempted and an exception is raised. Adapting our existing FileAvoidWrite consumers to consistently use str/unicode would be a major effort. Furthermore, io.StringIO doesn't perform as well as StringIO.StringIO. I believe the perf issues were addressed in Python 3. But this isn't something we should bring upon ourselves unless we really need Python 3 compatibility.

Comment 3

[Mike Hommey [:glandium]]

This was fixed as part of the changes in bug 1236111. Specifically, https://hg.mozilla.org/mozilla-central/rev/d423db25aa46