Skip to content

feat: add requires_file attr for py_wheel#644

Closed
weixiao-huang wants to merge 3 commits intobazel-contrib:mainfrom
weixiao-huang:feat/add-requires-file
Closed

feat: add requires_file attr for py_wheel#644
weixiao-huang wants to merge 3 commits intobazel-contrib:mainfrom
weixiao-huang:feat/add-requires-file

Conversation

@weixiao-huang
Copy link
Copy Markdown

@weixiao-huang weixiao-huang commented Mar 8, 2022

PR Checklist

Please check if your PR fulfills the following requirements:

  • Tests for the changes have been added (for bug fixes / features)
  • Docs have been added / updated (for bug fixes / features)

PR Type

What kind of change does this PR introduce?

  • Bugfix
  • Feature (please, look at the "Scope of the project" section in the README.md file)
  • Code style update (formatting, local variables)
  • Refactoring (no functional changes, no api changes)
  • Build related changes
  • CI related changes
  • Documentation content changes
  • Other... Please describe:

What is the current behavior?

Issue Number: N/A

What is the new behavior?

Does this PR introduce a breaking change?

  • Yes
  • No

Other information

@google-cla
Copy link
Copy Markdown

google-cla bot commented Mar 8, 2022

Thanks for your pull request! It looks like this may be your first contribution to a Google open source project. Before we can look at your pull request, you'll need to sign a Contributor License Agreement (CLA).

For more information, open the CLA check for this pull request.

@weixiao-huang weixiao-huang force-pushed the feat/add-requires-file branch 3 times, most recently from 0f95f75 to 28e48e7 Compare March 9, 2022 02:59
@pstradomski
Copy link
Copy Markdown
Contributor

Any background for why this is needed? Maybe the documentation should be updated to reflect that and what's the interaction with "requires" attribute?

@weixiao-huang
Copy link
Copy Markdown
Author

Because sometimes we may write codes like this in setup.py

#!/usr/bin/env python

from setuptools import setup, find_packages

with open("requirements.txt") as fp:
    install_requires = fp.read()


setup(
    # get requires from file
    install_requires=install_requires,
)

However, in py_wheel, it only provides requires from bazel string. It seems we cannot get requires from files. So I add requires_file in py_wheel.

If requires specified, requires_file will not work

@github-actions
Copy link
Copy Markdown

github-actions bot commented Sep 9, 2022

This Pull Request has been automatically marked as stale because it has not had any activity for 180 days. It will be closed if no further activity occurs in 30 days.
Collaborators can add an assignee to keep this open indefinitely. Thanks for your contributions to rules_python!

@github-actions github-actions bot added the Can Close? Will close in 30 days if there is no new activity label Sep 9, 2022
@weixiao-huang
Copy link
Copy Markdown
Author

Is there any update of this PR?

@github-actions github-actions bot removed the Can Close? Will close in 30 days if there is no new activity label Sep 10, 2022
@UebelAndre
Copy link
Copy Markdown
Contributor

@weixiao-huang would you be able to rebase this and address conflicts?

@weixiao-huang weixiao-huang force-pushed the feat/add-requires-file branch 2 times, most recently from 35abc61 to feba774 Compare October 11, 2022 09:27
@weixiao-huang weixiao-huang force-pushed the feat/add-requires-file branch from feba774 to 458d28c Compare October 11, 2022 09:28
Copy link
Copy Markdown
Contributor

@UebelAndre UebelAndre left a comment

Choose a reason for hiding this comment

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

I'm no maintainer but did have some requests 😅

@weixiao-huang weixiao-huang force-pushed the feat/add-requires-file branch from cd7edd7 to aa3beac Compare October 12, 2022 15:03
if arguments.requires_file:
with open(arguments.requires_file) as fp:
additive_requires_list = ["Requires-Dist: {}".format(line) for line in fp.read().strip().split("\n")]
metadata += "\n" + "\n".join(additive_requires_list)
Copy link
Copy Markdown
Contributor

@pstradomski pstradomski Oct 13, 2022

Choose a reason for hiding this comment

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

Won't that leave you with two \n in a row, so everything below will be interpreted as a description?

Can we have a test?


if arguments.requires_file:
with open(arguments.requires_file) as fp:
additive_requires_list = ["Requires-Dist: {}".format(line) for line in fp.read().strip().split("\n")]
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

why read().strip().split() and not readlines()?

@github-actions
Copy link
Copy Markdown

This Pull Request has been automatically marked as stale because it has not had any activity for 180 days. It will be closed if no further activity occurs in 30 days.
Collaborators can add an assignee to keep this open indefinitely. Thanks for your contributions to rules_python!

@github-actions github-actions bot added the Can Close? Will close in 30 days if there is no new activity label Apr 11, 2023
@chrislovecnm
Copy link
Copy Markdown
Contributor

@weixiao-huang knudge

@github-actions github-actions bot removed the Can Close? Will close in 30 days if there is no new activity label Apr 24, 2023
@weixiao-huang
Copy link
Copy Markdown
Author

Sorry, I'll check it recently

@github-actions
Copy link
Copy Markdown

This Pull Request has been automatically marked as stale because it has not had any activity for 180 days. It will be closed if no further activity occurs in 30 days.
Collaborators can add an assignee to keep this open indefinitely. Thanks for your contributions to rules_python!

@github-actions github-actions bot added the Can Close? Will close in 30 days if there is no new activity label Oct 22, 2023
@github-actions
Copy link
Copy Markdown

This PR was automatically closed because it went 30 days without a reply since it was labeled "Can Close?"

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Can Close? Will close in 30 days if there is no new activity

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants