Skip to content

bpo-12800: tarfile: Restore fix from 011525ee9 #21409

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 3 commits into from
Nov 25, 2020

Conversation

JulienPalard
Copy link
Member

@JulienPalard JulienPalard commented Jul 9, 2020

While reviewing #20972 I spotted the code already existed, the test already existed, but was not seeing the error due to:

  • The test not testing the actual condition of having the extracted symlink where a symlink already existed
  • The test was running with errorlevel=1 and expecting an exception, while errorlevel=1 does log instead of raising for this error.

https://bugs.python.org/issue12800

@JulienPalard JulienPalard changed the title bpo-12800: tarfile: Restore fix from 011525ee92eb1c13ad1a62d28725a840e28f8160 bpo-12800: tarfile: Restore fix from 011525ee9 Jul 9, 2020
@taleinat
Copy link
Contributor

Closing and re-opening to restart the Travis-CI check.

@@ -2232,6 +2232,8 @@ def makelink(self, tarinfo, targetpath):
try:
# For systems that support symbolic and hard links.
if tarinfo.issym():
if os.path.lexists(targetpath):
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@JulienPalard, perhaps include a short comment here explaining why this is needed? Something along the lines of "tar should overwrite existing files with symlinks, but os.symlink raises an exception rather than overwriting."

Copy link
Contributor

@taleinat taleinat left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This LGTM, just waiting on the possible addition of a code comment.

@JulienPalard JulienPalard merged commit 4fedd71 into python:master Nov 25, 2020
@miss-islington
Copy link
Contributor

Thanks @JulienPalard for the PR 🌮🎉.. I'm working now to backport this PR to: 3.8, 3.9.
🐍🍒⛏🤖

miss-islington pushed a commit to miss-islington/cpython that referenced this pull request Nov 25, 2020
Restore fix from 011525e.
(cherry picked from commit 4fedd71)

Co-authored-by: Julien Palard <julien@palard.fr>
@bedevere-bot
Copy link

GH-23508 is a backport of this pull request to the 3.9 branch.

@bedevere-bot bedevere-bot removed the needs backport to 3.9 only security fixes label Nov 25, 2020
miss-islington pushed a commit to miss-islington/cpython that referenced this pull request Nov 25, 2020
Restore fix from 011525e.
(cherry picked from commit 4fedd71)

Co-authored-by: Julien Palard <julien@palard.fr>
@bedevere-bot
Copy link

GH-23509 is a backport of this pull request to the 3.8 branch.

miss-islington added a commit that referenced this pull request Nov 25, 2020
Restore fix from 011525e.
(cherry picked from commit 4fedd71)

Co-authored-by: Julien Palard <julien@palard.fr>
miss-islington added a commit that referenced this pull request Nov 25, 2020
Restore fix from 011525e.
(cherry picked from commit 4fedd71)

Co-authored-by: Julien Palard <julien@palard.fr>
@JulienPalard JulienPalard deleted the tarfile-symlink branch November 25, 2020 13:44
adorilson pushed a commit to adorilson/cpython that referenced this pull request Mar 13, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants