Last Comment Bug 752873 - mobile/android/base/Makefile.in: $(shell cat) deps
: mobile/android/base/Makefile.in: $(shell cat) deps
Status: RESOLVED FIXED
:
Product: Core
Classification: Components
Component: Build Config (show other bugs)
: unspecified
: All All
: -- normal (vote)
: mozilla15
Assigned To: John Ford [:jhford]
:
Mentors:
Depends on:
Blocks: 748452
  Show dependency treegraph
 
Reported: 2012-05-08 05:43 PDT by Joey Armstrong [:joey]
Modified: 2012-05-10 08:03 PDT (History)
5 users (show)
See Also:
Crash Signature:
(edit)
QA Whiteboard:
Iteration: ---
Points: ---
Has Regression Range: ---
Has STR: ---


Attachments
remove the tr call (2.15 KB, patch)
2012-05-09 11:02 PDT, John Ford [:jhford]
khuey: review+
Details | Diff | Review
alternate approach where the android-sync build system generates the makfile and we include it (1.44 KB, patch)
2012-05-09 11:22 PDT, John Ford [:jhford]
khuey: review+
Details | Diff | Review
sample makefile generated by android-sync changes (33.51 KB, text/plain)
2012-05-09 11:23 PDT, John Ford [:jhford]
no flags Details

Description Joey Armstrong [:joey] 2012-05-08 05:43:53 PDT
mobile/android/base/Makefile.in
===============================

Data loading 'shell cat' commands in this makefile can be optimized.  Another patch has wrapped these calls in a conditional so they will only impose overhead once when a lib* target is being processed.

Optimizations:

  o If the *.mn files are only used for makefile dependencies contents can be re-ordered to one file per line removing the need for calling the 'tr' command.

  o Replace shell overhead with a makefile include.  Add make targets to derive *.mk from *.mn data, possibly defining SYNC variables as needed.  Add deps to regenerate individual makefiles when *.mn changes.  A simple include of the data makefile should be enough to force regeneration.


SYNC_JAVA_FILES=$(shell cat $(topsrcdir)/mobile/android/sync/java-sources.mn | tr '\n' ' ';)
SYNC_PP_JAVA_FILES=$(shell cat $(topsrcdir)/mobile/android/sync/preprocess-sources.mn | tr '\n' ' ';)
SYNC_THIRDPARTY_JAVA_FILES=$(shell cat $(topsrcdir)/mobile/android/sync/java-third-party-sources.mn | tr '\n' ' ';)
SYNC_RES_DRAWABLE=$(shell cat $(topsrcdir)/mobile/android/sync/android-drawable-resources.mn | tr '\n' ' ';)
SYNC_RES_DRAWABLE_LDPI=$(shell cat $(topsrcdir)/mobile/android/sync/android-drawable-ldpi-resources.mn | tr '\n' ' ';)
SYNC_RES_DRAWABLE_MDPI=$(shell cat $(topsrcdir)/mobile/android/sync/android-drawable-mdpi-resources.mn | tr '\n' ' ';)
SYNC_RES_DRAWABLE_HDPI=$(shell cat $(topsrcdir)/mobile/android/sync/android-drawable-hdpi-resources.mn | tr '\n' ' ';)
SYNC_RES_LAYOUT=$(shell cat $(topsrcdir)/mobile/android/sync/android-layout-resources.mn | tr '\n' ' ';)
SYNC_RES_VALUES=$(shell cat $(topsrcdir)/mobile/android/sync/android-values-resources.mn | tr '\n' ' ';)
Comment 1 Ted Mielczarek [:ted.mielczarek] 2012-05-08 06:20:48 PDT
I'm not really sure if these files are used for anything else, CCing rnewman who is apparently the author of the script that creates them:
https://github.com/mozilla-services/android-sync/blob/develop/fennec-copy-code.sh#L45

If they only exist for the sake of the Fennec build, then we should be able to just generate Makefiles instead. Perhaps something like:
echo "SYNC_JAVA_SOURCES := $SOURCEFILES" > $SYNC/java-sources.mk

Then we could include that directly.
Comment 2 Richard Newman [:rnewman] 2012-05-08 08:53:08 PDT
We actually want to switch to using the file input to javac, rather than filenames-as-arguments, so we will still want some kind of manifest file for some of these. (That's Bug 736463.)

At that point the source files will be file-per-line.

I'd be very happy to take patches for these post-release; Fennec's and Sync's build system are a little bit rushed, to say the least.

I'm not wedded to a particular approach; I just suck at Make, and needed a solution quickly that wouldn't involve editing Fennec's Makefile every time we add a Sync source file.

These manifests are only used for Fennec's build.
Comment 3 John Ford [:jhford] 2012-05-09 10:53:24 PDT
I've created a pull requeset https://github.com/mozilla-services/android-sync/pull/185, which creates the manifests with one file per line
Comment 4 John Ford [:jhford] 2012-05-09 11:02:07 PDT
Created attachment 622419 [details] [diff] [review]
remove the tr call

This patch corresponds to my pull request in github.  This removes the call to tr from the mozilla build system.  I'll put together a patch that generates a makefile as well as the manifests.
Comment 5 Kyle Huey [:khuey] (khuey@mozilla.com) 2012-05-09 11:04:21 PDT
Comment on attachment 622419 [details] [diff] [review]
remove the tr call

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

If everything still works, r=me.
Comment 6 John Ford [:jhford] 2012-05-09 11:22:23 PDT
Created attachment 622424 [details] [diff] [review]
alternate approach where the android-sync build system generates the makfile and we include it

This doesn't have the sample makefile generated, but I'll include that.
Comment 7 John Ford [:jhford] 2012-05-09 11:23:19 PDT
Created attachment 622425 [details]
sample makefile generated by android-sync changes

This is a sample makefile generated by my android-sync changes that I'm about to add to my pull request on github
Comment 8 John Ford [:jhford] 2012-05-09 12:10:47 PDT
Comment on attachment 622424 [details] [diff] [review]
alternate approach where the android-sync build system generates the makfile and we include it

this works, https://tbpl.mozilla.org/?tree=Try&rev=d70761ec02ed and http://ftp.mozilla.org/pub/mozilla.org/firefox/try-builds/jford@mozilla.com-d70761ec02ed/try-android/try-android-bm14-try1-build1657.txt.gz

This does depend on https://github.com/mozilla-services/android-sync/pull/185 to work.  I did my own sync code drop for the try run.

Note You need to log in before you can comment on or make changes to this bug.