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

Entrypoint script generation bug fix #936

Closed
wants to merge 1 commit into from
Closed

Conversation

antter
Copy link

@antter antter commented Dec 19, 2022

Some entrypoints can be weird, calling a method of an imported class rather than directly calling a function.

In the case of invoke, having this entrypoint, the generated file had a line saying from invoke.main import program.run, which raises a syntax error.

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?

A syntax error comes up if an entrypoint is a method of a class.

What is the new behavior?

No more syntax error.

Does this PR introduce a breaking change?

  • Yes
  • No

Other information

Some entrypoints can be weird, calling a method of an imported class rather than directly calling a function.

In the case of `invoke`, having [this entrypoint](https://github.com/pyinvoke/invoke/blob/8f6c0617c7dc59b105dd1b92fb417e75adc21bea/setup.py#L51), the generated file had a line saying `from invoke.main import program.run`, which raises a syntax error.
@google-cla
Copy link

google-cla bot commented Dec 19, 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).

View this failed invocation of the CLA check for more information.

For the most up to date status, view the checks section at the bottom of the pull request.

@alexeagle alexeagle self-requested a review December 19, 2022 20:43
Copy link
Contributor

@hrfuller hrfuller 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 your contribution! In order to get this merged there are a few things to take care of. First of all please add a test for this under python.pip_install.extract_wheels.bazel_test:BazelTestCase.

Also you can see that there are some tests failing in CI. These tests have generated files that are being compared verbatim. Those files need to be regenerated and committed into the repo. The failing test logs have some information on how to do that.

@chrislovecnm
Copy link
Collaborator

@antter knudge

@aignas
Copy link
Collaborator

aignas commented Sep 20, 2023

FYI, #1363 has included support for the case that fixes this, the correct way may be to just start using that rule instead.

@aignas aignas added the Can Close? Will close in 30 days if there is no new activity label Nov 5, 2023
Copy link

github-actions bot commented Dec 5, 2023

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

@github-actions github-actions bot closed this Dec 5, 2023
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