Skip to content
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.

Sign up for GitHub

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

Jump to bottom

escape globs in generate_file_targets #15383

Merged
merged 5 commits into from
May 11, 2022
Merged

escape globs in generate_file_targets #15383

merged 5 commits into from
May 11, 2022

Conversation

caleblloyd
Copy link
Contributor

@kaos kaos added the category:bugfix Bug fixes for released features label May 10, 2022
@kaos kaos changed the title excape globs in generate_file_targets escape globs in generate_file_targets May 10, 2022
@caleblloyd
Copy link
Contributor Author

Is this the correct approach? Should globs even be allowed in file.source?

If there is a better way, could you point me in the right direction?

@caleblloyd
Copy link
Contributor Author

I have moved the glob escaping into target._generate_file_level_targets. From what I can tell, this is the main path for breaking a MultipleSourcesField field down into SingleSourceField

If all of the resolver for SingleSourceField support globbing, then this is probably the correct approach. It still feels odd to support globbing for a SingleSourceField, but from what I can tell the hydration path is not much different than a MultipleSourcesField so the glob escaping may be the best approach to prevent a large refactor

stuhood
stuhood approved these changes May 10, 2022
View reviewed changes
Copy link
Member

@stuhood stuhood left a comment

Choose a reason for hiding this comment

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

Thanks for the contribution!

src/python/pants/engine/target.py Outdated Show resolved Hide resolved
@stuhood stuhood added this to the 2.11.x milestone May 10, 2022
@stuhood stuhood enabled auto-merge (squash) May 10, 2022 21:26
Eric-Arellano and others added 2 commits May 10, 2022 17:22
@Eric-Arellano
Fix lint
597f485 
# Rust tests and lints will be skipped. Delete if not intended.
[ci skip-rust]

# Building wheels and fs_util will be skipped. Delete if not intended.
[ci skip-build-wheels]
@caleblloyd
use list instead of generator for type checker
76de2a2 
Signed-off-by: Caleb Lloyd <[email protected]>
auto-merge was automatically disabled May 11, 2022 14:21

Head branch was pushed to by a user without write access

@caleblloyd
Copy link
Contributor Author

I had to push an updated commit to fix a Type Checker error, could you please re-approve the workflow run? Thanks!

@stuhood
Copy link
Member

stuhood commented May 11, 2022

I had to push an updated commit to fix a Type Checker error, could you please re-approve the workflow run? Thanks!

Thanks for iterating! There was some minor confusion on our end because your branch is main on your fork, which kicked off extra CI runs on our end.

@Eric-Arellano
Copy link
Contributor

Thanks!

@Eric-Arellano Eric-Arellano merged commit 3beda5d into pantsbuild:main May 11, 2022
Eric-Arellano pushed a commit to Eric-Arellano/pants that referenced this pull request May 11, 2022
@caleblloyd @Eric-Arellano
Fix [] in file names with target generators (pantsbuild#15383)
3d7e729 
Fixes pantsbuild#15381.

[ci skip-rust]
# Building wheels and fs_util will be skipped. Delete if not intended.
[ci skip-build-wheels]
Eric-Arellano pushed a commit to Eric-Arellano/pants that referenced this pull request May 11, 2022
@caleblloyd @Eric-Arellano
Fix [] in file names with target generators (pantsbuild#15383)
8952d96 
Fixes pantsbuild#15381.

[ci skip-rust]
# Building wheels and fs_util will be skipped. Delete if not intended.
[ci skip-build-wheels]
Eric-Arellano added a commit that referenced this pull request May 11, 2022
@Eric-Arellano @caleblloyd
Fix [] in file names with target generators (Cherry-pick of #15383) (…
24cec54 
…#15402)

Fixes #15381.

[ci skip-rust]

Co-authored-by: Caleb Lloyd <[email protected]>
Eric-Arellano added a commit that referenced this pull request May 11, 2022
@Eric-Arellano @caleblloyd
Fix [] in file names with target generators (Cherry-pick of #15383) (…
2b40bcb 
…#15403)

Fixes #15381.

[ci skip-rust]

Co-authored-by: Caleb Lloyd <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@stuhood stuhood stuhood approved these changes

Assignees
No one assigned
Labels
category:bugfix Bug fixes for released features
Projects
None yet
Milestone
2.11.x
Development

Successfully merging this pull request may close these issues.

Unmatched glob when files matches filename with square brackets
4 participants
@caleblloyd @stuhood @Eric-Arellano @kaos