Movatterモバイル変換


[0]ホーム

URL:


Skip to content

Navigation Menu

Sign in
Appearance settings

Search code, repositories, users, issues, pull requests...

Provide feedback

We read every piece of feedback, and take your input very seriously.

Saved searches

Use saved searches to filter your results more quickly

Sign up
Appearance settings

gh-87264: tarfile list() method does not show file type.#29974

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 ourterms of service andprivacy statement. We’ll occasionally send you account related emails.

Already on GitHub?Sign in to your account

Closed

Conversation

@val-shkolnikov
Copy link
Contributor

@val-shkolnikovval-shkolnikov commentedDec 7, 2021
edited by bedevere-bot
Loading

The list() method of TarFile tarfile object shows question mark in place of file type in verbose mode. For instance
python3 -m tarfile -v -l my.tar
?rwxr-xr-x vals/users 0 2021-01-16 18:28:30 bin/
?rwxr-x--- vals/users 266 2021-01-16 18:28:30 bin/backmine.sh
?rwxr-xr-x vals/users 950 2018-01-02 10:39:18 bin/tm.sh
?rwxrwxrwx vals/users 0 2021-01-16 11:49:36 bin/source-highlight -> ../source-highlight/bin/source-highlight
?rwxr-xr-x vals/users 435 2011-03-27 15:38:06 bin/dd.sh

This is because the call stat.filemode(tarinfo.mode) does not pass tarinfo.type to stat.filemode. The type needs to be properly encoded and added to the argument.

@the-knights-who-say-ni

Hello, and thanks for your contribution!

I'm a bot set up to make sure that the project can legally accept this contribution by verifying everyone involved has signed thePSF contributor agreement (CLA).

Recognized GitHub username

We couldn't find abugs.python.org (b.p.o) account corresponding to the following GitHub usernames:

@val-shkolnikov

This might be simply due to a missing "GitHub Name" entry in one's b.p.o account settings. This is necessary for legal reasons before we can look at this contribution. Please followthe steps outlined in the CPython devguide to rectify this issue.

You cancheck yourself to see if the CLA has been received.

Thanks again for the contribution, we look forward to reviewing it!

@github-actions
Copy link

This PR is stale because it has been open for 30 days with no activity.

@github-actionsgithub-actionsbot added the staleStale PR or inactive for long period of time. labelJan 7, 2022
@MaxwellDupre
Copy link
Contributor

Could you change the title for conformance: 'bpo' also the PR number? Also, please check why checks are failing, e.g. why no news?

Copy link
Member

@iritkatrieliritkatriel left a comment

Choose a reason for hiding this comment

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

Please add a unit test covering this change.

@bedevere-bot
Copy link

A Python core developer has requested some changes be made to your pull request before we can consider merging it. If you could please address their requests along with any other requests in other reviews from core developers that would be appreciated.

Once you have made the requested changes, please leave a comment on this pull request containing the phraseI have made the requested changes; please review again. I will then notify any core developers who have left a review that you're ready for them to take another look at this pull request.

@ethanfurman
Copy link
Member

@iritkatriel How do we check if the CLA has been signed?

@iritkatriel
Copy link
Member

@iritkatriel How do we check if the CLA has been signed?

that should be automated.

@bedevere-bot
Copy link

Most changes to Pythonrequire a NEWS entry.

Please add it using theblurb_it web app or theblurb command-line tool.

@ghost
Copy link

ghost commentedNov 26, 2022
edited by ghost
Loading

The following commit authors need to sign the Contributor License Agreement:

Click the button to sign:
CLA not signed

@val-shkolnikov
Copy link
ContributorAuthor

val-shkolnikov commentedNov 27, 2022
edited
Loading

The following commit authors need to sign the Contributor License Agreement:

* [val.shkolnikov@broadcom.net](mailto:val.shkolnikov@broadcom.net)

This is an incorrect email. My email isval@nvsoft.net. I signed the CLA with that email.

@iritkatriel
Copy link
Member

The following commit authors need to sign the Contributor License Agreement:

* [val.shkolnikov@broadcom.net](mailto:val.shkolnikov@broadcom.net)

This is an incorrect email. My email isval@nvsoft.net. I signed the CLA with that email.

The bot picked up this email from somewhere. I can think of two easy solutions - sign the CLA again with that email, or close this PR and create a new one with the email you used to sign the CLA.

@iritkatriel
Copy link
Member

Please add a unit test covering this change.

The tests for the tar file module are athttps://github.com/python/cpython/blob/main/Lib/test/test_tarfile.py

@val-shkolnikov
Copy link
ContributorAuthor

Please add a unit test covering this change.

The tests for the tar file module are athttps://github.com/python/cpython/blob/main/Lib/test/test_tarfile.py

I modified the related test. The patch is attached.
test_patch.txt

@val-shkolnikov
Copy link
ContributorAuthor

The following commit authors need to sign the Contributor License Agreement:

* [val.shkolnikov@broadcom.net](mailto:val.shkolnikov@broadcom.net)

This is an incorrect email. My email isval@nvsoft.net. I signed the CLA with that email.

The bot picked up this email from somewhere. I can think of two easy solutions - sign the CLA again with that email, or close this PR and create a new one with the email you used to sign the CLA.

If I close the PR and start a new one, do I need to re-submit all the material from scratch?

@iritkatriel
Copy link
Member

Please add a unit test covering this change.

The tests for the tar file module are athttps://github.com/python/cpython/blob/main/Lib/test/test_tarfile.py

We don't accept patches as attachments anymore. You need to include it in the PR.

@val-shkolnikov
Copy link
ContributorAuthor

val-shkolnikov commentedNov 28, 2022
edited by terryjreedy
Loading

Please add a unit test covering this change.

The tests for the tar file module are athttps://github.com/python/cpython/blob/main/Lib/test/test_tarfile.py

We don't accept patches as attachments anymore. You need to include it in the PR.

--- a/test_tarfile.py   2022-11-27 15:49:38.038411000 -0800+++ b/test_tarfile.py   2022-11-27 15:49:49.286715000 -0800@@ -302,11 +302,23 @@ class ListTest(ReadTest, unittest.TestCa         # accessories if verbose flag is being used         # ...         # ?rw-r--r-- tarfile/tarfile     7011 2003-01-06 07:19:43 ustar/conttype-        # ?rw-r--r-- tarfile/tarfile     7011 2003-01-06 07:19:43 ustar/regtype+        # -rw-r--r-- tarfile/tarfile     7011 2003-01-06 07:19:43 ustar/regtype+        # drwxr-xr-x tarfile/tarfile        0 2003-01-05 15:19:43 ustar/dirtype/         # ...-        self.assertRegex(out, (br'\?rw-r--r-- tarfile/tarfile\s+7011 '-                               br'\d{4}-\d\d-\d\d\s+\d\d:\d\d:\d\d '-                               br'ustar/\w+type ?\r?\n') * 2)+        #+        # Array of values to modify the regex below:+        #  ((file_type, file_permissions, file_length), ...)+        type_perm_lengths = (+            (r'\?', 'rw-r--r--', '7011'), ('-', 'rw-r--r--', '7011'),+            ('d', 'rwxr-xr-x', '0'), ('d', 'rwxr-xr-x', '255'),+            (r'\?', 'rw-r--r--', '0'), ('l', 'rwxrwxrwx', '0'),+            ('b', 'rw-rw----', '3,0'), ('c', 'rw-rw-rw-', '1,3'),+            ('p', 'rw-r--r--', '0'))+        self.assertRegex(out.decode(), ''.join(+            [(tp + (r'%s tarfile/tarfile\s+%s ' % (perm, ln) ++                    r'\d{4}-\d\d-\d\d\s+\d\d:\d\d:\d\d '+                    r'ustar/\w+type[/>\sa-z-]*\n')) for tp, perm, ln+                    in type_perm_lengths]))         # Make sure it prints the source of link with verbose flag         self.assertIn(b'ustar/symtype -> regtype', out)         self.assertIn(b'./ustar/linktest2/symtype -> ../linktest1/regtype', out)

@iritkatriel
Copy link
Member

Please go over the ‘collaborative coding’ section of the GitHub documentation to see how it works:https://docs.github.com/en

@terryjreedyterryjreedy changed the titleIssue43098: tarfile list() method does not show file type.gh-87264: tarfile list() method does not show file type.Nov 28, 2022
@terryjreedy
Copy link
Member

Issue numbers on PRs need to be disambiguated with abpo- orgh- prefix. The initialissue# was a bpo number. I replaced it, with a prefix, with the new github issue number.

Because there are usually over 100 blurbs per release, we try to limit them to a line or two. Details, like the example, belong on the issue.

The test revision needs to be added to the PR by switching to your tarfile-fix branch, editing test_tarfile, saving it, and running the test suite locally to test that the test runs on your machine without breaking anything. If it passes, commit it and push the new commit to your fork, as you did to make the PR. The new commit should show up here. This should not affect my revision of the blurb. Do this a few times and revising PRs will become routine.

@ethanfurman
Copy link
Member

@val-shkolnikov : the CLA is still an issue. I would suggest just signing it again with the email address tied to the account you are using for PRs.

@val-shkolnikov
Copy link
ContributorAuthor

@val-shkolnikov : the CLA is still an issue. I would suggest just signing it again with the email address tied to the account you are using for PRs.

Unfortunately I cannot do that. This patch dates way back. Broadcom changed their email to go to broadcom.com instead of broadcom.net, so this email address is not valid anymore.

If I create a new PR, is there a way to reference this PR in there or have some method to make sure that the new PR gets reviewed by the same people?

@ethanfurman
Copy link
Member

When you create the new PR, post a comment here with the number, then anyone following can click through to it.

@ethanfurman
Copy link
Member

Automated systems failed us on this one. See above PR for current attempt.

Sign up for freeto join this conversation on GitHub. Already have an account?Sign in to comment

Reviewers

@iritkatrieliritkatrieliritkatriel requested changes

@ethanfurmanethanfurmanAwaiting requested review from ethanfurmanethanfurman is a code owner

Assignees

No one assigned

Labels

awaiting changesstaleStale PR or inactive for long period of time.

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

9 participants

@val-shkolnikov@the-knights-who-say-ni@MaxwellDupre@bedevere-bot@ethanfurman@iritkatriel@terryjreedy@ezio-melotti@AlexWaygood

[8]ページ先頭

©2009-2025 Movatter.jp