Provide visible yaml settings or document organize plugin behavior for directory merging

[Dmitrii Shcherbakov]

Hi,

I was working on a change for snap-libvirt and encountered a behavior which is not apparent on the first sight.

https://github.com/openstack-snaps/snap-libvirt/commit/8cd9af031a9b01f360975bddc1964e9380b983e6

Originally '*' has been used in the organize plugin which lead to an error due to the fact that the directory structure used by both parts (libvirt and qemu) is similar:

"path ./ already exists"

This error is cryptic unless you look at the source code of snapcraft:

def _organize:
https://github.com/snapcore/snapcraft/blob/9e00ffc151b46026b414c8b4b8e44a3a9d4c0b75/snapcraft/internal/pluginhandler/__init__.py#L449

calls def _organize_filesets(fileset, base_dir):
https://github.com/snapcore/snapcraft/blob/9e00ffc151b46026b414c8b4b8e44a3a9d4c0b75/snapcraft/internal/pluginhandler/__init__.py#L907

Now this function has 3 code paths:

def _organize_filesets(fileset, base_dir):
    for key in sorted(fileset, key=lambda x: ['*' in x, x]):
        src = os.path.join(base_dir, key)
        dst = os.path.join(base_dir, fileset[key])

        sources = iglob(src, recursive=True)

        for src in sources:
            if os.path.isdir(src) and '*' not in key: # <---- first
                file_utils.link_or_copy_tree(src, dst)
                # TODO create alternate organization location to avoid
                # deletions.
                shutil.rmtree(src)
            elif os.path.isfile(dst): # <----- second
                raise EnvironmentError(
                    'Trying to organize file {key!r} to {dst!r}, '
                    'but {dst!r} already exists'.format(
                        key=key, dst=os.path.relpath(dst, base_dir)))
            else: # <----- third
                os.makedirs(os.path.dirname(dst), exist_ok=True)
                shutil.move(src, dst)

The third code path is what gets picked in case there is a '*' in the key and the destination in not a file.

shutil.move is the call that gave 'path ./ already exists' error which I learned by grepping cpython source code:

def move function
https://hg.python.org/cpython/file/2.7/Lib/shutil.py#l306

Now, looking at an alternative code path (the first one) which calls link_or_copy_tree I see:

https://github.com/snapcore/snapcraft/blob/05ba34a324c781f3e454d88f88267c5356771f9a/snapcraft/file_utils.py#L93

def link_or_copy_tree(source_tree, destination_tree,
                      copy_function=link_or_copy):
    """Copy a source tree into a destination, hard-linking if possile.
    :param str source_tree: Source directory to be copied.
    :param str destination_tree: Destination directory. If this directory
                                 already exists, the files in
                                 `source_tree` will take precedence.
The destination_tree argument gives a very good description in terms of what is going to happen but it is "hidden" in the source code which forces a regular snapcrafter to do some grepping.

Avoiding '*' helped me to select the right code path but it would be nice to have more visibility for that (either in a form of a knob in snapcraft.yaml or in the docs).

Thanks!