Extract ZipInfo for archive functionality

In #123354, I found the need to copy code from zipfile into the test:

cpython/Lib/test/test_zipfile/_path/test_path.py

Lines 662 to 676 in 9e108b8

    
               def for_name(cls, name, archive): 
        
                   """ 
        
                   Construct the same way that ZipFile.writestr does. 
        
                   TODO: extract this functionality and re-use 
        
                   """ 
        
                   self = cls(filename=name, date_time=time.localtime(time.time())[:6]) 
        
                   self.compress_type = archive.compression 
        
                   self.compress_level = archive.compresslevel 
        
                   if self.filename.endswith('/'):  # pragma: no cover 
        
                       self.external_attr = 0o40775 << 16  # drwxrwxr-x 
        
                       self.external_attr |= 0x10  # MS-DOS directory flag 
        
                   else: 
        
                       self.external_attr = 0o600 << 16  # ?rw------- 
        
                   return self

Let's instead extract that functionality in the zipfile module for re-use in the test (before it starts diverging).

Linked PRs

gh-123424: add ZipInfo._for_archive to set suitable default properties #123429

@jaraco

@jaraco are you taking it or not? if not, I can take it

Originally, I was planning to do it, but then I realized it might be an interesting one for others. Have at it!

See also #121402 that implements a similar feature.

I paused it because I am planning to try a different approach.

Nothing against extracting the functionality if it's useful elsewhere, but the test doesn't actually depend on the compression or any of the metadata set by for_name(), just the file name. Nothing else should matter to zipfile.Path here. So removing for_name() and just using zf.writestr(DirtyZipInfo("foo\\bar"), b"content") should work fine.

When I added the for_name to DirtyZipInfo, I was aiming to replace writestr without altering the way it worked. That is, every other test relies on writestr, so I wanted to construct an equivalent ZipInfo, assuming that whoever wrote writestr had reasons for defaulting the date_time, compression type, compression level, and external attrs. I wanted to avoid having to research the motivations for each of those concerns to see if they may apply to this specific case. I wanted to avoid the possibility that those attributes had a meaningful effect or worse did not have an effect today but would in the future. More importantly, I wanted to be sure that if writestr changed in the future to add additional behavior, this routine would inherit that same behavior. By reducing incidental variance, we improve reliability long-term.

Passing a ZipInfo to writestr() will not set any of those attributes like passing only a filename (and thus having writestr() create the ZipInfo) will and is clearly supported as part of the current API. I rely on this regularly for Reproducible Builds where I need to set the attributes to specific values and using the current time would definitely be unacceptable.

And the fact that writestr() by default uses the current time means no two test runs ever use the exact same metadata, which is something I would generally try to avoid.

That said, even if I very much doubt any of the metadata would ever affect this specific test, and would consider that changing to be a bug, I agree that ensuring consistency wrt to writestr() behaviour between different tests is good. Just wanted to clarify the current situation :)

Removing my assignment on this task since I didn't continue working on the associated PR (and have to prioritize my backlog).

python3-zipp: upgrade 3.21.0 -> 3.23.0

Navigation Menu

Repository navigation

Extract ZipInfo for archive functionality #123424

Linked PRs

12 remaining items

Metadata

Assignees

Labels

Projects

Milestone

Relationships

Development

	def for_name(cls, name, archive):
	"""
	Construct the same way that ZipFile.writestr does.

	TODO: extract this functionality and re-use
	"""
	self = cls(filename=name, date_time=time.localtime(time.time())[:6])
	self.compress_type = archive.compression
	self.compress_level = archive.compresslevel
	if self.filename.endswith('/'): # pragma: no cover
	self.external_attr = 0o40775 << 16 # drwxrwxr-x
	self.external_attr \|= 0x10 # MS-DOS directory flag
	else:
	self.external_attr = 0o600 << 16 # ?rw-------
	return self

Extract ZipInfo for archive functionality #123424

Description

Linked PRs

Activity

picnixz commented on Aug 28, 2024

jaraco commented on Aug 28, 2024

serhiy-storchaka commented on Aug 28, 2024

obfusk commented on Sep 2, 2024

jaraco commented on Sep 2, 2024

obfusk commented on Sep 2, 2024

picnixz commented on Dec 22, 2024

12 remaining items

Metadata

Metadata

Assignees

Labels

Projects

Milestone

Relationships

Development

Issue actions