758925 - update in-tree virtualenv to 1.7.2

Status: RESOLVED FIXED

(Firefox Build System :: General, defect)

Description

[Kartikaya Gupta (email:[e-mail])]

Attachment: 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:[e-mail])]

In case it matters, I'm running OS X 10.7.4 and python 2.7.1

Comment 2

[(not currently active) Ted Mielczarek]

Huh, I have no idea what's going on there.

Comment 3

[Brad Jackson]

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)]

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]]

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:[e-mail])]

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:[e-mail])]

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:[e-mail])]

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:[e-mail])]

Build worked. Yay!

Comment 10

[Justin Wood (:Callek)]

(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:[e-mail])]

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]]

Attachment: Avoid relying on install_name_tool to modify the python binary on OSX

Comment 13

[Mike Hommey [:glandium]]

(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

[(not currently active) Ted Mielczarek]

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)]

(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)]

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)]

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]

Attachment: pointing to current tip

Taking virtualenv tip as of today

Comment 19

[Jeff Hammel]

(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]

(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]

Thanks.  Now it needs to be pushed

Comment 22

[Jeff Hammel]

This should also fix bug 767329

Comment 23

[Ryan VanderMeulen [:RyanVM]]

https://hg.mozilla.org/mozilla-central/rev/8534fbb1a5da