[issue30264] [Windows] test_sax: Warning -- files was modified by test_sax

Serhiy Storchaka added the comment: > If you use directly the parser class and pass an open file object, in that > case, yes, my change closes the file. Or if the high-level *function* parse() is called with an open file object. I don't know whether this is desirable change, but this change

[issue30264] [Windows] test_sax: Warning -- files was modified by test_sax

STINNER Victor added the comment: > This changes the behavior if pass a file object to the parser. When the > parser failed it was possible to use the passed file object (for example call > tell()). Now it is closed. If the high-level *function* parse() is called with a filename, the caller

[issue30264] [Windows] test_sax: Warning -- files was modified by test_sax

Serhiy Storchaka added the comment: This changes the behavior if pass a file object to the parser. When the parser failed it was possible to use the passed file object (for example call tell()). Now it is closed. -- ___ Python tracker

[issue30264] [Windows] test_sax: Warning -- files was modified by test_sax

STINNER Victor added the comment: I applied my fix to 2.7, 3.5, 3.6 and master (3.7). Thanks for the helpful reviews Serhiy ;-) -- resolution: -> fixed stage: patch review -> resolved status: open -> closed ___ Python tracker

[issue30264] [Windows] test_sax: Warning -- files was modified by test_sax

STINNER Victor added the comment: New changeset d81f9e24ea89c0aaded1e0d3f8d8076bbd58c19a by Victor Stinner in branch '2.7': bpo-30264: ExpatParser now closes the source (#1476) https://github.com/python/cpython/commit/d81f9e24ea89c0aaded1e0d3f8d8076bbd58c19a --

[issue30264] [Windows] test_sax: Warning -- files was modified by test_sax

STINNER Victor added the comment: New changeset 0c9aa6ffd318c04ce23997b4704477d4a4d82829 by Victor Stinner in branch '3.5': bpo-30264: ExpatParser closes the source on error (#1451) (#1475) https://github.com/python/cpython/commit/0c9aa6ffd318c04ce23997b4704477d4a4d82829 --

[issue30264] [Windows] test_sax: Warning -- files was modified by test_sax

STINNER Victor added the comment: New changeset 0fe870f3f95ba883b2b06bc0d814bdab8d53df98 by Victor Stinner in branch '3.6': bpo-30264: ExpatParser closes the source on error (#1451) (#1474) https://github.com/python/cpython/commit/0fe870f3f95ba883b2b06bc0d814bdab8d53df98 --

[issue30264] [Windows] test_sax: Warning -- files was modified by test_sax

STINNER Victor added the comment: Oh, the fix for Python 2.7 is different. In fact, the file object was never explicitly closed by the parser. It is now always closed by the parser, on success or error. -- ___ Python tracker

[issue30264] [Windows] test_sax: Warning -- files was modified by test_sax

Changes by STINNER Victor : -- pull_requests: +1575 ___ Python tracker ___ ___

[issue30264] [Windows] test_sax: Warning -- files was modified by test_sax

Changes by STINNER Victor : -- pull_requests: +1574 ___ Python tracker ___ ___

[issue30264] [Windows] test_sax: Warning -- files was modified by test_sax

Changes by STINNER Victor : -- pull_requests: +1573 ___ Python tracker ___ ___

[issue30264] [Windows] test_sax: Warning -- files was modified by test_sax

STINNER Victor added the comment: New changeset ef9c0e732fc50aefbdd7c5a80e04e14b31684e66 by Victor Stinner in branch 'master': bpo-30264: ExpatParser closes the source on error (#1451) https://github.com/python/cpython/commit/ef9c0e732fc50aefbdd7c5a80e04e14b31684e66 --

[issue30264] [Windows] test_sax: Warning -- files was modified by test_sax

STINNER Victor added the comment: > I made one design choice: the close() method of byte and character streams > are called twice, I didn't call setByteStream(None) / > setCharacterStream(None) to avoid creating an inconsistent source. If you > really care, maybe we can reset the source to a

[issue30264] [Windows] test_sax: Warning -- files was modified by test_sax

STINNER Victor added the comment: Serhiy: "As you said, calling close() can have side effects (for example invoking self._cont_handler.endDocument()). This was the argument against PR 1444. It seems to me that if _entity_stack is not empty (this happens in case of error in entity parsing) the

[issue30264] [Windows] test_sax: Warning -- files was modified by test_sax

Serhiy Storchaka added the comment: As you said, calling close() can have side effects (for example invoking self._cont_handler.endDocument()). This was the argument against PR 1444. It seems to me that if _entity_stack is not empty (this happens in case of error in entity parsing) the

[issue30264] [Windows] test_sax: Warning -- files was modified by test_sax

STINNER Victor added the comment: > What if the third-party parser don't use prepare_input_source()? It can use > more efficient way if pass just a file name. About "third-party parsers": I have no idea of what are these parsers. It seems like Jython uses provides a parser. But I'm not

[issue30264] [Windows] test_sax: Warning -- files was modified by test_sax

Serhiy Storchaka added the comment: What if the third-party parser don't use prepare_input_source()? It can use more efficient way if pass just a file name. Wouldn't be better to move your code into the parser's method parse()? If a file is opened inside the parse() method and is not exposed

[issue30264] [Windows] test_sax: Warning -- files was modified by test_sax

STINNER Victor added the comment: I wrote a different change which doesn't use the parser anymore, since it seems like xml.sax allows to plug third-party parsers. My new PR 1451 closes directly the file or urllib object, instead of touching the parser (which can have more visible side

[issue30264] [Windows] test_sax: Warning -- files was modified by test_sax

Changes by STINNER Victor : -- pull_requests: +1549 ___ Python tracker ___ ___

[issue30264] [Windows] test_sax: Warning -- files was modified by test_sax

STINNER Victor added the comment: > The problem is that os.unlink(TESTFN) ignores all OSError: os.unlink(TESTFN) > fails because there is still an open file object somewhere. I created the issue #30265: test.support.unlink() should fail or emit a warning on WindowsError: [Error 32] The

[issue30264] [Windows] test_sax: Warning -- files was modified by test_sax

STINNER Victor added the comment: > The question is if all parser provide a close() method? It seems like xml.sax supports pluggable parsers, at least using the PY_SAX_PARSER environment variable. So it would be safer to add a cheap "if hasattr(parser, 'close'):" test; --

[issue30264] [Windows] test_sax: Warning -- files was modified by test_sax

STINNER Victor added the comment: https://github.com/python/cpython/pull/1444 is written for the master branch. IMHO the bug must be fixed in all supported branches. The question is if all parser provide a close() method? -- versions: +Python 3.5, Python 3.6, Python 3.7

[issue30264] [Windows] test_sax: Warning -- files was modified by test_sax

Changes by STINNER Victor : -- pull_requests: +1542 ___ Python tracker ___ ___

[issue30264] [Windows] test_sax: Warning -- files was modified by test_sax

New submission from STINNER Victor: Running test_sax of Python 2.7 on Windows emits the following warning: Warning -- files was modified by test_sax The problem is that os.unlink(TESTFN) ignores all OSError: os.unlink(TESTFN) fails because there is still an open file object somewhere. The