Closed
Bug 752873
Opened 13 years ago
Closed 13 years ago
mobile/android/base/Makefile.in: $(shell cat) deps
Categories
(Firefox Build System :: General, defect)
Firefox Build System
General
Tracking
(Not tracked)
RESOLVED
FIXED
mozilla15
People
(Reporter: joey, Assigned: jhford)
References
Details
Attachments
(2 files, 1 obsolete file)
(deleted),
patch
|
khuey
:
review+
|
Details | Diff | Splinter Review |
(deleted),
text/plain
|
Details |
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•13 years ago
|
||
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•13 years ago
|
||
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.
Assignee | ||
Comment 3•13 years ago
|
||
I've created a pull requeset https://github.com/mozilla-services/android-sync/pull/185, which creates the manifests with one file per line
Assignee | ||
Comment 4•13 years ago
|
||
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.
Attachment #622419 -
Flags: review?(khuey)
Attachment #622419 -
Flags: feedback?(rnewman)
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+
Assignee | ||
Comment 6•13 years ago
|
||
This doesn't have the sample makefile generated, but I'll include that.
Assignee | ||
Comment 7•13 years ago
|
||
This is a sample makefile generated by my android-sync changes that I'm about to add to my pull request on github
Assignee | ||
Comment 8•13 years ago
|
||
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.
Attachment #622424 -
Flags: review?(khuey)
Attachment #622424 -
Flags: review?(khuey) → review+
Comment 9•13 years ago
|
||
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
Updated•13 years ago
|
Attachment #622419 -
Attachment is obsolete: true
Attachment #622419 -
Flags: feedback?(rnewman)
Assignee | ||
Comment 10•13 years ago
|
||
Comment 11•13 years ago
|
||
https://hg.mozilla.org/mozilla-central/rev/3a783d744843
https://hg.mozilla.org/mozilla-central/rev/b13f64598c37
Status: ASSIGNED → RESOLVED
Closed: 13 years ago
Resolution: --- → FIXED
Updated•7 years ago
|
Product: Core → Firefox Build System
You need to log in
before you can comment on or make changes to this bug.
Description
•