Skip to content
This repository has been archived by the owner on Jul 4, 2023. It is now read-only.

(WIP) ddar: installation fixed #43358

Closed
wants to merge 1 commit into from
Closed

(WIP) ddar: installation fixed #43358

wants to merge 1 commit into from

Conversation

bfontaine
Copy link
Contributor

The installed script wasn’t finding its own library. This fixes it, but it still fails to find protobuf:

$ ddar -h
Traceback (most recent call last):
  File "/usr/local/Cellar/ddar/1.0_1/libexec/bin/ddar", line 20, in <module>
    import synctus.ddar_pb2, synctus.dds
  File "/usr/local/lib/python2.7/site-packages/synctus/ddar_pb2.py", line 6, in <module>
ImportError: No module named google.protobuf

I don’t know how to fix that. cc @tdsmith

TODO:

  • Fix the installation
  • Add a test

@bfontaine bfontaine mentioned this pull request Aug 28, 2015
27 tasks
@tdsmith
Copy link
Contributor

tdsmith commented Aug 28, 2015

Where do you see this? I think you shouldn't see that on the bot, or ever during brew test. If you're seeing it locally, brew doctor should scold you about your Python configuration.

@bfontaine
Copy link
Contributor Author

@tdsmith I have some trouble understanding how PYTHONPATH works here. The executable doesn’t find its own Python modules:

==> ddar -h
Traceback (most recent call last):
  File "/usr/local/Cellar/ddar/1.0_1/libexec/bin/ddar", line 20, in <module>
    import synctus.ddar_pb2, synctus.dds
ImportError: No module named synctus.ddar_pb2
…snip…

I was able to fix it locally by setting ENV["PYTHONPATH"] in the formula but it then failed because it couldn’t find google.protobuf, which available through $(brew --prefix)/lib/python2.7/site-packages/homebrew-protobuf.pth.


def install
ENV.prepend_create_path "PYTHONPATH", libexec/"vendor/lib/python2.7/site-packages"
ENV.prepend_path "PYTHONPATH", Language::Python.homebrew_site_packages
Copy link
Contributor

Choose a reason for hiding this comment

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

this should not be necessary; it won't help pick up protobuf because protobuf is brought in through a .pth file and pythonpath doesn't help python pick up .pths

@tdsmith
Copy link
Contributor

tdsmith commented Aug 30, 2015

libexec/"lib/python2.7/site-packages" is missing from PYTHONPATH, which is at least part of why the synctus packages can't be found. If libexec/"lib/python2.7/site-packages/synctus" only contains directories and no regular files, then it's probably a namespace package; namespace packages are kinda broken in Python and you may have to manually touch __init__.py in that directory.

@apjanke
Copy link
Contributor

apjanke commented Oct 7, 2015

Hey folks.

I think part of the issue may be that the main ddar script has a hardcoded #!/usr/bin/python shebang line, so it always uses the system python. This may not have cropped up as a problem for users before, because if you build any formula with Python bindings against the system python, then Caveats tells you to add a ~/Library/Python/2.7/lib/python/site-packages/homebrew.pth that will make system python invocations pick up the modules installed under /usr/local. These caveats may not pop up for ddar, because brew info is looking at your $PATH to see what Python you're running, not the shebang line in installed files. Similarly, there's no brew doctor complaint about it.

Here's a branch I made that fixes ddar by just modifying the shebang line. It will get ddar to run correctly if you have Homebrew Python installed, and you'll see the Caveats about the Python path in brew install ddar if you don't. I'm not sure if this is the best way to handle it, though. The caveats talk about using Python modules, but users will just be running ddar as a command, and shouldn't have to know about its implementation that much. And it seems like we shouldn't have to rely on the user Python site configuration to get a Homebrew-installed command working (as opposed to a Homebrew-installed Python library, which a developer can be expected to do a little more tweaking to get to run).

Reading through the Python "site" path stuff, I don't see a way to get Python to load in .pth stuff using just environment variables or runtime configuration; it seems to want config files in the existing site-packages dirs on disk.

UPDATE: I made a PR #44695 for that branch, so we can see its behavior on the test bot.

@bfontaine
Copy link
Contributor Author

Thank you @apjanke 😄

@bfontaine
Copy link
Contributor Author

Closing in favor of #44695.

@bfontaine bfontaine closed this Oct 7, 2015
@bfontaine bfontaine deleted the ddar-fix branch October 7, 2015 09:51
@Homebrew Homebrew locked and limited conversation to collaborators Jul 10, 2016
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants