Code review comment for lp:~songofacandy/bzr/fix-524560

Revision history for this message
Martin Packman (gz) wrote :

Some general comments:

* Adding O_NOINHERIT to the default flags bzr uses seems sensible.
* Don't like the style of having shadows in osutils that are almost-but-not-quite builtins or standard library functions.

> Python 2.4 uses msvcrt71 (VC++2003?) and msdn doesn't document 'N' mode
> and O_NOINHERIT. But os module in Python 2.4 have O_NOINHERIT so fcntl.h in
> VC++2003 should have O_NOINHERIT.
> I don't know the file is inherited to child process or not.
> But I installed Python 2.4 and ``open('foo', 'wbN').write('foo')`` just works.

MSDN is correct, it does *not* work on Pythons compiled with VS < 2005. The open succeeds but the flag is ignored. I threw together some quick tests that show this, and don't think this change should go in untested or this lightly documented.

review: Abstain

« Back to merge proposal