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.

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

Ban * globs and ! ignores in source: str field (Cherry-pick of #13629) #13636

Merged
merged 1 commit into from
Nov 16, 2021

Conversation

Eric-Arellano
Copy link
Contributor

There's no reason to support them. The SingleSourceField should be 0-1 file, so a glob would cause an error. While it is technically possible to have a glob that only matches one file, that's error-prone.

Enforcing this unblocks us from rendering a useful file name representing the target type in more places without needing to hydrate. For example, our streaming workunit handler code uses Address.filename, but that doesn't work with manually created per-file targets like python_source: #13627. This PR means that we can instead use SingleSourceField.file_path for the same effect.

[ci skip-rust]
[ci skip-build-wheels]

…antsbuild#13629)

There's no reason to support them. The `SingleSourceField` should be 0-1 file, so a glob would cause an error. While it is technically possible to have a glob that only matches one file, that's error-prone.

Enforcing this unblocks us from rendering a useful file name representing the target type in more places without needing to hydrate. For example, our streaming workunit handler code uses `Address.filename`, but that doesn't work with manually created per-file targets like `python_source`: pantsbuild#13627. This PR means that we can instead use `SingleSourceField.file_path` for the same effect.

[ci skip-rust]
[ci skip-build-wheels]

# Conflicts:
#	src/python/pants/engine/target_test.py
@Eric-Arellano Eric-Arellano merged commit 9857563 into pantsbuild:2.8.x Nov 16, 2021
@Eric-Arellano Eric-Arellano deleted the cp-ban-glob branch November 16, 2021 04:11
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants