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

Simplify excluding the requirements.txt file generated from python_requirements() #9640

Merged
merged 2 commits into from
Apr 28, 2020

Conversation

Eric-Arellano
Copy link
Contributor

@Eric-Arellano Eric-Arellano commented Apr 27, 2020

We never want to include the generated requirements.txt created by python_requirements().

Previously, we had to explicitly exclude it because we had PythonRequirementsFileSources subclass FilesSources. There was no good reason for this. Instead, it can simply subclass Sources so be a stand-alone type.

[ci skip-jvm-tests]
[ci skip-rust-tests]

# Delete this line to force CI to run Clippy and the Rust tests.
[ci skip-rust-tests]
# Delete this line to force CI to run the JVM tests.
[ci skip-jvm-tests]
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.

Looks good!

One typo in the description probably:

Instead, it can simply subclass PythonRequirementsFileSources so be a stand-alone type.

@Eric-Arellano Eric-Arellano merged commit 8e5003b into pantsbuild:master Apr 28, 2020
@Eric-Arellano Eric-Arellano deleted the python-reqs branch April 28, 2020 03:20
Eric-Arellano added a commit that referenced this pull request Mar 17, 2021
…ugins (#11712)

The `PythonRequirementsFileSources` is like a `files()` target in that its source roots should not be stripped. However, in #9640, we "fixed" it to not subclass `FilesSources` because this was causing lots of issues with the file being unintentionally included in chroots. Yet, as a result, using `await Get(StrippedSourceFiles)` will try to strip the file, unless the plugin author uses `for_sources_type` and leaves off the type in the allowlist.

This PR allows us to properly handle both use cases by generifying the mechanism for "unrooted sources". Now, any `Sources` subclass can express whether it uses source roots, i.e. it's no longer hardcoded to `FilesSources`.

[ci skip-rust]
[ci skip-build-wheels]
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