Skip to content
This repository was archived by the owner on Apr 22, 2023. It is now read-only.

deps: update gyp to 96a394e#6730

Closed
indutny wants to merge 1 commit intonodejs:masterfrom
indutny:feature/update-gyp
Closed

deps: update gyp to 96a394e#6730
indutny wants to merge 1 commit intonodejs:masterfrom
indutny:feature/update-gyp

Conversation

@indutny
Copy link
Member

@indutny indutny commented Dec 18, 2013

Suggested reviewer: @tjfontaine

fix #3681

@mistydemeo
Copy link

Gyp still needs a couple of fixes before it will be ready for Mac CLT users.

@TooTallNate
Copy link

@indutny +1, you beat me to it :)

@mistydemeo Like what? I'm getting reports that this version of gyp is working for CLT users in nodejs/node-gyp#341.

@mistydemeo
Copy link

@TooTallNate That patch doesn't address the Xcode version checking, which still uses xcodebuild, and thus can still tank the configure for users CLT-only users. I have a patch for that that I'll be submitting to gyp this evening.

@tjfontaine
Copy link

I'm inclined to land this anyway

@mistydemeo
Copy link

According to users in Homebrew, the current version doesn't actually work for most people. The next patch will fix it for sure. I can provide the patch here when I submit it to gyp, so you can include it downstream without waiting for gyp to merge if you want.

@mistydemeo
Copy link

(A version of the patch I'm submitting to gyp is already in the Homebrew formula for node and is confirmed to be working.)

@tjfontaine
Copy link

Does this upgrade in and of itself break homebrew users, or are they merely broken in the same way unless they had already installed gcc or xcode? Or is the homebrew formula patching gyp in place to make it work without xcode?

@mistydemeo
Copy link

The Homebrew formula is patching gyp in place to make it work without Xcode. It includes my earlier patch, now in gyp, and the one I'm submitting tonight.

@tjfontaine
Copy link

ok thanks, @mistydemeo I'll hold off on this until you get upstreamed then, I appreciate all the help and feedback on this!

@mistydemeo
Copy link

Both patches have now landed in gyp.

@indutny
Copy link
Member Author

indutny commented Dec 27, 2013

Thanks, landed in 96dffb1

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants