Fix gyptest-type-envvars.py for Xcode 5.0.0

Fix gyptest-type-envvars.py for Xcode 5.0.0

With Xcode 5.0.0, the SDKROOT environment variable is always set
when building, even if not set in the Xcode project.

BUG=381
R=mark@chromium.org

Committed: https://code.google.com/p/gyp/source/detail?r=1863

[Mark Mentovai, 2013-11-22 14:49:44]

Don’t the ninja and make generators need to be updated too?

[Mark Mentovai, 2013-12-06 20:14:40]

Does https://codereview.chromium.org/83463003 fix this for ninja?

[sdefresne, 2014-03-06 16:28:31]

On 2013/12/06 20:14:40, Mark Mentovai wrote:
> Does https://codereview.chromium.org/83463003 fix this for ninja?

Sorry for coming back to such an old issue.

To answer your question, no https://codereview.chromium.org/83463003 didn't fix
it.
I've updated my change and the tests now pass locally. I don't know why they
fail on the bots though (crash of Xcode).

Could you take a look?

[Mark Mentovai, 2014-03-06 17:27:29]

https://codereview.chromium.org/82763006/diff/170001/test/mac/type_envvars/te...
File test/mac/type_envvars/test_bundle_executable.sh (right):

https://codereview.chromium.org/82763006/diff/170001/test/mac/type_envvars/te...
test/mac/type_envvars/test_bundle_executable.sh:20: source "$(dirname
"$0")/test_check_sdkroot.sh"
These should all work without “source”.

https://codereview.chromium.org/82763006/diff/170001/test/mac/type_envvars/te...
File test/mac/type_envvars/test_check_sdkroot.sh (right):

https://codereview.chromium.org/82763006/diff/170001/test/mac/type_envvars/te...
test/mac/type_envvars/test_check_sdkroot.sh:17: xcodebuild -version|head -n1|sed
's,.* ,0000,;s,\.,,g;s,.*\(....\)$,\1,'
This works for me locally too, but I have Xcode 5 also. The bots have
4.something.

The bot results from your patch set 7 (with extra debug) are at

http://build.chromium.org/p/tryserver.nacl/builders/gyp-mac/builds/1570/steps...

Here’s an excerpt

PhaseScriptExecution "Postbuild \"envtest\"" […]
[…]
** INTERNAL ERROR: Uncaught exception **
Exception: *** -[NSConcreteFileHandle writeData:]: Broken pipe
Stack:
[…]
xcodeversion=0042
SDKROOT=

EPIPE being treated as a fatal error in xcodebuild. Let’s try to fix the EPIPE.
Where’s it coming from? Well, you’re piping xcodebuild -version to head, which
is going to close its stdin early, after it’s read a single line. That could
cause EPIPE in the writer, couldn’t it?

Options:

xcodebuild -version | sed -En -e 's/^Xcode ([0-9])\.([0-9])\.([0-9])/0\1\2\3/p'

(because sed doesn’t close its input before EOF)

or

local xcodebuild_version=$(mktemp -f "$(basename "$0")")
xcodebuild -version > "${xcodebuild_version}"
head -n1 "${xcodebuild_version}" | sed 's,.* ,0000,;s,\.,,g;s,.*\(....\)$,\1,'
rm "${xcodebuild_version}"

https://codereview.chromium.org/82763006/diff/170001/test/mac/type_envvars/te...
test/mac/type_envvars/test_check_sdkroot.sh:24: local min="$(echo -ne
"${1}\n${2}\n"|sort -n|head -n1)"
Can you put some spaces around your pipes for readability?

[Mark Mentovai, 2014-03-06 17:28:10]

Oh, and the

xcodeversion=0042

in the “extra debugging” log was troubling, so you might want to double-check
the sed anyway.

[sdefresne, 2014-03-07 21:16:11]

Please take another look.

I'd like to apologise for the noise but I couldn't access the bots, and I didn't
understand why they failed so I used try bots to do printf debugging. This was
painful, but now the tests pass both on my desktop with Xcode 5.0.2 and on the
bots. \o/

https://codereview.chromium.org/82763006/diff/170001/test/mac/type_envvars/te...
File test/mac/type_envvars/test_bundle_executable.sh (right):

https://codereview.chromium.org/82763006/diff/170001/test/mac/type_envvars/te...
test/mac/type_envvars/test_bundle_executable.sh:20: source "$(dirname
"$0")/test_check_sdkroot.sh"
On 2014/03/06 17:27:29, Mark Mentovai wrote:
> These should all work without “source”.

I'm using a git checkout, and even if I give the file +x mode, it end up not
executable in the svn repository (I guess I'll have to use another CL using svn
only to set the correct svn property on that file and then remove the source).

The error when not using the source is:
http://build.chromium.org/p/tryserver.nacl/builders/gyp-mac/builds/1581/steps...

https://codereview.chromium.org/82763006/diff/170001/test/mac/type_envvars/te...
File test/mac/type_envvars/test_check_sdkroot.sh (right):

https://codereview.chromium.org/82763006/diff/170001/test/mac/type_envvars/te...
test/mac/type_envvars/test_check_sdkroot.sh:17: xcodebuild -version|head -n1|sed
's,.* ,0000,;s,\.,,g;s,.*\(....\)$,\1,'
On 2014/03/06 17:27:29, Mark Mentovai wrote:
> This works for me locally too, but I have Xcode 5 also. The bots have
> 4.something.
> 
> The bot results from your patch set 7 (with extra debug) are at
> 
>
http://build.chromium.org/p/tryserver.nacl/builders/gyp-mac/builds/1570/steps...
> 
> Here’s an excerpt
> 
> PhaseScriptExecution "Postbuild \"envtest\"" […]
> […]
> ** INTERNAL ERROR: Uncaught exception **
> Exception: *** -[NSConcreteFileHandle writeData:]: Broken pipe
> Stack:
> […]
> xcodeversion=0042
> SDKROOT=
> 
> EPIPE being treated as a fatal error in xcodebuild. Let’s try to fix the
EPIPE.
> Where’s it coming from? Well, you’re piping xcodebuild -version to head, which
> is going to close its stdin early, after it’s read a single line. That could
> cause EPIPE in the writer, couldn’t it?
> 
> Options:
> 
> xcodebuild -version | sed -En -e 's/^Xcode
([0-9])\.([0-9])\.([0-9])/0\1\2\3/p'
> 
> (because sed doesn’t close its input before EOF)
> 
> or
> 
> local xcodebuild_version=$(mktemp -f "$(basename "$0")")
> xcodebuild -version > "${xcodebuild_version}"
> head -n1 "${xcodebuild_version}" | sed 's,.* ,0000,;s,\.,,g;s,.*\(....\)$,\1,'
> rm "${xcodebuild_version}"

OK, there were multiple incorrect assumption in my command. So, instead of using
sed, I've used awk that can do more complex conditionals. It now does work on
the bots (BTW, they have Xcode 4.2 installed, and since the sed expression
didn't support that). This is now fixed and works with the version on the bots
and locally.

https://codereview.chromium.org/82763006/diff/170001/test/mac/type_envvars/te...
test/mac/type_envvars/test_check_sdkroot.sh:24: local min="$(echo -ne
"${1}\n${2}\n"|sort -n|head -n1)"
On 2014/03/06 17:27:29, Mark Mentovai wrote:
> Can you put some spaces around your pipes for readability?

Done.

[Mark Mentovai, 2014-03-07 21:51:08]

OK, I can LG this if you fix these last things up.

https://codereview.chromium.org/82763006/diff/170001/test/mac/type_envvars/te...
File test/mac/type_envvars/test_bundle_executable.sh (right):

https://codereview.chromium.org/82763006/diff/170001/test/mac/type_envvars/te...
test/mac/type_envvars/test_bundle_executable.sh:20: source "$(dirname
"$0")/test_check_sdkroot.sh"
sdefresne wrote:
> I'm using a git checkout, and even if I give the file +x mode, it end up not
> executable in the svn repository (I guess I'll have to use another CL using
svn
> only to set the correct svn property on that file and then remove the source).

You still don’t want to use source, though, because it pollutes the current
environment. test_check_sdkroot.sh is intended to run as a standalone, so let’s
run it that way. If file modes are a problem, then use “sh
test_check_sdkroot.sh”.

https://codereview.chromium.org/82763006/diff/370001/test/mac/type_envvars/te...
File test/mac/type_envvars/test_check_sdkroot.sh (right):

https://codereview.chromium.org/82763006/diff/370001/test/mac/type_envvars/te...
test/mac/type_envvars/test_check_sdkroot.sh:17: # Note that version can be
'Xcode 4.2' which is converted to '0420'.
Instead of writing this here, put it above in the `xcodebuild -version` output
section. This comment here as a standalone gets lost. It really should be listed
above where the other `xcodebuild -version` output is shown.

https://codereview.chromium.org/82763006/diff/370001/test/mac/type_envvars/te...
test/mac/type_envvars/test_check_sdkroot.sh:19: xcodebuild -version|awk '/Xcode
([0-9]+\.[0-9]+(\.[0-9]+)?)/ {
Spaces around pipes, please.

[sdefresne, 2014-03-08 10:33:40]

https://codereview.chromium.org/82763006/diff/370001/test/mac/type_envvars/te...
File test/mac/type_envvars/test_check_sdkroot.sh (right):

https://codereview.chromium.org/82763006/diff/370001/test/mac/type_envvars/te...
test/mac/type_envvars/test_check_sdkroot.sh:17: # Note that version can be
'Xcode 4.2' which is converted to '0420'.
On 2014/03/07 21:51:09, Mark Mentovai wrote:
> Instead of writing this here, put it above in the `xcodebuild -version` output
> section. This comment here as a standalone gets lost. It really should be
listed
> above where the other `xcodebuild -version` output is shown.

Done.

https://codereview.chromium.org/82763006/diff/370001/test/mac/type_envvars/te...
test/mac/type_envvars/test_check_sdkroot.sh:19: xcodebuild -version|awk '/Xcode
([0-9]+\.[0-9]+(\.[0-9]+)?)/ {
On 2014/03/07 21:51:09, Mark Mentovai wrote:
> Spaces around pipes, please.

Done.

[Mark Mentovai, 2014-03-08 14:25:42]

LGTM

[sdefresne, 2014-03-09 17:30:20]

Committed patchset #21 manually as r1863 (presubmit successful).