Skip to content

Conversation

@graingert
Copy link
Contributor

@graingert graingert commented Aug 26, 2020

@@ -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

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

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)

Copy link
Member

Choose a reason for hiding this comment

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

meaning is not None I assume!

@ambv ambv requested a review from zooba August 27, 2020 00:41
@graingert
Copy link
Contributor Author

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

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

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?

@tadejmagajna
Copy link
Contributor

Hi @graingert. The make_archive code has changed a bit since this PR was opened in 2020 and the proposed changes here are not as applicable anymore.

Would you be okay for this PR to be closed so that I can take this over (I have a complete fix+tests+docs ready) or are you still working on this?

@graingert
Copy link
Contributor Author

Please go ahead

@graingert graingert closed this Jan 7, 2026
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.

8 participants