Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add create mode to open() #56969

Closed
DavidTownshend mannequin opened this issue Aug 16, 2011 · 44 comments
Closed

Add create mode to open() #56969

DavidTownshend mannequin opened this issue Aug 16, 2011 · 44 comments
Labels
stdlib Python modules in the Lib dir topic-IO type-feature A feature request or enhancement

Comments

@DavidTownshend
Copy link
Mannequin

DavidTownshend mannequin commented Aug 16, 2011

BPO 12760
Nosy @amauryfa, @ncoghlan, @pitrou, @vstinner, @benjaminp, @ssbr, @merwok, @Julian, @akheron
Files
  • open_create.patch: Patch implementing 'c' mode for open()
  • open_create_x-2.patch: Implement 'x' mode for open() (doc typo corrected)
  • open_create_x-3.patch
  • x_flag.diff
  • x_diff2.patch
  • Note: these values reflect the state of the issue at the time it was migrated and might not reflect the current state.

    Show more details

    GitHub fields:

    assignee = None
    closed_at = <Date 2012-01-10.07:57:38.782>
    created_at = <Date 2011-08-16.13:49:18.454>
    labels = ['type-feature', 'library', 'expert-IO']
    title = 'Add create mode to open()'
    updated_at = <Date 2012-05-20.09:43:36.422>
    user = 'https://bugs.python.org/DavidTownshend'

    bugs.python.org fields:

    activity = <Date 2012-05-20.09:43:36.422>
    actor = 'python-dev'
    assignee = 'none'
    closed = True
    closed_date = <Date 2012-01-10.07:57:38.782>
    closer = 'neologix'
    components = ['Library (Lib)', 'IO']
    creation = <Date 2011-08-16.13:49:18.454>
    creator = 'David.Townshend'
    dependencies = []
    files = ['22910', '22935', '24179', '24195', '24210']
    hgrepos = []
    issue_num = 12760
    keywords = ['patch']
    message_count = 44.0
    messages = ['142193', '142194', '142198', '142200', '142201', '142308', '142309', '142314', '142315', '142320', '142321', '142333', '142335', '142352', '142902', '142910', '142912', '142916', '142923', '142928', '142930', '146684', '146686', '147205', '147841', '150262', '150273', '150391', '150402', '150406', '150674', '150860', '150867', '150881', '150884', '150901', '150982', '151008', '151028', '151113', '151244', '151249', '161150', '161193']
    nosy_count = 15.0
    nosy_names = ['amaury.forgeotdarc', 'ncoghlan', 'pitrou', 'vstinner', 'benjamin.peterson', 'Devin Jeanpierre', 'eric.araujo', 'Arfrever', 'cvrebert', 'neologix', 'docs@python', 'Julian', 'python-dev', 'petri.lehtinen', 'David.Townshend']
    pr_nums = []
    priority = 'normal'
    resolution = 'fixed'
    stage = 'resolved'
    status = 'closed'
    superseder = None
    type = 'enhancement'
    url = 'https://bugs.python.org/issue12760'
    versions = ['Python 3.3']

    @DavidTownshend
    Copy link
    Mannequin Author

    DavidTownshend mannequin commented Aug 16, 2011

    Currently, opening a file with open(file, 'w') overwrites existing files. It would be useful for open() to raise an error when the file exists. This proposal is to add a 'c' mode to open, which has the effect to creating a file and opening it for writing, but raising an IOError if it already exists (i.e. the same as os.open(file, os.O_EXCL|os.O_CREAT).

    The attached patch implements this, including tests and documentation.

    @DavidTownshend DavidTownshend mannequin added docs Documentation in the Doc dir tests Tests in the Lib/test dir stdlib Python modules in the Lib dir topic-IO type-feature A feature request or enhancement labels Aug 16, 2011
    @vstinner
    Copy link
    Member

    I'm not sure that O_EXCL is portable (exist on all platforms) because Python source code uses "#ifdef O_EXCL".

    @benjaminp
    Copy link
    Contributor

    I think this should be brought up on python-ideas or python-dev.

    @neologix
    Copy link
    Mannequin

    neologix mannequin commented Aug 16, 2011

    See also bpo-12105.
    A couple downsides:

    • O_EXCL is not necessarily portable (doesn't work well with NFS, maybe not on Windows?)
    • O_EXCL is useful, as is O_CLOEXEC: to be consistent, we should also end up adding all other commonly-used flags, which are really OS-specific

    Furthermore, you can achieve the same thing with:
    os.fdopen(os.open('/etc/fstab', os.O_WRONLY|os.O_CLOEXEC|os.O_CREAT))
    it's more verbose, though.

    @DavidTownshend
    Copy link
    Mannequin Author

    DavidTownshend mannequin commented Aug 16, 2011

    It was discussed on python-ideas, but the subject of the thread was actually on shutils.move so it was not really discussed much. I will repost this idea separately.

    @pitrou
    Copy link
    Member

    pitrou commented Aug 18, 2011

    The "#ifdef O_EXCL" in the source code is probably very old. Copying a message I posted on python-ideas:

    O_EXCL is a POSIX standard. It is also supported
    under Windows by the _open/_wopen compatibility functions (which we use
    for file I/O).

    Probably there are very old systems which don't support it, and perhaps
    new systems that don't implement it *correctly* (meaning not
    atomically); for the former I'd say we just don't care (who's gonna run
    Python 3 on a 1995 system?) and for the latter, well, if the OS
    designers think it's fine, let's just expose it as it is.

    As for NFS, there's an interesting comment from 2007 here:
    http://lwn.net/Articles/251971/

    “My NFS tester shows that it at least appears to work with Linux,
    Solaris and FreeBSD:
    http://www.dovecot.org/list/dovecot/2007-July/024102.html. Looking at
    Linux 2.6 sources it doesn't look like it tries to implement a racy
    O_EXCL check in client side (fs/nfs/nfs3proc.c nfs3_proc_create()), so
    the test's results should be correct. I don't know if other OSes do
    that. I guess it would be nice to have a better O_EXCL tester which
    tries to catch race conditions.”

    @pitrou pitrou removed docs Documentation in the Doc dir tests Tests in the Lib/test dir labels Aug 18, 2011
    @DavidTownshend
    Copy link
    Mannequin Author

    DavidTownshend mannequin commented Aug 18, 2011

    My aim isn't to add all the commonly used flags, that would be pointless since its already possible using os.open. The aim is to add a missing feature to the builtin open(), i.e. file creation. At the moment open() implements read, write, and append, and creation is only implied by write. But in many cases, an explicit creation is desired (i.e, specifically create a new file, with an exception on failure). It is true that this is possible with os.open, but it is somewhat obscure, especially for beginners, and is not as easy to read as "open(file, 'c')"

    @neologix
    Copy link
    Mannequin

    neologix mannequin commented Aug 18, 2011

    The "#ifdef O_EXCL" in the source code is probably very old. Copying a message I posted on python-ideas:

    O_EXCL is a POSIX standard. It is also supported
    under Windows by the _open/_wopen compatibility functions (which we use
    for file I/O).

    If it's supported by Windows then I'm OK (not that I personaly care
    about Windows :-).

    and is not as easy to read as "open(file, 'c')"

    Well, I'd rather have this flag called 'x', to be consistent with
    glibc's fopen():

    """
    c (since glibc 2.3.3)
    Do not make the open operation, or subsequent read and write
    operations, thread cancellation points.

       x      Open the file exclusively (like the O_EXCL flag of
    

    open(2)). If the
    file already exists, fopen() fails, and sets errno to
    EEXIST. This
    flag is ignored for fdopen().
    """

    By the way, could you submit your patch as a mercurial diff ("hg diff")?
    It makes it easier to review under Rietveld.

    @DavidTownshend
    Copy link
    Mannequin Author

    DavidTownshend mannequin commented Aug 18, 2011

    Changing form 'c' to 'x' is easy enough, and if there is already a convention it makes sense to stick to it.

    I thought I had done a mercurial diff! I'll try again and resubmit.

    @Julian
    Copy link
    Mannequin

    Julian mannequin commented Aug 18, 2011

    A minor documentation error in io.rst line 475 which was changed to:

    • The mode can be 'c', 'r', 'w' or 'a' for reading
    • (default), writing, or appending.

    and should have "creating" at the front I assume, like you have it later.

    @pitrou
    Copy link
    Member

    pitrou commented Aug 18, 2011

    Well, I'd rather have this flag called 'x', to be consistent with
    glibc's fopen():

    """
    c (since glibc 2.3.3)
    Do not make the open operation, or subsequent read and write
    operations, thread cancellation points.

       x      Open the file exclusively (like the O_EXCL flag of
    

    open(2)). If the
    file already exists, fopen() fails, and sets errno to
    EEXIST. This
    flag is ignored for fdopen().

    Yeah, but I think "exclusively" is quite misleading since it does not
    perform any locking of any kind. Also, I don't think we'll ever
    integrate the glibc's "c" option in io.open().

    @neologix
    Copy link
    Mannequin

    neologix mannequin commented Aug 18, 2011

    Yeah, but I think "exclusively" is quite misleading since it does not
    perform any locking of any kind.

    It might be misleading, but I find it clear enough, and this name has been endorsed by POSIX.

    Furthermore, there's an added bonus: actually, with the old I/O layer, one can already pass an 'x' flag to open, since it just calls fopen:
    """
    cf@neobox:$ strace -e open python -c "open('/tmp/foo', 'wx')"
    [...]
    open("/tmp/foo", O_WRONLY|O_CREAT|O_EXCL|O_TRUNC|O_LARGEFILE, 0666) = 3
    cf@neobox:
    $ strace -e open python -c "open('/tmp/foo', 'wx')"
    [...]
    open("/usr/lib/pymodules/python2.6/<string>", O_RDONLY|O_LARGEFILE) = -1 ENOENT (No such file or directory)
    IOError: [Errno 17] File exists: '/tmp/foo'
    """

    I don't know if it's documented behavior, but the OP in bpo-12105 was using it with python 2.
    Changing it to 'x' would make such code backward-compatible.

    Finally, when I read open('/tmp/foo', 'wx'), it's immediately clear to me what's going on, while I'd have to look at open()'s documentation to find out what the 'c' flag does.

    @vstinner
    Copy link
    Member

    I don't know if it's documented behavior

    See bpo-12103: it is not documented.

    @DavidTownshend
    Copy link
    Mannequin Author

    DavidTownshend mannequin commented Aug 18, 2011

    I hope this patch suits you better :-)

    I've updated the documentation typo (thanks for pointing that out). I've also changed 'c' to 'x', since I think that if there is a convention we should stick to it. I don't think that it matters if the glibc docs say 'exclusive', as long it its clear in the python docs what it does, which I think it is. Having said that, I don't really have a strong opinion either way, so I'll happily change it back to 'c' if its preferred.

    @neologix
    Copy link
    Mannequin

    neologix mannequin commented Aug 24, 2011

    So, what was the conclusion of the discussion brought up on python-dev?
    I had a feeling some core devs were opposed to this (I'm personally -0).

    @pitrou
    Copy link
    Member

    pitrou commented Aug 24, 2011

    I haven't seen any discussion on python-dev. Have I missed something?

    @neologix
    Copy link
    Mannequin

    neologix mannequin commented Aug 24, 2011

    I haven't seen any discussion on python-dev. Have I missed something?

    It was on Python-ideas actually:
    http://mail.python.org/pipermail/python-ideas/2011-August/011183.html

    @pitrou
    Copy link
    Member

    pitrou commented Aug 24, 2011

    Ah, right. Well I think all arguments against it were quite weak (or even wrong). Let's see:

    • it's not cross-platform: actually it is (OS_EXCL has been POSIX since at least 1997 (*), and Windows also has it)

    • os.open followed by os.fdopen is easy: it isn't that easy to get the incantation right (the pure Python open() in _pyio is 70 lines of code), especially if you want the file object to have the right "name" attribute

    • it doesn't fill a use case: actually, avoiding race conditions is an important use case, even though many people may never encounter it (I must admit I myself never really cared about this)

    So this looks like a reasonable feature request IMHO.

    (*) http://pubs.opengroup.org/onlinepubs/007908799/xsh/open.html

    @amauryfa
    Copy link
    Member

    • os.open followed by os.fdopen is easy: it isn't that easy to get
      the incantation right (the pure Python open() in _pyio is 70 lines
      of code), especially if you want the file object to have the right
      "name" attribute

    What if we can override the inner call to os.open()?
    For example, io.open() could grow an additional argument "fd_opener" which defaults to the equivalent of os.open.

    Then creation mode can be expressed like this:
    open(filename, 'w',
    fd_opener=lambda path, mode: os.open(path, mode|os.O_CREAT)
    Another use case with openat (see bpo-12797):
    open(filename,
    fd_opener=lambda path, mode: os.openat(fd, path, mode)

    @pitrou
    Copy link
    Member

    pitrou commented Aug 24, 2011

    What if we can override the inner call to os.open()?
    For example, io.open() could grow an additional argument "fd_opener"
    which defaults to the equivalent of os.open.

    I agree it would be a much better situation than what we have now.

    @vstinner
    Copy link
    Member

    open(filename, 'w',
    fd_opener=lambda path, mode: os.open(path, mode|os.O_CREAT)

    I prefer open(name, "c").

    it doesn't fill a use case: actually, avoiding race conditions
    is an important use case, ...

    It may help the issue bpo-8604 (and maybe bpo-8828).

    @neologix neologix mannequin closed this as completed Oct 29, 2011
    @DavidTownshend
    Copy link
    Mannequin Author

    DavidTownshend mannequin commented Oct 31, 2011

    I see this has been marked as a duplicate of http://bugs.python.org/issue12797. Please explain how this is, since that proposal does not appear to provide the functionality discussed here.

    @amauryfa
    Copy link
    Member

    bpo-12797 would allow things like:

    def create_exclusive_file(filename):
      return open(filename, "w",
                  opener=lambda path, mode: os.open(path, mode|os.O_CREAT|os.O_EXCL))

    @DavidTownshend
    Copy link
    Mannequin Author

    DavidTownshend mannequin commented Nov 7, 2011

    It is already possible to write a wrapper function that does it:

    def create(file):
        fd = os.open(file, os.O_EXCL | os.O_CREAT | os.O_WRONLY)
        return os.fdopen(fd)

    The point it not that it can't be done, but that it is not straight forward. The docs say this about os.open(): "This function is intended for low-level I/O. For normal usage, use the built-in function open()"

    I wouldn't call creating a new file low-level I/O, but it is normal usage.

    @merwok
    Copy link
    Member

    merwok commented Nov 18, 2011

    See bpo-13424 for a doc request about this.

    @ssbr
    Copy link
    Mannequin

    ssbr mannequin commented Dec 26, 2011

    C11 uses 'x' for this, for what it's worth.

    This is not a "duplicate issue". The openat solution is no easier than the os.open solution.

    @pitrou
    Copy link
    Member

    pitrou commented Dec 27, 2011

    C11 uses 'x' for this, for what it's worth.

    This is not a "duplicate issue". The openat solution is no easier than
    the os.open solution.

    Ok, let's re-open then. I'm not sold on the feature, but the fact C11 adds a dedicated letter mode for it could be a good excuse for us to mimick it :)

    @pitrou pitrou reopened this Dec 27, 2011
    @merwok
    Copy link
    Member

    merwok commented Dec 30, 2011

    This is not a "duplicate issue". The openat solution is no easier than the os.open
    solution.

    Amaury did not suggest to use openat, but the new opener argument to open, which was especially added for use cases such as the one discussed here:

    ...
    open_exclusive = lambda path, mode: os.open(path, mode|os.O_CREAT|os.O_EXCL))
    ...
    fp = open(filename, 'w', opener=open_exclusive)
    ...
    

    That’s why this bug was initially closed as duplicate.

    @ssbr
    Copy link
    Mannequin

    ssbr mannequin commented Dec 30, 2011

    Amaury did not suggest to use openat, but the new opener argument to open, which was especially added for use cases such as the one discussed here:

    Sorry, yes. Wrong words, same thought. We can implement this using opener, but we could implement this with os.open before. What's changed, except that there's more ways to do it? (There is slightly more versatility with the opener method, but no more obviousness and no less typing).

    My understanding from reading the other thread is that this is not the primary use-case of the new parameter for open(). In fact, this ticket was not really mentioned at all there.

    @merwok
    Copy link
    Member

    merwok commented Dec 31, 2011

    [...] There is slightly more versatility with the opener method, but no more obviousness
    and no less typing.

    I agree with your opinion. I re-read this report:

    • Antoine thinks this fills an important use case, namely avoiding race conditions
    • Amaury then suggested the opener argument idea, which was implemented in the openat bug
    • Antoine judged it would be better than what we had before

    I don’t have a strong opinion on “opener is generic and good enough” vs. “not as ice as it could be”. Antoine seems to agree with you, so your patch will get reviewed and eventually accepted. Cheers

    @neologix
    Copy link
    Mannequin

    neologix mannequin commented Jan 5, 2012

    I've done a small review.

    @neologix
    Copy link
    Mannequin

    neologix mannequin commented Jan 8, 2012

    I intend to commit this patch within a couple days (unless anyone objects of course).

    @pitrou
    Copy link
    Member

    pitrou commented Jan 8, 2012

    I don't think the "created()" method has to be exposed. People can inspect the "mode" attribute if they want to have that information.
    (besides, the semantics are misleading since a new file opened with "w" has also be created, but created() would return False)

    There's some bogus indentation in the patch (it uses tab characters in some places).

    Also:

    + if not (creating + reading or writing or appending):

    Not sure why the "+". Shouldn't it be "or" instead?

    @neologix
    Copy link
    Mannequin

    neologix mannequin commented Jan 8, 2012

    Here's a new version of the patch that should address all the comments.

    @pitrou
    Copy link
    Member

    pitrou commented Jan 8, 2012

    Here's a new version of the patch that should address all the comments.

    Just a small note: FileExistsError is raised, not exactly OSError, when
    the file exists.

    @neologix
    Copy link
    Mannequin

    neologix mannequin commented Jan 8, 2012

    Just a small note: FileExistsError is raised, not exactly OSError,
    when the file exists.

    I've updated the doc accordingly.

    @python-dev
    Copy link
    Mannequin

    python-dev mannequin commented Jan 9, 2012

    New changeset bf609baff4d3 by Charles-François Natali in branch 'default':
    Issue bpo-12760: Add a create mode to open(). Patch by David Townshend.
    http://hg.python.org/cpython/rev/bf609baff4d3

    @neologix
    Copy link
    Mannequin

    neologix mannequin commented Jan 10, 2012

    Committed.
    David, thanks for the patch!

    @neologix neologix mannequin closed this as completed Jan 10, 2012
    @neologix
    Copy link
    Mannequin

    neologix mannequin commented Jan 10, 2012

    Nick suggested to call the new flag "exclusive create" in the doc (and explain in whatsnew that it's based C11 new 'x' flag).
    Could someone please check the attached patch?
    My wording sounds really clumsy, so I'd prefer if a native speaker could review it.

    @DavidTownshend
    Copy link
    Mannequin Author

    DavidTownshend mannequin commented Jan 12, 2012

    I've done a bit or rewording in the io docs, but I honestly don't know if this is any better that what you already had!

    @python-dev
    Copy link
    Mannequin

    python-dev mannequin commented Jan 14, 2012

    New changeset 8bcbe2dc3835 by Charles-François Natali in branch 'default':
    Issue bpo-12760: Refer to the new 'x' open mode as "exclusive creation" mode.
    http://hg.python.org/cpython/rev/8bcbe2dc3835

    @neologix
    Copy link
    Mannequin

    neologix mannequin commented Jan 14, 2012

    Thanks, I've committed your version.

    @akheron
    Copy link
    Member

    akheron commented May 19, 2012

    Shouldn't the documentation of builtin open() (in Doc/library/functions.rst) be updated too?

    @python-dev
    Copy link
    Mannequin

    python-dev mannequin commented May 20, 2012

    New changeset ef406c4c6463 by Charles-François Natali in branch 'default':
    Issue bpo-12760: Add some mising documentation about the new x exclusive
    http://hg.python.org/cpython/rev/ef406c4c6463

    @ezio-melotti ezio-melotti transferred this issue from another repository Apr 10, 2022
    Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
    Labels
    stdlib Python modules in the Lib dir topic-IO type-feature A feature request or enhancement
    Projects
    None yet
    Development

    No branches or pull requests

    6 participants