1048905 - Copy icon-font into system/fonts/hidden/ on build

Status: RESOLVED FIXED

(Core Graveyard :: Widget: Gonk, defect)

Comment 1

[Wilson Page [:wilsonpage]]

Attachment: pull-request (gaia:master)

Comment 2

[Wilson Page [:wilsonpage]]

Attachment: pull-request (gonk-misc:master)

Comment 3

[Yuren [:yurenju] (AFK ~ Oct. 12) (volunteer mode)]

Wilson, why do we need gaia.mk?

Comment 4

[Michael Wu [:mwu]]

That was my recommendation. PRODUCT_COPY_FILES is the standard (Android) way to specify installation of simple files and it makes sense to have it specified in the repo that the file comes from.

Comment 5

[Yuren [:yurenju] (AFK ~ Oct. 12) (volunteer mode)]

Comment on attachment 8467861 [details] [review]
pull-request (gaia:master)

also set feedback? to Tim since I'm not sure that's right if we push files outside /system/b2g/ on gaia build system. from my view it should be a part of B2G/gecko build system.

Comment 6

[Wilson Page [:wilsonpage]]

(In reply to Yuren [:yurenju] from comment #5)
> Comment on attachment 8467861 [details] [review]
> pull-request (gaia:master)
> 
> also set feedback? to Tim since I'm not sure that's right if we push files
> outside /system/b2g/ on gaia build system. from my view it should be a part
> of B2G/gecko build system.

We have a patch (attachment 8467862 [details] [review]) that covers the B2G/Gecko build side of things. We need the Gaia build patch so that development devices stay up to date with the latest font file.

Comment 7

[Tim Guan-tin Chien [:timdream] (please needinfo)]

Comment on attachment 8467861 [details] [review]
pull-request (gaia:master)

I don't have a strong opinion on what dir "Gaia" build script should cover, we define it ourselves based on our needs. 

In this case I think it make sense to use PRODUCT_COPY_FILES to interface with B2G build script (maybe all Gaia file should be copied this way).

However the push-to-device part is incomplete, maybe it should skip if:

-- if (APP !== '*') ?
-- if app(s) are not push to system partition, meaning we are not remounting the partition?

Comment 8

[Yuren [:yurenju] (AFK ~ Oct. 12) (volunteer mode)]

(In reply to Tim Guan-tin Chien [:timdream] (OOO ~Arg 9) (MoCo-TPE) (please ni?) from comment #7)
> -- if app(s) are not push to system partition, meaning we are not remounting
> the partition?

yes but we only need to remount to rw for system partition.

Comment 9

[Yuren [:yurenju] (AFK ~ Oct. 12) (volunteer mode)]

Comment on attachment 8467861 [details] [review]
pull-request (gaia:master)

Wilson, two issues:
1. just like Tim mentioned, please add if condition for the command (see comment on github)
2. this change break builg system unit test, you can run test by |make build-test-unit|, it can be fixed by a small patch for test case, will be attched.

Comment 10

[Yuren [:yurenju] (AFK ~ Oct. 12) (volunteer mode)]

Attachment: fix-unit-test.patch

Comment 11

[Wilson Page [:wilsonpage]]

Comment on attachment 8467861 [details] [review]
pull-request (gaia:master)

- Update to explicitly push to '//system/fonts/hidden/gaia-icons.ttf' so that adb push still succeeds when hidden/ dir doesn't exist
- Update so command doesn't get run on single app installs
- Fix unit-test with Yuren's patch

Comment 12

[Yuren [:yurenju] (AFK ~ Oct. 12) (volunteer mode)]

Comment on attachment 8467861 [details] [review]
pull-request (gaia:master)

since "gaia-icons.ttf" is added into push-to-devices.js, please also add it into push-to-devices.test.js to pass test, otherwise looks good, r=yurenju

Comment 13

[Yuren [:yurenju] (AFK ~ Oct. 12) (volunteer mode)]

noted, build unit test is not run on try server, so for now we need to run it manually, I will file a bug to enable it on try server

Comment 14

[Wilson Page [:wilsonpage]]

Comment on attachment 8467861 [details] [review]
pull-request (gaia:master)

Landed https://github.com/mozilla-b2g/gaia/commit/c4d36794ba8ffcf4a728d5d49cfb5efdc944f941

Comment 15

[Wilson Page [:wilsonpage]]

Comment on attachment 8467862 [details] [review]
pull-request (gonk-misc:master)

Landed https://github.com/mozilla-b2g/gonk-misc/commit/3bb61a27cd2941b2ba9b616a11aaa44269210396