Closed Bug 1331090 Opened 3 years ago Closed 3 years ago

Strip the gtest libxul during packaging

Categories

(Firefox Build System :: General, defect)

defect
Not set

Tracking

(firefox54 fixed)

RESOLVED FIXED
mozilla54
Tracking Status
firefox54 --- fixed

People

(Reporter: cmanchester, Assigned: cmanchester)

Details

Attachments

(1 file)

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 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 ...` &&
```
Attachment #8826791 - Flags: review?(ted) → review+
Pushed by cmanchester@mozilla.com:
https://hg.mozilla.org/integration/autoland/rev/ce506ee62e4d
Strip the gtest libxul prior to packaging. r=ted
https://hg.mozilla.org/mozilla-central/rev/ce506ee62e4d
Status: NEW → RESOLVED
Closed: 3 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla54
Product: Core → Firefox Build System
You need to log in before you can comment on or make changes to this bug.