tarfile doesn't overwrite symlink by directory

[storchaka_gmail.com]

http://bugs.python.org/review/19974/diff/10333/Lib/tarfile.py
File Lib/tarfile.py (right):

http://bugs.python.org/review/19974/diff/10333/Lib/tarfile.py#newcode1999
Lib/tarfile.py:1999: drv, normalized_path = os.path.splitdrive(path)
Why these changes are needed? They are not look related.

http://bugs.python.org/review/19974/diff/10333/Lib/tarfile.py#newcode2002
Lib/tarfile.py:2002: # We don't want to join '/home/user' and 'home/user/file'
to avoid
Why not?

http://bugs.python.org/review/19974/diff/10333/Lib/tarfile.py#newcode2171
Lib/tarfile.py:2171: if os.path.exists(targetpath):
I think we should remove targetpath in all cases. Not only when softlink is
extracted.

http://bugs.python.org/review/19974/diff/10333/Lib/test/test_tarfile.py
File Lib/test/test_tarfile.py (right):

http://bugs.python.org/review/19974/diff/10333/Lib/test/test_tarfile.py#newco...
Lib/test/test_tarfile.py:1074: open(source_file, 'w').close()
Now source file is empty. It will be better make it nonempty and then check the
content of extracted file.

http://bugs.python.org/review/19974/diff/10333/Lib/test/test_tarfile.py#newco...
Lib/test/test_tarfile.py:1092: self.assertEqual(os.readlink(target_file),
source_file)
Add a check that content (or md5sum) of other_source_file is not changed.

Also needed a test for symlink to directory.

[vajrasky]

http://bugs.python.org/review/19974/diff/10333/Lib/tarfile.py
File Lib/tarfile.py (right):

http://bugs.python.org/review/19974/diff/10333/Lib/tarfile.py#newcode1999
Lib/tarfile.py:1999: drv, normalized_path = os.path.splitdrive(path)
On 2013/12/15 21:38:11, storchaka wrote:
> Why these changes are needed? They are not look related.

There is an underlying bug on Windows. Without these changes, the patch will not
work on Windows as it currently does not. Maybe we should make this bug as a
separate ticket. When I get to my Windows machine, I'll explain the bug.

http://bugs.python.org/review/19974/diff/10333/Lib/tarfile.py#newcode2002
Lib/tarfile.py:2002: # We don't want to join '/home/user' and 'home/user/file'
to avoid
On 2013/12/15 21:38:11, storchaka wrote:
> Why not?

I'll explain it later when I get back to my Windows machine.

http://bugs.python.org/review/19974/diff/10333/Lib/test/test_tarfile.py
File Lib/test/test_tarfile.py (right):

http://bugs.python.org/review/19974/diff/10333/Lib/test/test_tarfile.py#newco...
Lib/test/test_tarfile.py:1074: open(source_file, 'w').close()
On 2013/12/15 21:38:11, storchaka wrote:
> Now source file is empty. It will be better make it nonempty and then check
the
> content of extracted file.

Okay, I will.

http://bugs.python.org/review/19974/diff/10333/Lib/test/test_tarfile.py#newco...
Lib/test/test_tarfile.py:1092: self.assertEqual(os.readlink(target_file),
source_file)
On 2013/12/15 21:38:11, storchaka wrote:
> Add a check that content (or md5sum) of other_source_file is not changed.
> 
> Also needed a test for symlink to directory.

Okay, I will.

[storchaka_gmail.com]

http://bugs.python.org/review/19974/diff/10333/Lib/tarfile.py
File Lib/tarfile.py (right):

http://bugs.python.org/review/19974/diff/10333/Lib/tarfile.py#newcode1999
Lib/tarfile.py:1999: drv, normalized_path = os.path.splitdrive(path)
On 2014/01/16 08:21:52, vajrasky wrote:
> On 2013/12/15 21:38:11, storchaka wrote:
> > Why these changes are needed? They are not look related.
> 
> There is an underlying bug on Windows. Without these changes, the patch will
not
> work on Windows as it currently does not. Maybe we should make this bug as a
> separate ticket. When I get to my Windows machine, I'll explain the bug.

Yes, please open a separate ticket.

[Claudiu.Popa]

http://bugs.python.org/review/19974/diff/11106/Lib/test/test_tarfile.py
File Lib/test/test_tarfile.py (right):

http://bugs.python.org/review/19974/diff/11106/Lib/test/test_tarfile.py#newco...
Lib/test/test_tarfile.py:1204: tar = tarfile.open(temparchive,'w')
I guess the code could be simpler if you use context manager for tarfile:

with tarfile.open(..) as tar:
   ...

[Martin Panter]

https://bugs.python.org/review/19974/diff/11106/Lib/tarfile.py
File Lib/tarfile.py (right):

https://bugs.python.org/review/19974/diff/11106/Lib/tarfile.py#newcode50
Lib/tarfile.py:50: import errno
What is this needed for?

https://bugs.python.org/review/19974/diff/11106/Lib/tarfile.py#newcode2089
Lib/tarfile.py:2089: if os.path.exists(targetpath) and \
What about “lexists” for handling broken links? See
<https://bugs.python.org/issue12088>.