xcode_emulation: fix on CLT-only systems redux

xcode_emulation: fix on CLT-only systems redux

This reimplements r1819 in a way that should avoid breaking Xcode, and
should also allow setting framework paths on CLT-only systems.

BUG=code.google.com/p/gyp/issues/detail?id=292

[mistydemeo, 2013-12-24 06:34:00]

Here's a redo of https://codereview.chromium.org/102733012 - this *should* pass
tests and work on Xcode. It also should pass framework paths correctly on CLT,
as well.

I can confirm that the one test that seems to have been broken by the previous
patch, test/mac/gyptest-framework-dirs.py, works now on 10.8 with Xcode 5. There
aren't any other test failures in common between what I ran locally and what was
reported previously, so I don't think there are any other issues - however, the
test results don't seem to be consistent either. I'd appreciate it if you could
run the tests there and see if all looks good.

[Nico, 2013-12-24 06:37:46]

Thanks. I sent try jobs here: https://codereview.chromium.org/117013003

[mistydemeo, 2013-12-24 06:41:26]

On 2013/12/24 06:37:46, Nico wrote:
> Thanks. I sent try jobs here: https://codereview.chromium.org/117013003

Thanks!

I noticed one minor thing I missed fixing (won't affect tests, will affect
CLT-only systems possibly):

L862-865
    if self._SdkPath():
      return l.replace('$(SDKROOT)', self._SdkPath(config_name))
    else:
      return l

Should be:

    if self._SdkPath():
      sdk_root = self._SdkPath(config_name)
    else:
      sdk_root = ''
    return l.replace('$(SDKROOT)', sdk_root)

What's the preferred workflow for amending a commit? Should I alter the commit
and repush? Add a second commit?

[Nico, 2013-12-24 06:54:11]

On 2013/12/24 06:41:26, mistydemeo wrote:
> On 2013/12/24 06:37:46, Nico wrote:
> > Thanks. I sent try jobs here: https://codereview.chromium.org/117013003

Looks like the 3 ios tests are still red:
http://build.chromium.org/p/tryserver.nacl/builders/gyp-mac/builds/1491/steps...

> I noticed one minor thing I missed fixing (won't affect tests, will affect
> CLT-only systems possibly):
> 
> L862-865
>     if self._SdkPath():
>       return l.replace('$(SDKROOT)', self._SdkPath(config_name))
>     else:
>       return l
> 
> Should be:
> 
>     if self._SdkPath():
>       sdk_root = self._SdkPath(config_name)
>     else:
>       sdk_root = ''
>     return l.replace('$(SDKROOT)', sdk_root)
> 
> What's the preferred workflow for amending a commit? Should I alter the commit
> and repush? Add a second commit?

If you do a second commit on a branch and then do `git cl upload`, it'll upload
a squashed diff. Since the gyp master repo is svn anyhoo, it'll land in a single
commit once it's all said and done. (I tend to have ~5-30 commits all with the
informative commit description '.' on my feature branches).

[mistydemeo, 2013-12-24 07:01:56]

On 2013/12/24 06:54:11, Nico wrote:
> On 2013/12/24 06:41:26, mistydemeo wrote:
> > On 2013/12/24 06:37:46, Nico wrote:
> > > Thanks. I sent try jobs here: https://codereview.chromium.org/117013003
> 
> Looks like the 3 ios tests are still red:
>
http://build.chromium.org/p/tryserver.nacl/builders/gyp-mac/builds/1491/steps...
> 
> > I noticed one minor thing I missed fixing (won't affect tests, will affect
> > CLT-only systems possibly):
> > 
> > L862-865
> >     if self._SdkPath():
> >       return l.replace('$(SDKROOT)', self._SdkPath(config_name))
> >     else:
> >       return l
> > 
> > Should be:
> > 
> >     if self._SdkPath():
> >       sdk_root = self._SdkPath(config_name)
> >     else:
> >       sdk_root = ''
> >     return l.replace('$(SDKROOT)', sdk_root)
> > 
> > What's the preferred workflow for amending a commit? Should I alter the
commit
> > and repush? Add a second commit?
> 
> If you do a second commit on a branch and then do `git cl upload`, it'll
upload
> a squashed diff. Since the gyp master repo is svn anyhoo, it'll land in a
single
> commit once it's all said and done. (I tend to have ~5-30 commits all with the
> informative commit description '.' on my feature branches).

Thanks, uploaded. Looks like it 500ed on me again so let me know if something
seems wrong.

Looks like the Xcode and make tests pass, but there are three ninja failures.
Unfortunately I haven't been able to get the ninja tests to run locally at all,
so it's difficult for me to diagnose. Is there a specific version of ninja
required? I've been using 1.4.0 (the latest release). I haven't touched anything
ninja-specific. Are those tests passing on the latest gyp master?

[mistydemeo, 2013-12-24 09:35:59]

On 2013/12/24 07:01:56, mistydemeo wrote:
> On 2013/12/24 06:54:11, Nico wrote:
> > On 2013/12/24 06:41:26, mistydemeo wrote:
> > > On 2013/12/24 06:37:46, Nico wrote:
> > > > Thanks. I sent try jobs here: https://codereview.chromium.org/117013003
> > 
> > Looks like the 3 ios tests are still red:
> >
>
http://build.chromium.org/p/tryserver.nacl/builders/gyp-mac/builds/1491/steps...
> > 
> > > I noticed one minor thing I missed fixing (won't affect tests, will affect
> > > CLT-only systems possibly):
> > > 
> > > L862-865
> > >     if self._SdkPath():
> > >       return l.replace('$(SDKROOT)', self._SdkPath(config_name))
> > >     else:
> > >       return l
> > > 
> > > Should be:
> > > 
> > >     if self._SdkPath():
> > >       sdk_root = self._SdkPath(config_name)
> > >     else:
> > >       sdk_root = ''
> > >     return l.replace('$(SDKROOT)', sdk_root)
> > > 
> > > What's the preferred workflow for amending a commit? Should I alter the
> commit
> > > and repush? Add a second commit?
> > 
> > If you do a second commit on a branch and then do `git cl upload`, it'll
> upload
> > a squashed diff. Since the gyp master repo is svn anyhoo, it'll land in a
> single
> > commit once it's all said and done. (I tend to have ~5-30 commits all with
the
> > informative commit description '.' on my feature branches).
> 
> Thanks, uploaded. Looks like it 500ed on me again so let me know if something
> seems wrong.
> 
> Looks like the Xcode and make tests pass, but there are three ninja failures.
> Unfortunately I haven't been able to get the ninja tests to run locally at
all,
> so it's difficult for me to diagnose. Is there a specific version of ninja
> required? I've been using 1.4.0 (the latest release). I haven't touched
anything
> ninja-specific. Are those tests passing on the latest gyp master?

Caught it. I'd made a mistake with a call to SdkPath() that caused issues with
ninja only. I was able to get the ninja tests running locally, and they all pass
now - this should be good to go.

[Nico, 2013-12-24 19:10:43]

Is this on top of current trunk? The diff from patch set 3 doesn't apply for me
(the first 2 hunks I can figure out, but I'm not sure what the 3rd should do)

[Nico, 2013-12-24 19:13:33]

oooh I see, you somehow put one commit per patch set :-D

In rietveld, every patch set is supposed to be self-contained, and newer patch
sets conceptually replace older versions (the idea is that you get comments on a
patch set, then you'll address them, upload a new patch set, and the reviewer
can look at the delta to the previous patch set to figure out what you changed).
Anyways, I'll send try jobs for this, one sec…

[Nico, 2013-12-24 19:16:21]

Try bots on the way at patch set 2 at https://codereview.chromium.org/117013003

https://codereview.chromium.org/118473009/diff/100001/pylib/gyp/xcode_emulati...
File pylib/gyp/xcode_emulation.py (right):

https://codereview.chromium.org/118473009/diff/100001/pylib/gyp/xcode_emulati...
pylib/gyp/xcode_emulation.py:864: if not sdk_root:
`sdk_root is None` maybe?

[Nico, 2013-12-24 20:27:17]

Try bots came back green. lgtm, r1825

[mistydemeo, 2013-12-24 20:53:53]

On 2013/12/24 20:27:17, Nico wrote:
> Try bots came back green. lgtm, r1825

Great, thanks!

Didn't realize that was how rietveld worked, sorry! It let me `git cl upload`
when I added new commits on top and that submitted the new commits as separate
patchsets.