Skip to content

Conversation

@fvj
Copy link

@fvj fvj commented Aug 14, 2025

This PR adds partial (first issue raised) support for #11.

Caution

This is a breaking change. extra_files entries are now interpreted by glob.glob. Files that can be referred to without any escaping in UNIX shells are unaffected, but special characters are now interpreted. Thus, special characters need to be escaped using []; for example, a literal ? is to be expressed as [?].

More details on the semantics of entries in extra_files can be found here.

Changes

  1. Directories can be specified in extra_files. Directories are removed from the validation and are now considered valid inputs. When a directory is encountered, a suffix of /** is added, so the glob recursively picks up all files inside the directory.
  2. Any glob pattern (supported by glob.glob) is now supported inside the extra_files.
  3. For now, the glob uses recursive matches, so the operator ** is also supported. Hidden directories are not included. This enables nested patterns such as dir/**/prefix*.ext.

I'm not sure this is in line with the direction you want to take this package, it just was the functionality I needed for my project. So, please feel free to reject the PR/propose changes in the way things work.

Thanks for writing this Action!

@autarch
Copy link
Member

autarch commented Aug 16, 2025

Hi, thanks for your PR!

I'm pretty finicky about my projects (see this blog post for details), so I rarely merge a PR as-is.

For larger PRs, I typically use the GitHub PR review process and provide feedback directly on the code, asking you to make some changes. For smaller PRs, where I just want to tweak some small stuff (like doc wording, comments, code formatting, etc.), I don't do a PR review.

Whether or not I do a review, there are two options for merging the PR:

  1. I check your PR out locally, fiddle with it as needed, merge it locally, and simply close the PR on GitHub. This will preserve at least one commit with your name on it, but the PR will show up as closed in your GitHub stats.
  2. If you enable me to push directly to your PR branch (which is the default when you make a PR), I can do my fiddling, then force push to your PR branch and merge the resulting PR. Again, this will preserve at least one commit with your name on it, but you also get credit for the PR merge in your GitHub stats. The only downside is that I will be force pushing directly to your PR branch. Note that this will not work if the PR branch is named master. GitHub doesn't allow me to push to the default branch of your fork.

Please let me know which approach you'd prefer. If I don't hear from you before I get around to working on this PR I'll go with option 1.

Thanks again for your contribution!

@fvj
Copy link
Author

fvj commented Aug 16, 2025

I'm fine with either, though from a purely process-oriented perspective I'd prefer 2. Truth be told, I'm not concerned with having my name on something. You can remove my name from any code added in this PR, as far as I'm concerned.

Though I think before you do any work on it, I'd highlight again that there's a big semantic change in here: all extra_files are now globs. This might break existing userspace!

I would, and forgive me for giving unsolicited advice here, decide whether that's something I want in this package before I concern myself with the code. :)

@autarch
Copy link
Member

autarch commented Aug 17, 2025

Thanks for the heads up. My initial response is just a canned reply I give right away so that when I do get around to looking at the PR, I know what the submitter wants me to do if I plan to merge it.

For this one, I need to think about the implications of the change. And if I do decide to merge it, I would bump the major version for the next release.

@fvj
Copy link
Author

fvj commented Aug 18, 2025

Sounds good! Let me know if I can support in any way. Cheers!

@autarch autarch force-pushed the jvf/add-glob-support-to-extra-files branch from e0740a6 to 8c55620 Compare January 1, 2026 05:06
@autarch autarch changed the base branch from v0 to v1 January 1, 2026 05:07
@autarch autarch force-pushed the v1 branch 3 times, most recently from 0c924d9 to 232fa15 Compare January 1, 2026 05:31
@autarch autarch force-pushed the jvf/add-glob-support-to-extra-files branch from 8c55620 to 8d5e97b Compare January 2, 2026 12:41
@autarch autarch requested a review from Copilot January 2, 2026 12:46
Copy link

Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull request overview

This PR adds glob pattern support to the extra_files input, enabling users to specify file patterns and directories rather than just individual files. This is a breaking change as special characters in file paths will now be interpreted as glob patterns.

  • Removes validation that rejected directories in extra_files
  • Adds glob pattern expansion with support for recursive patterns (**)
  • Converts directory paths to recursive glob patterns (directory/**)

Reviewed changes

Copilot reviewed 3 out of 3 changed files in this pull request and generated 2 comments.

File Description
validate-inputs.py Removes directory validation error, allowing directories as valid extra_files entries
make-archive.py Implements glob expansion logic for extra_files and adds comprehensive test coverage
README.md Documents the new glob pattern functionality and breaking changes

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

All entries in `extra_files` are now interpreted as zglob-style globs. Additionally, directories can
be specified and will be expanded to a recursive glob under the directory. File path handling is the
same as it has always been.

Hidden files are not included in globs and must be spelled out explicitly.
@autarch autarch force-pushed the jvf/add-glob-support-to-extra-files branch from 8d5e97b to 41576f9 Compare January 2, 2026 12:50
@autarch autarch requested a review from Copilot January 3, 2026 01:42
Copy link

Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull request overview

Copilot reviewed 3 out of 3 changed files in this pull request and generated 3 comments.


💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Development

Successfully merging this pull request may close these issues.

2 participants