Skip to content
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

gh-117084: Fix zip extraction bug #117085

Closed

Conversation

fliiiix
Copy link

@fliiiix fliiiix commented Mar 20, 2024

On Windows a directory is ending in \ and not /.

This is the smallest fix i could think of.
Not sure if there are edge cases with files containing \\ at the end.

On Windows a directory is ending in \\ and not /.
Copy link

cpython-cla-bot bot commented Mar 20, 2024

All commit authors signed the Contributor License Agreement.
CLA signed

@bedevere-app
Copy link

bedevere-app bot commented Mar 20, 2024

Most changes to Python require a NEWS entry. Add one using the blurb_it web app or the blurb command-line tool.

If this change has little impact on Python users, wait for a maintainer to apply the skip news label instead.

@serhiy-storchaka
Copy link
Member

Thank you for your contribution @fliiiix. I have two requirements:

  • It should not change the behavior on non-Windows platforms by default. The logic should match the logic of the extraction code.
  • Tests are required.

Creating specific tests is something non-trivial task, so instead of guiding you to find the right solution (which I myself didn't know at first), it was easier for me to do it myself (#117129). Anyway, thank you. I hope for fruitful cooperation in the future.

@fliiiix
Copy link
Author

fliiiix commented Mar 22, 2024

so instead of guiding you to find the right solution

Did you just denied my first contribution to cpython? 😢

@serhiy-storchaka
Copy link
Member

I'm sorry. But now that you've taken the first steps, cloned the repository, and created your first PR, creating other PRs should be much easier, so I look forward to your contribution in the future.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants