gh-117829: add --include and --exclude flag to zipapp #120537

VietThan · 2024-06-15T03:30:25Z

Hi, I'm a first time contributor to Python. I saw issue gh-117829 is interesting and took a stab at it. Currently it's a Work in Progress PR for others to see if my direction is good. Will add tests soon.

…829' into implement-issue-117829

godlygeek · 2024-08-19T17:40:25Z

Lib/test/test_zipapp.py

+        (source / 'bin' / 'baz').touch()
+        (source / '__main__.py').touch()
+
+        args = [str(source), '--include-pattern', r'.*\.py', '--exclude-pattern', r'.*z.*']


Can self.tmpdir contain a z? If so, I believe this exclude pattern would ignore all files (since AFAICS it's matched against absolute paths, not against paths relative to the source directory)

Thanks @godlygeek , note that this is fixed.

…829' into implement-issue-117829

erlend-aasland · 2024-11-05T07:40:11Z

Doc/library/zipapp.rst

   this case, any other options are ignored and SOURCE must be an archive, not a
   directory.

+.. option:: --include-pattern


Tian suggested --include and --exclude; what is the reason for the -pattern suffix?

I agree, --include-pattern is more verbose without adding any significant benefit (IMO).

Thanks for the feedback, I have addressed this suggestion

erlend-aasland · 2024-11-05T07:43:02Z

Lib/zipapp.py

+                   filter=None, compressed=False, include_pattern=None,
+                   exclude_pattern=None):


It should be possible to directly translate the --include and --exclude CLI options into a filter function, thus leaving the create_archive API unchanged, no?

Agreed, I'm a strong -1 on changing the create_archive API for this. The functionality is already available there, this should be purely a CLI change.

Thanks for your feedback, I have updated the PR accordingly

pfmoore · 2025-03-24T21:36:28Z

Lib/test/test_zipapp.py

        with zipfile.ZipFile(new_target, 'r') as z:
            self.assertEqual(set(z.namelist()), {'__main__.py'})

+    def test_create_archive_with_include_pattern(self):


This test isn't needed, as we shouldn't be changing create_archive.

thank you, i have addressed this

bedevere-app · 2025-09-27T03:46:35Z

Thanks for making the requested changes!

@pfmoore: please review the changes made to this pull request.

pfmoore · 2025-09-27T12:08:29Z

Doc/library/zipapp.rst

+   $ ls myapp
+   __main__.py helper.py   notthis.py
+
+   $ python -m zipapp myapp -o myapp.pyz --include "help*" --include "not*" --exclude "n*"


This example seems over-complicated. I'd suggest two examples, one showing --include (and specifically that anything not covered by the --include pattern is implicitly excluded), and one showing --exclude (demonstrating that everything except what's excluded gets added to the archive).

IMO the case where both --include and --exclude are used is too much of an edge case to warrant an example.

I've updated the examples as requested. Please let me know of other changes that are needed.

pfmoore · 2025-09-27T12:18:33Z

Doc/library/zipapp.rst

+
+   Accept glob-patterns filtering files to be denied inclusion in output archive. This will
+   run second if :option:`--include` is also used.
+


Remove the second sentence (explaining priorities) from these two descriptions, because it's not very clear. Instead, have a separate paragraph that explains how the options interact. Something like this:

If both --include and --exclude are specified, the files to be included are picked first. Then from that list of files, any to be excluded are removed. The order of the options does not affect how they are processed.

Also, add a note against --include saying "If this option is not specified, all files in the given directory are included by default."

I've made that "second paragraph" as a note. Please let me know what other things need to be changed.

I don't think these need to be notes, they are too visually imposing like that. Please make them ordinary text.

I have made the change, please let me know if I should change something else

pfmoore · 2025-09-27T12:22:27Z

Lib/zipapp.py


+def _normalize_patterns(values: Iterable[str] | None) -> list[str]:
+    """
+    Split comma-separated items, strip whitespace, drop empties.


This should not allow comma-separated items in one argument. Instead of --include a,b the user should use --include a --include b.

So I've chosen to interpret "should not allow comma-separated items in one argument" as in we'll take the patterns as-is and won't make any assumptions about commas actually have extra semantics. I've also made the change

unless you prefer that I throw a "ValueError" ?

I agree commas should have no special semantics.

pfmoore · 2025-09-27T12:33:32Z

Lib/zipapp.py

+            return False
+        return True
+
+    return _filter


This all feels way too complicated. We should just use the pathlib glob matching function, pth.match(). Something like:

If there are no --include or --exclude options, the filter is none.

Otherwise, filter(pth) should check whether pth.match(inc) for each include pattern. If none of them match, return False. Then, check pth.match(exc) against each exclude filter. If any of them match, return False. Otherwise, return True.

The documentation can simply state that patterns are standard glob patterns, as implemented by PurePath.match.

In particular, matches should not assume Posix path separators or case sensitivity. The pathlib functions handle this automatically, your current code doesn't.

And yes, this does mean that to match a directory and all of its contents, you need to use --include foo/**. I'm fine with that.

thank you for the thorough review. I was also hesitant at first about whether I wanted to implement all this complexities but I figured let's see if it's possible and let you/other reviewers see if it's appropriate. I've made the changes

bedevere-app · 2025-09-27T12:35:14Z

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 phrase I 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.

…, as implemented by PurePath.match

VietThan · 2025-09-28T02:08:25Z

I have made the requested changes; please review again

bedevere-app · 2025-09-28T02:08:30Z

Thanks for making the requested changes!

@pfmoore: please review the changes made to this pull request.

pfmoore · 2025-09-28T11:03:04Z

Doc/library/zipapp.rst

+
+   Accept glob-patterns filtering files to be denied inclusion in output archive. This will
+   run second if :option:`--include` is also used.
+


I don't think these need to be notes, they are too visually imposing like that. Please make them ordinary text.

pfmoore · 2025-09-28T11:04:41Z

Doc/library/zipapp.rst

+   * Patterns follow :class:`pathlib.PurePath.match`. To match all of a
+     directory contents, use ``dir/**``.
+   * Path separator handling is platform-specific. On POSIX, backslashes are
+     literals (``\`` does not act as a separator).


I don't think the second bullet is worth including. It probably causes more confusion than help.

I have removed the second bullet point

pfmoore · 2025-09-28T11:07:55Z

Lib/zipapp.py

+    # build a filter from include and exclude flags
+    filter_fn = None
+    src_path = pathlib.Path(args.source)
+    if src_path.exists() and src_path.is_dir() and (args.include or args.exclude):


Why are we only supporting --include and --exclude for directory sources? I don't see why they wouldn't be useful for archives as well. What's the motivation for this limitation?

Currently, if a file source is given, we do a byte-copy operation other than modifying the shebang (_copy_archive), so my implementation applied --include/--exclude only when we’re creating a new archive from a directory as I wasn't sure if applying filtering to an archive source is expected/unexpected. If we want those filters to work with an existing archive, that mode would no longer be a pure copy as we’d need to unzip, filter, then re-zip the contents.

That being said, I'm happy to implement a "repack with filtering" functionality. Or I could have the CLI error when an archive source has filters applied.

if os.path.isfile(args.source) and (args.include or args.exclude): raise SystemExit("--include/--exclude only apply when SOURCE is a directory")

Please let me know what you prefer and I'll update the PR accordingly @pfmoore .

I don't have a good instinct here. This is a problem that was introduced when we added the filter argument to create_archive, but which was not picked up then. IMO it's a bad UI for the filter argument to do nothing when copying an archive - but there's no practical use case that I'm aware of for filtering an existing archive.

I don't want to add extra complexity (that needs to be maintained, tested and supported) when no-one will actually use it, but equally I don't think we should have a UI that suggests an "obvious" usage (zipapp foo.pyz -o new_foo.pyz --exclude secret_data.txt) but doesn't actually implement it.

Honestly, at this point I'm wishing I'd stuck to my original principles and rejected the addition of the filter argument in the first place 🙁

Let's just raise an error when the source is an existing zipapp and --include/--exclude are specified. But please also add a similar error in the create_archive function. Also add a note to the command line help, and to the documentation of the create_archive function, explaining that when the source is an existing zipapp, the only supported functionality is to modify the shebang line (via the -p/--python command line argument, and the interpreter argument of the function, respectively).

pfmoore · 2025-09-28T11:09:19Z

Lib/zipapp.py

+        v = v.strip()
+        if v:
+            out.append(v)
+    return out


This function doesn't feel very useful. Why not just inline its logic into _make_glob_filter?

I have made it inline

pfmoore · 2025-09-28T11:11:19Z

Lib/zipapp.py

+    exc = _normalize_patterns(values=excludes)
+
+    if not inc and not exc:
+        return None


This can never happen, because we check in the caller that one of includes or excludes is present.

I have removed this check

pfmoore · 2025-09-28T11:12:09Z

Lib/zipapp.py

+      - Patterns are standard glob patterns as implemented by PurePath.match.
+      - If 'includes' is empty, all files/dirs are initially eligible.
+      - If any exclude pattern matches, the path is rejected.
+      - Matching respects the current platform's path flavor (separators, case).


You don't need the final bullet point, as it's implied by the use of PurePath.match.

I have removed the last bullet point

bedevere-app · 2025-09-28T11:12:56Z

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 phrase I 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.

VietThan · 2025-09-28T19:38:40Z

I have made the requested changes; please review again

bedevere-app · 2025-09-28T19:38:45Z

Thanks for making the requested changes!

@pfmoore: please review the changes made to this pull request.

pfmoore

See comments. We need to explicitly reject using the filter argument and --include/--exclude when the source is a file. We also need to document the limitation that the only thing you can do when the source is a file is change the shebang.

bedevere-app · 2025-10-11T13:05:44Z

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 phrase I 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.

VietThan added 2 commits June 14, 2024 23:02

pythongh-117829 : start of something new

9dd3619

pythongh-117829 : logic implementation

b887165

bedevere-app bot mentioned this pull request Jun 15, 2024

zippapp/ macOS: .DS_Store files should be excluded from the generated archives #117829

Open

VietThan and others added 19 commits June 14, 2024 23:50

pythongh-117829 : remove whitespace pre-commit was complaining about

23785b2

pythongh-117829: testing out manually exclude-pattern

3286657

pythongh-117829: add example to --exclude-pattern

c28771f

📜🤖 Added by blurb_it.

935a1a5

pythongh-117829: added News with blurb_it

60a9aef

pythongh-117829 : adding to documentation

ee2b185

pythongh-117829: failed test on Docs

aaed611

Merge branch 'main' into implement-issue-117829

1abe95c

pythongh-117829: failed tests on Docs

ac29c0e

Merge remote-tracking branch 'refs/remotes/origin/implement-issue-117…

6d0ca9a

…829' into implement-issue-117829

pythongh-117829: linting complains

408dd71

pythongh-117829: linting complains

fdcd64f

pythongh-117829 : gotta be careful with rst formatting

da75715

Merge branch 'main' into implement-issue-117829

c4b7cd1

pythongh-117829 : adding tests for zipapp module

ab215b7

Merge remote-tracking branch 'refs/remotes/origin/implement-issue-117…

a85989c

…829' into implement-issue-117829

pythongh-117829 : stop lint problems

e2e6b35

pythongh-117829 : adding cmdline tests

62dc347

pythongh-117829 : prevent linting

bff0a07

VietThan marked this pull request as ready for review August 17, 2024 03:39

bedevere-app bot added the awaiting review label Aug 17, 2024

Merge branch 'main' into implement-issue-117829

a2bc0bf

godlygeek reviewed Aug 19, 2024

View reviewed changes

VietThan added 2 commits August 22, 2024 21:13

pythongh-117829 : fix the refex

36ac9b0

Merge remote-tracking branch 'refs/remotes/origin/implement-issue-117…

0f077eb

…829' into implement-issue-117829

erlend-aasland reviewed Nov 5, 2024

View reviewed changes

pfmoore reviewed Mar 24, 2025

View reviewed changes

bedevere-app bot requested a review from pfmoore September 27, 2025 03:46

This comment was marked as off-topic.

Sign in to view

pfmoore requested changes Sep 27, 2025

View reviewed changes

bedevere-app bot added awaiting changes and removed awaiting change review labels Sep 27, 2025

VietThan added 3 commits September 27, 2025 16:47

pythongh-117829 : simplify logic, patterns are standard glob patterns…

28c425d

…, as implemented by PurePath.match

pythongh-117829 : updated docs based on feedback

3265a5a

pythongh-117829 : updated docs for options based on feedback

c67b6b1

bedevere-app bot added awaiting change review and removed awaiting changes labels Sep 28, 2025

bedevere-app bot requested a review from pfmoore September 28, 2025 02:08

pfmoore requested changes Sep 28, 2025

View reviewed changes

bedevere-app bot added awaiting changes and removed awaiting change review labels Sep 28, 2025

VietThan added 2 commits September 28, 2025 15:14

pythongh-117829 : updated docs for options based on feedback

617cf18

pythongh-117829 : small changes based on feedback

e08e765

bedevere-app bot added awaiting change review and removed awaiting changes labels Sep 28, 2025

bedevere-app bot requested a review from pfmoore September 28, 2025 19:38

pfmoore requested changes Oct 11, 2025

View reviewed changes

bedevere-app bot added awaiting changes and removed awaiting change review labels Oct 11, 2025

		filter=None, compressed=False, include_pattern=None,
		exclude_pattern=None):


		Accept glob-patterns filtering files to be denied inclusion in output archive. This will
		run second if :option:`--include` is also used.

Uh oh!

gh-117829: add --include and --exclude flag to zipapp #120537

Are you sure you want to change the base?

gh-117829: add --include and --exclude flag to zipapp #120537

Uh oh!

Conversation

VietThan commented Jun 15, 2024 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

bedevere-app bot commented Sep 27, 2025

Uh oh!

This comment was marked as off-topic.

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

bedevere-app bot commented Sep 27, 2025

Uh oh!

VietThan commented Sep 28, 2025

Uh oh!

bedevere-app bot commented Sep 28, 2025

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

VietThan commented Jun 15, 2024 •

edited

Loading