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

Add support for long_description_path field in python_distribution #14448

Conversation

alexey-tereshenkov-oxb
Copy link
Contributor

@alexey-tereshenkov-oxb alexey-tereshenkov-oxb commented Feb 11, 2022

Work towards #11554

Closed #11554

The rule determine_setup_kwargs determines the kwargs that are either hardcoded in provides, or that are provided by the user's plugin for it. These SetupKwargs won't have everything, only the foundation for the FinalizedSetupKwargs.

We've decided to add the logic of extraction of the long_description file contents here, inside the rule generate_setup_py_kwargs that returns FinalizedSetupKwargs. That will make sure that this new logic works regardless of if you're using a plugin hook or not.

# 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]
Copy link
Contributor

@Eric-Arellano Eric-Arellano left a comment

Choose a reason for hiding this comment

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

This is looking great! Would you be willing to write a test for this? I'm definitely happy to pair program on how to do that :) writing tests can be tricky at first.

src/python/pants/backend/python/goals/setup_py.py Outdated Show resolved Hide resolved
src/python/pants/backend/python/goals/setup_py.py Outdated Show resolved Hide resolved
src/python/pants/backend/python/goals/setup_py.py Outdated Show resolved Hide resolved
src/python/pants/backend/python/target_types.py Outdated Show resolved Hide resolved
src/python/pants/backend/python/target_types.py Outdated Show resolved Hide resolved
# 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]
# 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]
Comment on lines +571 to +572
f"the {LongDescriptionPathField.alias} "
f"field of {exported_target.target.address}"
Copy link
Contributor Author

Choose a reason for hiding this comment

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

This is how the error message is shown when a non-existing file is passed:

Unmatched glob from the long_description_path field of src/python/foo:foo-dist: "src/python/foo/readme.md"

Copy link
Contributor

@Eric-Arellano Eric-Arellano left a comment

Choose a reason for hiding this comment

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

Yay! @jsirois if you're willing to wait, it would be great to include this in today's dev release.

@Eric-Arellano Eric-Arellano enabled auto-merge (squash) February 12, 2022 00:25
@Eric-Arellano
Copy link
Contributor

cc @asherf , I think this was your original feature request :)

@Eric-Arellano Eric-Arellano merged commit e75bef5 into pantsbuild:main Feb 12, 2022
@asherf
Copy link
Member

asherf commented Feb 12, 2022

YAY!!!!

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.

Add a long_description_path to python_distribution to read files when generating setup.py
3 participants