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

Added setup_py_script macro to pip_parse and pip_install. #575

Closed
wants to merge 2 commits into from

Conversation

UebelAndre
Copy link
Contributor

@UebelAndre UebelAndre commented Nov 28, 2021

PR Checklist

Please check if your PR fulfills the following requirements:

  • Does not include precompiled binaries, eg. .par files. See CONTRIBUTING.md for info
  • 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?

Some python packages provide executable scripts that are not tracked as entry points in the wheel. These come from setup.py and are only created once the package is installed (setup.py runs). I'm trying to find a way to use those scripts in other Bazel rules I've written.

Issue Number: N/A

What is the new behavior?

The changes here introduce a setup_py_script macro to pip_parse and pip_install that are similar to entry_point but specifically access scripts that created by setup.py scripts. This change saves users from having to write their own workaround by accessing the wheel or the output *.data directory contents directly.

Does this PR introduce a breaking change?

  • Yes
  • No

Other information

@UebelAndre
Copy link
Contributor Author

@alexeagle I think you mentioned at one point needing to write a wrapper for what I think (based on my recollection) were these setup.py generated scripts. Do the changes here work for your use cases?

@UebelAndre
Copy link
Contributor Author

Friendly ping on this PR 😅

@UebelAndre
Copy link
Contributor Author

@alexeagle another friendly ping 😅

@UebelAndre
Copy link
Contributor Author

This PR has been rebased on top of #589 to save myself from running into conflicts.

@UebelAndre UebelAndre force-pushed the setup-py branch 3 times, most recently from 427ee26 to cb135ea Compare January 19, 2022 16:19
@UebelAndre
Copy link
Contributor Author

This has now been rebased on #598

@UebelAndre
Copy link
Contributor Author

It seems the entry_points functionality made it into official bazel documentation 😄

Would be nice to get this in too so there are options for all pip install-able executables.

@groodt
Copy link
Collaborator

groodt commented May 31, 2022

Up to you @UebelAndre, but you may want to wait for this? #715

installer correctly handles unpacking of wheels etc and patching shebangs. I've seen it handle both scripts and entry_points.

Note: if you urgently need this, don't let that block. It can always be explored at some later date.

@UebelAndre
Copy link
Contributor Author

UebelAndre commented May 31, 2022

@groodt I definitely do want the ability to use setup.py generated scripts but I don’t care what the plumbing behind the scenes looks like. Are you saying this functionality might already be supported after that PR?

@groodt
Copy link
Collaborator

groodt commented May 31, 2022

Try this with one of your wheels and you might see what I mean.

python -m pip install installer
python -m installer --destdir /tmp/example some_wheel-0.0.0-py3-none-any.whl

If you take a look at https://github.com/bazelbuild/rules_python/pull/715/files#diff-0e793dde90299011d8b9034e303b0d029270c36f27c55dbd900a5835ccf2fa25R76 you are able to define where the scripts are installed (i.e. ./bin). This includes entry_point console scripts and legacy scripts (like those in this PR). It does so in a PEP compliant manner.

This hopefully means we can stop with so much custom code dealing with wheels as zip files and so much fiddling with locations on the filesystem where we unzip things in a non-standards-compliant manner. All we then need to do is point Bazel rules at the right locations once the wheels are unpacked.

@UebelAndre
Copy link
Contributor Author

This is still very much a desired feature but I no longer have bandwidth to continue working on it. Closing this out in hopes that someone else can contribute the functionality.

@UebelAndre UebelAndre closed this Jul 15, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants