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

(WIP) ddar: fix python shebang location, add test, fix audit #44695

Closed
wants to merge 1 commit into from
Closed

(WIP) ddar: fix python shebang location, add test, fix audit #44695

wants to merge 1 commit into from

Conversation

apjanke
Copy link
Contributor

@apjanke apjanke commented Oct 7, 2015

WIP. Please do not merge yet. I'm mainly PRing it so it can go through test-bot and CI.

Compare with other ddar WIP, #43358

@apjanke apjanke mentioned this pull request Oct 7, 2015
2 tasks
@@ -21,4 +24,8 @@ def install
bin.env_script_all_files(libexec+"bin", :PYTHONPATH => ENV["PYTHONPATH"])
man1.install Dir["*.1"]
end

test do
assert_match "ddar: store multiple files efficiently", shell_output("ddar --help")
Copy link
Contributor

Choose a reason for hiding this comment

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

It should be "#{bin}/ddar --help".

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Fixed.

@tdsmith
Copy link
Contributor

tdsmith commented Oct 7, 2015

We should install this to libexec and write wrapper scripts to add protobuf's modules to site-packages I guess. If we need to touch a faux __init__.py for the google namespace package while installing protobuf, we can; it should be harmless.

Vendoring the protobuf package sounds potentially nicer, actually.

Sorry, this all kinda sucks.

Distutils should actually already be rewriting the shebang so I don't think that inreplace is doing anything.

@apjanke
Copy link
Contributor Author

apjanke commented Oct 7, 2015

You're right: if I remove that inreplace, then the shebang becomes #!/usr/local/opt/python/bin/python2.7. I'll remove the inreplace.

With this patch, all the ddar stuff does get installed to libexec, and a wrapper script to /usr/local/bin. But I don't think that's sufficient in itself.

I don't think we can successfully add protobuf and other brewed modules to site-packages with a wrapper script at run time. Some of those paths are defined using .pth files, which are only processed in directories which are in Python's site path based on their location in the filesystem. Adding dirs to PYTHONPATH only gets those particular directories on the path, and .pth files in them are not followed. E.g. the protobuf modules get on the path via /usr/local/lib/python2.7/site-packages/homebrew-protobuf.pth. So, from what I can tell, that site-packages dir needs to either be the primary site-packages dir for the python you're running it under, or included via another .pth file in one of the primary site-packages dirs.

What does "vendoring the protobuf package" mean?

@apjanke
Copy link
Contributor Author

apjanke commented Oct 8, 2015

Here's an idea: would it be possible to wrap the program in a python wrapper script instead of a shell script, which checked the paths and called site.addsitedir to add the homebrew site-packages path if necessary, before running the main program in the same python instance? That way we maybe could add the site-packages directory at run time in a way that respected the .pth files in it.

@apjanke apjanke closed this Apr 5, 2016
@apjanke apjanke deleted the ddar-python-location branch April 5, 2016 03:57
@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

4 participants