Last Comment Bug 758925 - update in-tree virtualenv to 1.7.2
: update in-tree virtualenv to 1.7.2
Status: RESOLVED FIXED
: regression
Product: Core
Classification: Components
Component: Build Config (show other bugs)
: Trunk
: Other Android
: -- blocker (vote)
: mozilla16
Assigned To: Jeff Hammel
:
Mentors:
Depends on:
Blocks: 661908 767329
  Show dependency treegraph
 
Reported: 2012-05-26 20:31 PDT by Kartikaya Gupta (email:kats@mozilla.com)
Modified: 2012-06-28 18:35 PDT (History)
7 users (show)
ryanvm: in‑testsuite-
See Also:
Crash Signature:
(edit)
QA Whiteboard:
Iteration: ---
Points: ---
Has Regression Range: ---
Has STR: ---


Attachments
Build output (88.63 KB, text/plain)
2012-05-26 20:31 PDT, Kartikaya Gupta (email:kats@mozilla.com)
no flags Details
Avoid relying on install_name_tool to modify the python binary on OSX (7.17 KB, patch)
2012-06-14 07:36 PDT, Mike Hommey [:glandium]
carl: review-
Details | Diff | Splinter Review
pointing to current tip (886.52 KB, patch)
2012-06-27 15:38 PDT, Jeff Hammel
ted: review+
Details | Diff | Splinter Review

Description Kartikaya Gupta (email:kats@mozilla.com) 2012-05-26 20:31:01 PDT
Created attachment 627530 [details]
Build output

STR:
1. Update to latest m-c code (I'm using a git clone of https://github.com/mozilla/mozilla-central.git)
2. rm -rf objdir/*
3. make -f client.mk

Expected results:
- stuff builds

Actual results:
- build fails with python virtual env errors. See attached file for the full build output (error is at the end)
Comment 1 Kartikaya Gupta (email:kats@mozilla.com) 2012-05-26 20:36:50 PDT
In case it matters, I'm running OS X 10.7.4 and python 2.7.1
Comment 2 Ted Mielczarek [:ted.mielczarek] 2012-05-27 12:18:33 PDT
Huh, I have no idea what's going on there.
Comment 3 Brad Jackson 2012-05-27 17:01:50 PDT
Could be this error that causes the second.

Could not call install_name_tool -- you must have Apple's development tools installed
Comment 4 Carl Meyer (:carljm) 2012-05-27 21:10:02 PDT
Yes, virtualenv requires Xcode (or the new Apple command-line developer tools package) on OS X, and that's the source of the problem.
Comment 5 Mike Hommey [:glandium] 2012-05-28 01:13:19 PDT
AFAICT, this is what virtualenv does:
- It copies /System/Library/Frameworks/Python.framework/Versions/2.7/Resources/Python.app/Contents/MacOS/Python to _virtualenv/bin/python
- It copies /System/Library/Frameworks/Python.framework/Versions/2.7/Python in _virtualenv/.Python
- It uses install_name_tool to change the reference in _virtualenv/bin/python from /System/Library/Frameworks/Python.framework/Versions/2.7/Python to @executable_path/../.Python (which, in practice, means _virtualenv/.Python)

If, instead of doing the latter, we call python with DYLD_INSERT_LIBRARIES=$(DEPTH)/_virtualenv/.Python, then it should work without needing install_name_tool.
Comment 6 Kartikaya Gupta (email:kats@mozilla.com) 2012-05-28 09:49:45 PDT
I do have install_name_tool installed and on my path. The log output shows that it does run, but produces an error:

install_name_tool: object: ./_virtualenv/bin/python malformed object (unknown load command 4)
Comment 7 Kartikaya Gupta (email:kats@mozilla.com) 2012-05-28 09:52:32 PDT
Is there something I can try or back out to get this working? This is blocking me from doing anything.
Comment 8 Kartikaya Gupta (email:kats@mozilla.com) 2012-05-28 13:36:59 PDT
Updating XCode to the latest version (which presumably updated the version of install_name_tool) seems to have done the trick. At least, it's gotten past that first error, will WFM this bug if the build completes.
Comment 9 Kartikaya Gupta (email:kats@mozilla.com) 2012-05-28 14:34:12 PDT
Build worked. Yay!
Comment 10 Justin Wood (:Callek) (Away until Aug 29) 2012-05-28 18:32:59 PDT
(In reply to Kartikaya Gupta (:kats) from comment #8)
> Updating XCode to the latest version (which presumably updated the version
> of install_name_tool) seems to have done the trick. At least, it's gotten
> past that first error, will WFM this bug if the build completes.

What version did you have before/after?

(In reply to Mike Hommey [:glandium] from comment #5)
> If, instead of doing the latter, we call python with
> DYLD_INSERT_LIBRARIES=$(DEPTH)/_virtualenv/.Python, then it should work
> without needing install_name_tool.

Reoppening and assigning to Mike to figure out what the required way forward here is (if anything). Might be "we need to document that you need that version of XCode" might be "we need to back this out", might be "we need to do some magic DYLD_* change", but whatever it is I trust him to make that call.
Comment 11 Kartikaya Gupta (email:kats@mozilla.com) 2012-05-29 06:10:27 PDT
kats@kgupta-air ~$ pkgutil --file-info /usr/bin/install_name_tool
volume: /
path: /usr/bin/install_name_tool

pkgid: com.apple.pkg.DeveloperToolsCLILeo
pkg-version: 1.0.0.9000000000.1.1249367152
install-time: 1338237149
uid: 0
gid: 0
mode: 555

I had run this before and after doing the update, and the only thing that changed was the install time (in particular the pkg-version didn't change). When I fire up XCode now, it reports its version as 4.1 (4B110); I'm not sure what it was before I did the update.
Comment 12 Mike Hommey [:glandium] 2012-06-14 07:36:31 PDT
Created attachment 633133 [details] [diff] [review]
Avoid relying on install_name_tool to modify the python binary on OSX
Comment 13 Mike Hommey [:glandium] 2012-06-14 07:46:41 PDT
(In reply to Mike Hommey [:glandium] from comment #12)
> Created attachment 633133 [details] [diff] [review]
> Avoid relying on install_name_tool to modify the python binary on OSX

Sent a pull request upstream, too. https://github.com/pypa/virtualenv/pull/289
Comment 14 Ted Mielczarek [:ted.mielczarek] 2012-06-14 09:09:39 PDT
Normally I'd just say if you got upstream review you'd be fine here, but I'm probably fairly qualified to review this actual code.

Carl: do you want me to review this to help you land it upstream?
Comment 15 Carl Meyer (:carljm) 2012-06-14 09:19:19 PDT
(In reply to Ted Mielczarek [:ted] from comment #14)
> Normally I'd just say if you got upstream review you'd be fine here, but I'm
> probably fairly qualified to review this actual code.
> 
> Carl: do you want me to review this to help you land it upstream?

That would be great. The code looks fine to me, but I know next to nothing about OS X and Mach-O. Also, confirmation that it works, of course, especially if you have an older OS X available to test on.
Comment 16 Carl Meyer (:carljm) 2012-06-24 00:36:13 PDT
A pull request based on this patch has been merged into virtualenv's "develop" branch, and released as part of virtualenv 1.7.2. Thanks very much for the contribution; it's something virtualenv users have been asking for for a long time.

I r-ed the patch attached here, as it's not up to date with what was actually committed (particularly in that everything was moved into virtualenv.py). I suppose the correct patch here would simply be a full upgrade of virtualenv.py to version 1.7.2, rather than applying single upstream patches piecemeal.
Comment 17 Carl Meyer (:carljm) 2012-06-26 09:29:16 PDT
Note that the fix for bug 767329 was applied upstream just after the release of 1.7.2, so in order to fix both bugs virtualenv will need to be updated to the "develop" branch, not the 1.7.2 release.
Comment 18 Jeff Hammel 2012-06-27 15:38:56 PDT
Created attachment 637288 [details] [diff] [review]
pointing to current tip

Taking virtualenv tip as of today
Comment 19 Jeff Hammel 2012-06-27 15:41:38 PDT
(In reply to Jeff Hammel [:jhammel] from comment #18)
> Created attachment 637288 [details] [diff] [review]
> pointing to current tip
> 
> Taking virtualenv tip as of today

And pushed to try: https://tbpl.mozilla.org/?tree=Try&rev=97ee967242b5
Comment 20 Jeff Hammel 2012-06-28 12:26:01 PDT
(In reply to Jeff Hammel [:jhammel] from comment #19)
> (In reply to Jeff Hammel [:jhammel] from comment #18)
> > Created attachment 637288 [details] [diff] [review]
> > pointing to current tip
> > 
> > Taking virtualenv tip as of today
> 
> And pushed to try: https://tbpl.mozilla.org/?tree=Try&rev=97ee967242b5

ABICT none of the failures are due to the new virtualenv.
Comment 21 Jeff Hammel 2012-06-28 12:35:44 PDT
Thanks.  Now it needs to be pushed
Comment 22 Jeff Hammel 2012-06-28 15:22:31 PDT
This should also fix bug 767329
Comment 23 Ryan VanderMeulen [:RyanVM] 2012-06-28 18:35:26 PDT
https://hg.mozilla.org/mozilla-central/rev/8534fbb1a5da

Note You need to log in before you can comment on or make changes to this bug.