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

RESOLVED FIXED in 2.1 S2 (15aug)

Status

()

Core
Widget: Gonk
RESOLVED FIXED
3 years ago
3 years ago

People

(Reporter: wilsonpage, Assigned: wilsonpage)

Tracking

unspecified
2.1 S2 (15aug)
x86
Mac OS X
Points:
---

Firefox Tracking Flags

(Not tracked)

Details

Attachments

(3 attachments)

Comment hidden (empty)
(Assignee)

Comment 1

3 years ago
Created attachment 8467861 [details] [review]
pull-request (gaia:master)
Attachment #8467861 - Flags: review?(mwu)
(Assignee)

Comment 2

3 years ago
Created attachment 8467862 [details] [review]
pull-request (gonk-misc:master)
Attachment #8467862 - Flags: review?(mwu)

Updated

3 years ago
Attachment #8467861 - Flags: review?(mwu) → review+

Updated

3 years ago
Attachment #8467862 - Flags: review?(mwu) → review+
(Assignee)

Updated

3 years ago
Attachment #8467861 - Flags: review?(yurenju.mozilla)
Wilson, why do we need gaia.mk?
Flags: needinfo?(wilsonpage)

Comment 4

3 years ago
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.
Flags: needinfo?(wilsonpage)
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.
Attachment #8467861 - Flags: feedback?(timdream)
(Assignee)

Comment 6

3 years ago
(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 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?
Attachment #8467861 - Flags: feedback?(timdream)
(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 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.
Attachment #8467861 - Flags: review?(yurenju.mozilla)
Created attachment 8468974 [details] [diff] [review]
fix-unit-test.patch
(Assignee)

Comment 11

3 years ago
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
Attachment #8467861 - Flags: review?(yurenju.mozilla)
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
Attachment #8467861 - Flags: review?(yurenju.mozilla) → review+
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
See Also: → bug 1040658
(Assignee)

Comment 14

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

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

Comment 15

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

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

Updated

3 years ago
Status: NEW → RESOLVED
Last Resolved: 3 years ago
Resolution: --- → FIXED
(Assignee)

Updated

3 years ago
Blocks: 1036394
Target Milestone: --- → 2.1 S2 (15aug)
Duplicate of this bug: 1040663
You need to log in before you can comment on or make changes to this bug.