-
-
Notifications
You must be signed in to change notification settings - Fork 31.8k
Zipapp archives are always empty #130379
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
Comments
maybe related to #104527 ? |
Yeah, just noticed this too (after it caused an ongoing inadvertent production oopsie). Reproducer: https://github.com/akx/cpython-zipapp-bug I think #106076 was the PR breaking this. |
@swhcz Can you show your usage of |
Thanks for the reproducer. This bug wasn't caught by the existing test suite, unfortunately. The specific condition needed for this to fail are:
I agree that the change in #106076 is incorrect here. But IMO, the logic behind that change was what was flawed - rather than checking if we're trying to add the target to itself, we should collect the list of files to add before Please can you try the following patch to confirm if it fixes the issue for you?
|
@pfmoore 👍 That fixes my reproducer's assertion (because it removes the
However, I'm not sure if that issue ("don't self-zip", #104527 as referred to in #106076) in scope for this bug ("empty archives"). On that note, this also fixes one case of the other bug mentioned as a footnote in my reproducer's readme. Namely, one can now create an archive in the same source directory once, but rerunning the same command has zipapp hang forever compressing
|
IMO, that's correct behaviour. You have given #104527 would improve the behaviour in this area, by giving an error message rather than simply creating an ever-growing file, but that's an issue for the zipfile module, not for zipapp. I'd also be willing to accept a PR adding some form of Generally, though, I would strongly advise not creating the target in the source directory. |
Yeah, not disagreeing. Maybe the CLI could be made smart enough to default to a filter that excludes the target file if it inadvertently is among the sources, but if you use the library by hand, you need to do that yourself? |
I'm considering adding a check in the function to exclude Footnotes
|
Hi everyone, many thanks for looking into this!! @akx, yes, that's pretty much how my code looks. I have a small project that is packaged (in the same directory) to be copied to a RasPi as a .pyz-Package. While fiddling with the code I figured, leaving out the filter makes the whole packaging go bonkers, it fails with a file-too-large-exception when hitting the 4GB-mark, which can only mean using '.' as source causes something else to break, too, as there's not even remotely that many files in and below the current dir, meaning that large data comes from either a recursion or from outside. While I may agree creating a package in sourcedir could be a bad idea, it is not explicitly outruled, and using a filter (mine is actually checking for the package name, and returning false :) has worked just fine. Maybe the above behaviour is caused by using the source dir as target, haven't tested that. Here's a smaller version of my code for reference:
|
I've been thinking further about this. Now that we have a fix so that zipapp decides what files to include in the archive before it starts writing the file, the only way it can be writing to itself is if (an older version of) the output archive is part of the list of input files as specified. That's always a user error - there's no realistic reason for wanting a zipapp to contain an obsolete version of itself. Therefore, I'm now of the opinion that we shouldn't automatically ignore the target file. Instead, we should do the best we can to provide an informative error if the user makes this mistake. So I plan on doing a simple check - is the target file present in the list of source files (just a simple path equality check, no attempt to resolve symlinks or anything like that)? If so, then fail with an error explaining the issue. That will catch the common case, Anyone who wants to write the output archive to the directory containing the sources can still do so, but they will need to provide a filter argument to explicitly skip the output file (and they won't be able to use the basic CLI, as that doesn't support filters). IMO, that's a fair expectation for a situation that we'd advise against. I will update the PR with the fix to implement this approach. |
Fixed by #130509 |
…rget in itself (pythongh-130509) (cherry picked from commit 64ccbbb) Co-authored-by: Paul Moore <[email protected]>
Bug report
Bug description:
Good evening,
I have recently installed Python 3.13 (Windows x64) over 3.12.
Now
zipapp.create_archive(..)
is creating just empty archives.After some fiddling around I found there might be an issue in these lines:
(from zipapp.py, lines 113 ff)
This change was introduced in Python 3.13 (have verified against 3.12 sources) and seemingly causes the trouble.
My guess is, as
child
is pointing to the same file asarcname
, and naturally, they both resolve to the same file, no file gets added due to the!=
check, ever, to the archive-to-be-created.Maybe it was intended to check against
target
, notarcname
?Once I monkeypatched the
and child.resolve() != arcname.resolve()
part away, the archive got written again.Or am I just not doing it right?
Any help much appreciated!
Sebastian.
CPython versions tested on:
3.13
Operating systems tested on:
Windows
Linked PRs
The text was updated successfully, but these errors were encountered: