fix: use pathlib.Path.glob to include dotfiles in output manifests#1039
Draft
crowecawcaw wants to merge 1 commit intoaws-deadline:mainlinefrom
Draft
fix: use pathlib.Path.glob to include dotfiles in output manifests#1039crowecawcaw wants to merge 1 commit intoaws-deadline:mainlinefrom
crowecawcaw wants to merge 1 commit intoaws-deadline:mainlinefrom
Conversation
github-actions
Bot
added
the
waiting-on-maintainers
crowecawcaw
force-pushed
the
fix/glob-dotfiles-consistency
branch
from
a19d720 to
7a625cb
Compare
leongdl
reviewed
Mar 12, 2026
| Set of normalized file paths that match the patterns | ||
| """ | ||
| matched_files = set() | ||
| root = Path(base_path) |
Contributor
There was a problem hiding this comment.
I understand that this will work, but do we want to change this behavior? What if the customer's job checkout a git clone and now they get a whole new hidden git tree getting uploaded?
leongdl
reviewed
Mar 12, 2026
| matched_files.add(normalized_path) | ||
| for matched_path in root.glob(pattern): | ||
| if matched_path.is_file(): | ||
| matched_files.add(os.path.normpath(str(matched_path))) |
Contributor
There was a problem hiding this comment.
I did a quick search, it seems 3.11+ the default would support the hidden folders but < 3.11 the behavior depends. So I dont' think we want variation of behaviour on different versions of python.
leongdl
requested changes
Mar 12, 2026
Signed-off-by: Stephen Crowe <6042774+crowecawcaw@users.noreply.github.com>
crowecawcaw
force-pushed
the
fix/glob-dotfiles-consistency
branch
from
7a625cb to
9d04d87
Compare
|
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
What was the problem/requirement? (What/Why)
_match_files_with_patternin_glob.pyusedglob.glob(), which silently excludes dotfiles and dot-directories. The worker output sync inasset_sync.pyusespathlib.Path.glob(), which includes them. This inconsistency meant files like.gtn/and its contents would exist on disk but be missing from output manifests.What was the solution? (How)
Replaced
glob.glob()withpathlib.Path.glob()in_match_files_with_patternso both code paths handle dotfiles consistently.What is the impact of this change?
Output manifests generated via
_glob_pathswill now include dotfiles and files inside dot-directories. Users who were previously missing dotfiles in their synced outputs will now get them.How was this change tested?
downloadorasset_syncmodules? If so, then it is highly recommendedthat you ensure that the docker-based unit tests pass.
Added dotfile test fixtures (
.dotfile,.hidden_dir/inside_hidden.txt) and two new test cases intest_glob.py: one verifying dotfiles are included by default, one verifying they can be excluded via patterns. Updated existing test counts to reflect the new fixtures.Was this change documented?
Does this PR introduce new dependencies?
Is this a breaking change?
No. This is a bugfix. Output manifests will now include files that were previously silently dropped. No public API signatures changed.
Does this change impact security?
No. This change does not create, modify, or alter permissions on any files or directories.
By submitting this pull request, I confirm that you can use, modify, copy, and redistribute this contribution, under the terms of your choice.