fix 'dir already exists' when supplying both pip_args and site_packages

[jarekr]

This happens when i run shiv to package the current python project as well as the dependencies. I am running shiv like below and seeing this error without this change.

I am not totally sure how to create a test for this.

source /home/jarekr/venv/_tools_env/bin/activate && shiv -e we.cli:we -o dist/wep.linux . -E --python "/usr/local/bin/python3 -I" --site-packages venv/we-tools-py-linux/lib/python3.7/site-packages
Traceback (most recent call last):
  File "/home/jarekr/venv/_tools_env/bin/shiv", line 11, in <module>
    sys.exit(main())
  File "/home/jarekr/venv/_tools_env/lib/python3.7/site-packages/click/core.py", line 722, in __call__
    return self.main(*args, **kwargs)
  File "/home/jarekr/venv/_tools_env/lib/python3.7/site-packages/click/core.py", line 697, in main
    rv = self.invoke(ctx)
  File "/home/jarekr/venv/_tools_env/lib/python3.7/site-packages/click/core.py", line 895, in invoke
    return ctx.invoke(self.callback, **ctx.params)
  File "/home/jarekr/venv/_tools_env/lib/python3.7/site-packages/click/core.py", line 535, in invoke
    return callback(*args, **kwargs)
  File "/home/jarekr/venv/_tools_env/lib/python3.7/site-packages/shiv/cli.py", line 127, in main
    shutil.copytree(site_packages, tmp_site_packages)
  File "/usr/local/lib/python3.7/shutil.py", line 321, in copytree
    os.makedirs(dst)
  File "/usr/local/lib/python3.7/os.py", line 221, in makedirs
    mkdir(name, mode)
FileExistsError: [Errno 17] File exists: '/tmp/tmpuebw1ewo'
make: *** [build] Error 1

[lorencarvalho]

Hi @jarekr,

Thanks for the PR! This is an interesting bug and I'm glad you found it so we can fix it. Here's a quick recap of the bug and how it came to be:

Prior to 0.0.51, cli.py would copy any supplied site-packages dir into a staging directory prior to creating the pyz, PR #114 slightly modified that logic to only do the copying if a site-packages dir is supplied and the user wants to install something additional (in your case, your local package). The copying always worked before because our staging directory was created like so:

with TemporaryDirectory() as working_path:
    tmp_site_packages = Path(working_path, "site-packages")

But now we do not add the subdirectory (site-packages) in order to simplify the logic gate around whether we copy another dir into it or not.

So, we never needed to overwrite an existing directory, therefore never encountered this class of error before! Kind of a tricky bug, again, thanks for finding it :)

However, there is a bit of trouble now, because the fix you are proposing only fixes the problem for Python 3.8, as the dirs_exist_ok parameter was only added recently (python/cpython#8792), which is also why the pipelines are failing:

src/shiv/cli.py:127: error: Unexpected keyword argument "dirs_exist_ok" for "copytree"

Which makes me curious how you fixed the example you give, did you also upgrade to Python 3.8?

Regardless, I think the best path forward for us to maintain compatibility with 3.6 and 3.7 is to just implement our own copy/sync function to avoid the limitation in shutil.copytree. Something like this:

def copytree(src: Path, dst: Path) -> None:
    dst.mkdir(parents=True, exist_ok=True)
    for path in src.iterdir():
        if path.is_dir():
            copytree(path, dst / path.relative_to(src))
        else:
            shutil.copy2(path, dst / path.relative_to(src))

[jarekr]

Hey @lorencarvalho, yeah I realized my fix doesn't work as soon as all the checks failed and I saw the error. I am still not sure how it worked for me.

Another interesting thing is that on macos I don't see this problem, only on the linux (centos7) build hosts that we use.

I understand the motivation for the change, but wouldn't it be easier to tack on a non-existent dir to the path when pip_args is true? Maybe not since I can't seem to get a clean build.

[lorencarvalho]

Yeah, adding a trailing subdir here won't work because (as of 0.0.51) we are adding the 'site-packages' subdir at PYZ creation, here: https://github.com/linkedin/shiv/blob/master/src/shiv/builder.py#L89

So adding it beforehand doubles it, which breaks the bootstrapping process :/

[brianthelion]

Possibly helpful: Note that distutils has a copy_tree function that overcomes some of shutil.copytree's limitations.