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

bpo-41643: always support pathlib in shutil.make_archive #21962

Open
wants to merge 3 commits into
base: main
Choose a base branch
from

Conversation

graingert
Copy link
Contributor

@graingert graingert commented Aug 26, 2020

Lib/shutil.py Outdated Show resolved Hide resolved
@@ -1030,8 +1030,10 @@ def make_archive(base_name, format, root_dir=None, base_dir=None, verbose=0,
uses the current owner and group.
"""
Copy link
Contributor Author

@graingert graingert Aug 26, 2020

Choose a reason for hiding this comment

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

not sure what the rules are re-auditing, would it be better to do:

base_name = base_name ?? os.fsdecode(base_name)
root_dir = root_dir ?? os.fsdecode(root_dir)
base_dir = base_dir ?? os.fsdecode(base_dir)

before the audit call or after?

Copy link
Member

@merwok merwok Jan 5, 2022

Choose a reason for hiding this comment

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

What do you mean by ?? ?

Copy link
Contributor Author

@graingert graingert Jun 22, 2022

Choose a reason for hiding this comment

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

https://peps.python.org/pep-0505/ syntax, but it's not used should be something like

base_name = base_name if base_name is None else os.fsdecode(base_name) 
root_dir = root_dir if root_dir is None else os.fsdecode(root_dir)
base_dir = base_dir if base_dir is None else os.fsdecode(base_dir)

@ambv ambv requested a review from zooba Aug 27, 2020
@graingert
Copy link
Contributor Author

graingert commented Sep 14, 2020

@zooba @ambv should all the shutil stuff pass every filename arg through os.fsdecode?

@@ -1030,8 +1030,10 @@ def make_archive(base_name, format, root_dir=None, base_dir=None, verbose=0,
uses the current owner and group.
"""
sys.audit("shutil.make_archive", base_name, format, root_dir, base_dir)
base_name = os.fsdecode(base_name)
Copy link
Member

@merwok merwok Jan 5, 2022

Choose a reason for hiding this comment

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

Isn’t fsdecode about bytes-str decoding, and fspath for pathlike-to-OS-native conversion?

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.

None yet

6 participants