fix 'dir already exists' when supplying both pip_args and site_packages#116
fix 'dir already exists' when supplying both pip_args and site_packages#116jarekr wants to merge 4 commits intolinkedin:masterfrom
Conversation
|
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, 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 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: |
|
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. |
|
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 :/ |
…opytree Implements a fix for #116
|
Possibly helpful: Note that |
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.