Re: Add "standalone_static_library" flag (issue 11031005)

[bor...@google.com]

Uploaded patch set 4, which adds a test.

https://codereview.chromium.org/11031005/

[tha...@chromium.org]

LGTM!

Let me know if you want me to land this. (In that case, I can address the
nit
below myself before committing.)


https://codereview.chromium.org/11031005/diff/11001/test/standalone-static-library/gyptest-standalone-static-library.py
File test/standalone-static-library/gyptest-standalone-static-library.py
(right):

https://codereview.chromium.org/11031005/diff/11001/test/standalone-static-library/gyptest-standalone-static-library.py#newcode21
test/standalone-static-library/gyptest-standalone-static-library.py:21:
# Verify that the static library is copied to PRODUCT_DIR
nit:Style guide says to have trailing '.'s on all comments

https://codereview.chromium.org/11031005/

[bor...@google.com]

That would be great. Thanks!

https://codereview.chromium.org/11031005/

[tha...@chromium.org]

I landed this in 1510 (and addressed the nit in 1511). Sadly, all bots on
go/gypbot went red, for various reasons. I tried to fix in 1512, but didn't
fix
everything, so I reverted it all in 1513. Can you run trybots for this
change?
bungeman@ in your office can tell you how to do that I believe.

https://codereview.chromium.org/11031005/

[bor...@google.com]

Looks like the trybots pass with patch set 13.

https://codereview.chromium.org/11031005/

[tha...@chromium.org]

https://codereview.chromium.org/11031005/diff/15016/pylib/gyp/generator/make.py
File pylib/gyp/generator/make.py (right):

https://codereview.chromium.org/11031005/diff/15016/pylib/gyp/generator/make.py#newcode1544
pylib/gyp/generator/make.py:1544: if (sys.platform.startswith('linux')
and not
Basing this off the host platform doesn't seem right. For example, when
building chrome/android on mac, this is possible too.

The 'alink_thin' rule is based on `self.flavor not in ['mac', 'win']`,
so I'd use that here too.

https://codereview.chromium.org/11031005/diff/15016/test/standalone-static-library/gyptest-standalone-static-library.py
File test/standalone-static-library/gyptest-standalone-static-library.py
(right):

https://codereview.chromium.org/11031005/diff/15016/test/standalone-static-library/gyptest-standalone-static-library.py#newcode32
test/standalone-static-library/gyptest-standalone-static-library.py:32:
if test.format in ('make', 'msvs', 'ninja'):
Why is this necessary? scons? I know that xcode always puts static
libraries into the product dir too.

If this is just for scons, I'd probably rather use `test =
TestGyp.TestGyp(formats='!scons')` to disable the test on scons.

https://codereview.chromium.org/11031005/

[bor...@google.com]

Uploaded patch set 14, which succeeds on all trybots.

https://codereview.chromium.org/11031005/diff/15016/pylib/gyp/generator/make.py
File pylib/gyp/generator/make.py (right):

https://codereview.chromium.org/11031005/diff/15016/pylib/gyp/generator/make.py#newcode1544
pylib/gyp/generator/make.py:1544: if (sys.platform.startswith('linux')
and not

On 2012/10/04 02:18:58, Nico wrote:
> Basing this off the host platform doesn't seem right. For example,
when building
> chrome/android on mac, this is possible too.

> The 'alink_thin' rule is based on `self.flavor not in ['mac', 'win']`,
so I'd
> use that here too.

Done.

https://codereview.chromium.org/11031005/diff/15016/test/standalone-static-library/gyptest-standalone-static-library.py
File test/standalone-static-library/gyptest-standalone-static-library.py
(right):

https://codereview.chromium.org/11031005/diff/15016/test/standalone-static-library/gyptest-standalone-static-library.py#newcode32
test/standalone-static-library/gyptest-standalone-static-library.py:32:
if test.format in ('make', 'msvs', 'ninja'):

On 2012/10/04 02:18:58, Nico wrote:
> Why is this necessary? scons? I know that xcode always puts static
libraries
> into the product dir too.

> If this is just for scons, I'd probably rather use `test =
> TestGyp.TestGyp(formats='!scons')` to disable the test on scons.

Yep. I changed the condition; skipping the test seems excessive since
it all works as expected otherwise.

https://codereview.chromium.org/11031005/

[tha...@chromium.org]

Landed in gyp 1516 / 1517. Thanks for the patch!

https://codereview.chromium.org/11031005/

[bor...@google.com]

On 2012/10/05 04:24:01, Nico wrote:
> Landed in gyp 1516 / 1517. Thanks for the patch!

Thanks for taking your time to review/commit it!

https://codereview.chromium.org/11031005/