Skip to content

rubocops/lines: allow a runtime Python when supporting multiple Pythons#16119

Closed
cho-m wants to merge 1 commit intoHomebrew:masterfrom
cho-m:py-deps-rubocop
Closed

rubocops/lines: allow a runtime Python when supporting multiple Pythons#16119
cho-m wants to merge 1 commit intoHomebrew:masterfrom
cho-m:py-deps-rubocop

Conversation

@cho-m
Copy link
Copy Markdown
Member

@cho-m cho-m commented Oct 18, 2023

  • Have you followed the guidelines in our Contributing document?
  • Have you checked to ensure there aren't other open Pull Requests for the same change?
  • Have you added an explanation of what your changes do and why you'd like us to include them?
  • Have you written new tests for your changes? Here's an example.
  • Have you successfully run brew style with your changes locally?
  • Have you successfully run brew typecheck with your changes locally?
  • Have you successfully run brew tests with your changes locally?

Draft for now as unable to test out on Sonoma and not too sure about RuboCop logic. Maybe there is better way to handle.

Main thing is to handle recent changes to various formulae where multiple Python libraries were built and runtime dependency was dropped resulting in non-functioning executables installed/linked into user PATH, e.g. pyinvoke (Homebrew/homebrew-core#151248), python-tabulate, python-argcomplete, etc...

Comment thread Library/Homebrew/rubocops/lines.rb Outdated
@cho-m cho-m marked this pull request as ready for review October 19, 2023 18:43
@Bo98
Copy link
Copy Markdown
Member

Bo98 commented Oct 20, 2023

This makes sense in isolation.

Though it does seem problematic when we have any dependents using it as we'll get mixed runtime Pythons again and they will now need to be migrated as one.

Alternative solutions could be:

  • Introduce a shim executable that automatically picks whatever supported Python it can find in the PATH.
  • Separate executables into a separate formula (perhaps tricky given we don't support subpackages)

@cho-m
Copy link
Copy Markdown
Member Author

cho-m commented Oct 20, 2023

Though it does seem problematic when we have any dependents using it as we'll get mixed runtime Pythons again and they will now need to be migrated as one.

As I recall, we ignore mixed Pythons in dependency tree (#13714) so migration should still be possible separately.

Alternative solutions could be:

  • Introduce a shim executable that automatically picks whatever supported Python it can find in the PATH.

I've thought on this before but seemed like it would end up a bit hacky. Open to discussion if there is a reasonable way. However, some formulae will still be non-functional out-of-the-box unless user installs a python separately (which may require propagating caveats to impacted formulae).

  • Separate executables into a separate formula (perhaps tricky given we don't support subpackages)

We've tried this before and reverted those formulae now (e.g. python-tabulate + libpython-tabulate). It often required various workarounds to support given our lack of subpackages.

@Bo98
Copy link
Copy Markdown
Member

Bo98 commented Oct 23, 2023

As I recall, we ignore mixed Pythons in dependency tree (#13714) so migration should still be possible separately.

It's not a great user experience though for a formula to depend on two versions of Python, particularly when only one is really needed.

I'm fine with this change if it's preferred, but it does feel like there's better solutions.

@cho-m
Copy link
Copy Markdown
Member Author

cho-m commented Oct 23, 2023

It's not a great user experience though for a formula to depend on two versions of Python, particularly when only one is really needed.

Ideally, should only happen for short time during migration.

We probably should discuss/document further on Python package distribution as we don't seem to be following our original guidelines, e.g. https://docs.brew.sh/Python-for-Formula-Authors


I'm fine with this change if it's preferred, but it does feel like there's better solutions.

I am open to any other ideas. Just that during current python@3.11 --> python@3.12 migration we have impacted more formulae than previous migrations so may end up seeing more user issues.


An opposite approach would be preventing "multiple pythons" for formulae whose functionality depends on the bin scripts.

For example,

On other hand, there are some formulae that have bin scripts that are not commonly used (e.g. python-tabulate).

@MikeMcQuaid
Copy link
Copy Markdown
Member

We probably should discuss/document further on Python package distribution as we don't seem to be following our original guidelines, e.g. https://docs.brew.sh/Python-for-Formula-Authors

If we can turn any of those guidelines into code/audits/rubocops: that would be ideal.

An opposite approach would be preventing "multiple pythons" for formulae whose functionality depends on the bin scripts.

This makes sense to me 👍🏻

@cho-m cho-m added the discussion Input solicited from others label Oct 25, 2023
@cho-m
Copy link
Copy Markdown
Member Author

cho-m commented Nov 13, 2023

Will close for now until we finalize on how we want to handle Python formulae moving forward (especially given adoption of PEP 668 in python@3.12 and other changes ongoing in homebrew-core).

@cho-m cho-m closed this Nov 13, 2023
@github-actions github-actions bot added the outdated PR was locked due to age label Dec 14, 2023
@github-actions github-actions bot locked as resolved and limited conversation to collaborators Dec 14, 2023
@cho-m cho-m deleted the py-deps-rubocop branch March 5, 2024 03:46
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

discussion Input solicited from others outdated PR was locked due to age

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants