1331090 - Strip the gtest libxul during packaging

Status: RESOLVED FIXED

(Firefox Build System :: General, defect)

Description

[Chris Manchester (limited bugmail, email directly)]

Ted mentioned something in Hawaii about stripping the gtest libxul. I was reminded of this digging around testsuite-targets.mk this week, and it turns our we're not stripping the gtest libxul, and doing so is a huge size savings.

Comment 1

[Chris Manchester (limited bugmail, email directly)]

Attachment: Bug 1331090 - Strip the gtest libxul prior to packaging.

Review commit: https://reviewboard.mozilla.org/r/104666/diff/#index_header
See other reviews: https://reviewboard.mozilla.org/r/104666/

Comment 2

[(not currently active) Ted Mielczarek]

Comment on attachment 8826791 [details]
Bug 1331090 - Strip the gtest libxul prior to packaging.

https://reviewboard.mozilla.org/r/104666/#review110674

I still can't believe we've been sticking an unstripped libxul in the gtest package all this time. Nice catch!

::: testing/testsuite-targets.mk:249
(Diff revision 1)
> +endif
> +endif
> +
>  stage-gtest: make-stage-dir
> -	$(NSINSTALL) -D $(PKG_STAGE)/gtest/gtest_bin
> +	$(NSINSTALL) -D $(PKG_STAGE)/gtest/gtest_bin/gtest
> +ifdef STRIP_COMPILED_TESTS

I bet it'd be nicer to do this work in a Python script where we could use the mozpack copier stuff. Might be a nice future improvement.

::: testing/testsuite-targets.mk:253
(Diff revision 1)
> -	$(NSINSTALL) -D $(PKG_STAGE)/gtest/gtest_bin
> +	$(NSINSTALL) -D $(PKG_STAGE)/gtest/gtest_bin/gtest
> +ifdef STRIP_COMPILED_TESTS
> +# The libxul file basename will vary per platform. Fortunately
> +# dependentlibs.list always lists the library name as its final line, so we
> +# can get the value from there.
> +	LIBXUL_BASE=$(shell tail -1 $(DIST)/bin/dependentlibs.list) && \

The mixing of Makefile `$(shell)` and bash variables here makes me a little uneasy. Can you just make that like:
```
LIBXUL_BASE=`tail -1 ...` &&
```

Comment 3

[Chris Manchester (limited bugmail, email directly)]

Comment on attachment 8826791 [details]
Bug 1331090 - Strip the gtest libxul prior to packaging.

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/104666/diff/1-2/

Comment 4

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/ce506ee62e4d
Strip the gtest libxul prior to packaging. r=ted

Comment 5

[Wes Kocher (:KWierso) (Not reading bugmail; email directly if needed)]

https://hg.mozilla.org/mozilla-central/rev/ce506ee62e4d