A "safe" copy function for Python

Question

I have a Python script for doing "safe" file copying.

There are a couple of rules for this copy function:

• Existing files should never be overwritten. So if I try to copy $file1 to $location, and there is already $file2 at $location, then $file2 will not be overwritten.
• Exact filenames are not important. So if I try to copy a file to $location.txt, and because there’s already a different file there, it instead gets copied to $location-1.txt, that’s okay.
• Copying should be idempotent. That is, if I try to copy a file to $location, and it's already there, nothing else happens. I'm not going to create more copies at $location-1, $location-2, $location-3, etc.

An example use case: I'm importing some camera photos into a folder, but some of the photos may be duplicates (and I don't want two copies) or have the same filename as another photo (and I want to keep both, but small changes to the name aren't important).

import filecmp
import os
import shutil

def increment_filename(filename, marker="-"):
    """Appends a counter to a filename, or increments an existing counter."""
    basename, fileext = os.path.splitext(filename)

    # If there isn't a counter already, then append one
    if marker not in basename:
        components = [basename, 1, fileext]

    # If it looks like there might be a counter, then try to coerce it to an
    # integer and increment it. If that fails, then just append a new counter.
    else:
        base, counter = basename.rsplit(marker, 1)
        try:
            new_counter = int(counter) + 1
            components = [base, new_counter, fileext]
        except ValueError:
            components = [base, 1, fileext]

    # Drop in the marker before the counter
    components.insert(1, marker)

    new_filename = "%s%s%d%s" % tuple(components)
    return new_filename

def copyfile(src, dst):
    """Copies a file from path src to path dst.

    If a file already exists at dst, it will not be overwritten, but:

     * If it is the same as the source file, do nothing
     * If it is different to the source file, pick a new name for the copy that
       is distinct and unused, then copy the file there.

    Returns the path to the copy.
    """
    if not os.path.exists(src):
        raise ValueError("Source file does not exist: {}".format(src))

    # Create a folder for dst if one does not already exist
    if not os.path.exists(os.path.dirname(dst)):
        os.makedirs(os.path.dirname(dst))

    # Keep trying to copy the file until it works
    while True:

        # If there is no file of the same name at the destination path, copy
        # to the destination
        if not os.path.exists(dst):
            shutil.copyfile(src, dst)
            return dst

        # If the namesake is the same as the source file, then we don't need to
        # do anything else
        if filecmp.cmp(src, dst):
            return dst

        # There is a namesake which is different to the source file, so pick a
        # new destination path
        dst = increment_filename(dst)

    return dst

Particular areas I’m unsure about:

• Does it make sense? Is it clear what the code is doing?
• Can I replace any of this with standard library functions?
• I wrote it in Python 2, but will run it in 2 and 3. Brief testing suggests that it runs fine in Python 3, but pointers to anything I've missed would be helpful.
• Is this good Python?

Answer 1

As the documentation for shutil.copyfile(src, dst) says,

If dst already exists, it will be replaced.

You have attempted to work around that by checking if not os.path.exists(dst) first. However, that logic is vulnerable to a race condition if the destination file springs into existence just after the check.

A way to ensure that you do not overwrite an existing file is to use os.open() with the os.O_EXCL flag. You would need to write the read-write loop yourself to transfer the file contents, but that is not difficult.

Answer 2

Consider making increment_filename a generator. You could make it a generator that yields (in order) the filenames filename.txt, filename-1.txt, filename-2.txt etc without ever breaking out of the loop. The function calling it could take the next one until it finds one that doesn't exist yet. That would avoid unnecessary parsing of the filename again and again.