Fix the --always-copy bug (with new unit test) #496
Conversation
…tead full path; use local copyfile() function to update file permissions
|
This is fairly critical bug (cannot use virtualenv on file system that doesn't support symlinks), please release this fix soon. |
|
marked for 1.11 |
| cp_or_ln(os.path.abspath(os.path.join(home_dir, subdir_name)), \ | ||
| os.path.join(local_path, subdir_name)) | ||
| copyfileordir(os.path.abspath(os.path.join(home_dir, subdir_name)), \ | ||
| os.path.join(local_path, subdir_name), symlink) |
There was a problem hiding this comment.
I don't follow this change.
can you give a reproducible case I can use to know what you're fixing here?
There was a problem hiding this comment.
i.e the IOError mentioned in #495 is fixed with the other one-line change, not this.
your commit for this part says "Fix —always-copy to work with "posix_local" install scheme platforms (Ubuntu)"
that what's need an explanation for.
There was a problem hiding this comment.
latter is just DRY fix, symlink check is redundant as it is already defined in copyfile. It looks that copyfileordir() should be changed to copyfile() though. Should I change or remove those at all?
There was a problem hiding this comment.
with the test suite being poor, It's hard to be sure, os.symlink will be equal to copyfile(symlink=True) for this scenario.
I'm inclined to say pull this part out, unless @pfmoore feels different. He added all the symlink kwargs to these functions.
|
closing, see PR #511, which is a minor adjustment to this which preserves 2 of the commits. |
This fixes the
IOErrorbug with mentioned in issues #495 and #344, I've also added a unit test.