Closed Bug 752873 Opened 12 years ago Closed 12 years ago
.in: $(shell cat) deps
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' ' ';)
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.
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.
I've created a pull requeset https://github.com/mozilla-services/android-sync/pull/185, which creates the manifests with one file per line
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 on attachment 622419 [details] [diff] [review] remove the tr call Review of attachment 622419 [details] [diff] [review]: ----------------------------------------------------------------- If everything still works, r=me.
Attachment #622419 - Flags: review?(khuey) → review+
This doesn't have the sample makefile generated, but I'll include that.
This is a sample makefile generated by my android-sync changes that I'm about to add to my pull request on github
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://firstname.lastname@example.org/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.
Attachment #622424 - Flags: review?(khuey)
Attachment #622424 - Flags: review?(khuey) → review+
Merged with fixes: https://github.com/mozilla-services/android-sync/commit/35d76d9946a258ca40cd2b46aad64938f3dd2011 Landed on inbound: https://hg.mozilla.org/integration/mozilla-inbound/rev/3a783d744843 https://hg.mozilla.org/integration/mozilla-inbound/rev/b13f64598c37
Assignee: nobody → jhford
Status: NEW → ASSIGNED
Target Milestone: --- → mozilla15
12 years ago
Status: ASSIGNED → RESOLVED
Closed: 12 years ago
Resolution: --- → FIXED
You need to log in before you can comment on or make changes to this bug.