Makefile variables in mobile/android/base/android-services-files.mk are hard to parse

RESOLVED FIXED in mozilla21

Status

Android Background Services
Build & Test
P5
normal
RESOLVED FIXED
5 years ago
5 years ago

People

(Reporter: nalexander, Assigned: nalexander)

Tracking

unspecified
mozilla21
All
Android

Firefox Tracking Flags

(Not tracked)

Details

(URL)

Attachments

(1 attachment)

(Assignee)

Description

5 years ago
At the moment, we write

VAR := 1.java 2.java

Which, when you have 500+ files, makes it hard to see any differences.  We should write

VAR := \
  1.java \
  2.java \
  $(NULL)

which makes it easy to see what files were added or removed, and fits the Fennec Makefile.in style.
(Assignee)

Comment 1

5 years ago
Bug 736463 is relevant.
(Assignee)

Comment 2

5 years ago
Created attachment 707793 [details] [diff] [review]
Patch against m-i

Patch resulting from changes to Android Sync scripts.
Attachment #707793 - Flags: review?(rnewman)
(Assignee)

Updated

5 years ago
Assignee: nobody → nalexander
Priority: -- → P5
Comment on attachment 707793 [details] [diff] [review]
Patch against m-i

Review of attachment 707793 [details] [diff] [review]:
-----------------------------------------------------------------

::: mobile/android/base/android-services-files.mk
@@ +277,5 @@
> +  mobile/android/base/resources/drawable/pin_background.xml \
> +  $(NULL)
> +
> +SYNC_RES_DRAWABLE_LDPI := \
> +  $(NULL)

This doesn't seem right. Either you're not outputting stuff, or the var can get killed…
Attachment #707793 - Flags: review?(rnewman) → review+
(Assignee)

Comment 4

5 years ago
(In reply to Richard Newman [:rnewman] from comment #3)
> Comment on attachment 707793 [details] [diff] [review]
> Patch against m-i
> 
> Review of attachment 707793 [details] [diff] [review]:
> -----------------------------------------------------------------
> 
> ::: mobile/android/base/android-services-files.mk
> @@ +277,5 @@
> > +  mobile/android/base/resources/drawable/pin_background.xml \
> > +  $(NULL)
> > +
> > +SYNC_RES_DRAWABLE_LDPI := \
> > +  $(NULL)
> 
> This doesn't seem right. Either you're not outputting stuff, or the var can
> get killed…

It is right:

-SYNC_RES_DRAWABLE_LDPI := 
-SYNC_RES_DRAWABLE_MDPI := mobile/android/base/resources/drawable-mdpi/desktop.png mobile/android/base/resources/drawable-mdpi/mobile.png
-SYNC_RES_DRAWABLE_HDPI := 

The variable can be killed, but I'd rather keep it for when we actually will use it.

Comment 5

5 years ago
Try run for 9c60ed5b4161 is complete.
Detailed breakdown of the results available here:
    https://tbpl.mozilla.org/?tree=Try&rev=9c60ed5b4161
Results (out of 37 total builds):
    success: 31
    warnings: 6
Builds (or logs if builds failed) available at:
http://ftp.mozilla.org/pub/mozilla.org/firefox/try-builds/nalexander@mozilla.com-9c60ed5b4161
(Assignee)

Comment 6

5 years ago
https://hg.mozilla.org/integration/mozilla-inbound/rev/f13ebcc74a0d
Status: NEW → ASSIGNED
Target Milestone: --- → mozilla21
https://hg.mozilla.org/mozilla-central/rev/f13ebcc74a0d
Status: ASSIGNED → RESOLVED
Last Resolved: 5 years ago
Resolution: --- → FIXED
Component: Android Sync: Build & Test → Build & Test
Product: Mozilla Services → Android Background Services
You need to log in before you can comment on or make changes to this bug.