Last Comment Bug 746741 - add makefile targets to encapsulate rebuild/repackage/install steps on android
: add makefile targets to encapsulate rebuild/repackage/install steps on android
Status: RESOLVED FIXED
:
Product: Core
Classification: Components
Component: Build Config (show other bugs)
: Trunk
: ARM Android
: -- normal (vote)
: mozilla14
Assigned To: John O'Duinn [:joduinn] (please use "needinfo?" flag)
:
Mentors:
Depends on:
Blocks: 748452
  Show dependency treegraph
 
Reported: 2012-04-18 14:30 PDT by Ted Mielczarek [:ted.mielczarek]
Modified: 2012-04-25 20:15 PDT (History)
6 users (show)
See Also:
Crash Signature:
(edit)
QA Whiteboard:
Iteration: ---
Points: ---
Has Regression Range: ---
Has STR: ---
fixed


Attachments
new makefile target to streamline the build-deploy-test-build-deploy-test cycle for fennec developers (650 bytes, patch)
2012-04-23 00:13 PDT, John O'Duinn [:joduinn] (please use "needinfo?" flag)
ted: feedback+
Details | Diff | Splinter Review
new makefile target to streamline the build-deploy-test-build-deploy-test cycle for fennec developers (revised) (596 bytes, patch)
2012-04-23 18:48 PDT, John O'Duinn [:joduinn] (please use "needinfo?" flag)
ted: review+
Details | Diff | Splinter Review
new makefile target to streamline the build-deploy-test-build-deploy-test cycle for fennec developers (revised) (596 bytes, text/plain)
2012-04-24 07:32 PDT, John O'Duinn [:joduinn] (please use "needinfo?" flag)
john+bugzilla: review+
akeybl: approval‑mozilla‑aurora+
bhearsum: checkin+
Details

Description Ted Mielczarek [:ted.mielczarek] 2012-04-18 14:30:28 PDT
Right now when you rebuild Fennec for Android you have to build then package then adb install. We should add Makefile targets to encapsulate these steps so it's a one-step operation for developers.
Comment 1 John O'Duinn [:joduinn] (please use "needinfo?" flag) 2012-04-23 00:13:50 PDT
Created attachment 617405 [details] [diff] [review]
new makefile target to streamline the build-deploy-test-build-deploy-test cycle for fennec developers
Comment 2 Mike Hommey [:glandium] 2012-04-23 00:43:49 PDT
One can just make -f client.mk package install. Or make -f client.mk build package install.
Arguably, make -f client.mk install should probably trigger package if the package is older than the stuff in dist/bin. But do we really need a new target?
Comment 3 Joey Armstrong [:joey] 2012-04-23 12:07:39 PDT
If you want to go one step farther it would be fairly easy to generalize the logic for aliasing arbitrary targets.

Add a prefix like 'alias-', 'targets-in-order-', ${prefix}-${delimited-list-of-targets}
Declare a wildcard target able to match prefix then split on delimiter and iterate:

alias: # nop
alias-%:
    $(foreach target,$(subst -,$(SPACE),$@),$(MAKE) -C $(OBJ) $(target))


Then drop it all into a named makefile

target_aliases.mk
=================
build_and_deply: alias-build-package-install
Comment 4 Ted Mielczarek [:ted.mielczarek] 2012-04-23 13:39:04 PDT
Comment on attachment 617405 [details] [diff] [review]
new makefile target to streamline the build-deploy-test-build-deploy-test cycle for fennec developers

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

::: client.mk
@@ +200,5 @@
>  
> +# Android/fennec equivilent
> +build_and_deploy: build
> +	$(MAKE) -C $(MOZ_OBJDIR) package
> +	$(MAKE) -C $(MOZ_OBJDIR) install

We could certainly bikeshed the target name all day, so I won't bother.

Calling this the Android "equivalent" is a bit weird, I'd just say it's a helper or something.

It's kind of odd to have build be a dependency and to explicitly run "make" on the other targets. You should either move package and install up to be dependencies, or move build down to be an explicit make. (Dependencies should work fine because client.mk defines .NOTPARALLEL below, FWIW.)
Comment 5 Ted Mielczarek [:ted.mielczarek] 2012-04-23 13:41:23 PDT
While I agree that you could simply "make -f client.mk build package install", I also think it's worthwhile to have a single target to encapsulate all that. Joey's suggestion is intriguing, but it doesn't offer much advantage over specifying all the individual targets, in that if the user leaves one out they won't get the desired behavior.
Comment 6 John O'Duinn [:joduinn] (please use "needinfo?" flag) 2012-04-23 18:48:36 PDT
Created attachment 617735 [details] [diff] [review]
new makefile target to streamline the build-deploy-test-build-deploy-test cycle for fennec developers  (revised)

after comments in f+, moved "package" and "install" to proper dependencies alongside "build". I left the comment about "equivalent" as it seemed in keeping with the existing "Windows equivalents" comment just above, but can change comment if that is important.

fyi: This works for clobber builds, and depend/incremental builds.
Comment 7 Ted Mielczarek [:ted.mielczarek] 2012-04-24 06:09:43 PDT
Comment on attachment 617735 [details] [diff] [review]
new makefile target to streamline the build-deploy-test-build-deploy-test cycle for fennec developers  (revised)

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

I'm not even sure what the "Windows equivalents" comment *means*, honestly. I think it's some weird bit of historical junk. I would prefer a different comment, but otherwise this is fine.
Comment 8 John O'Duinn [:joduinn] (please use "needinfo?" flag) 2012-04-24 07:32:09 PDT
Created attachment 617862 [details]
new makefile target to streamline the build-deploy-test-build-deploy-test cycle for fennec developers (revised)

revised comment, and got r+ in irc from ted
Comment 9 Ben Hearsum (:bhearsum) 2012-04-24 07:49:48 PDT
Comment on attachment 617862 [details]
new makefile target to streamline the build-deploy-test-build-deploy-test cycle for fennec developers (revised)

https://tbpl.mozilla.org/?rev=d7f017e8e572
http://hg.mozilla.org/mozilla-central/rev/d7f017e8e572
Comment 10 John O'Duinn [:joduinn] (please use "needinfo?" flag) 2012-04-24 15:27:22 PDT
Comment on attachment 617862 [details]
new makefile target to streamline the build-deploy-test-build-deploy-test cycle for fennec developers (revised)

[Approval Request Comment]
Regression caused by (bug #): n/a
User impact if declined: n/a. however, harder for fennec developers to do their work.
Testing completed (on m-c, etc.): clobber and depend builds work fine.
Risk to taking this patch (and alternatives if risky): zero risk, not changing existing targets, NPOTB
String changes made by this patch: n/a
Comment 11 John O'Duinn [:joduinn] (please use "needinfo?" flag) 2012-04-24 15:28:49 PDT
all done, landed on m-c. Flagging to land on aurora; per akeybl, this is now ok to close as FIXED.
Comment 12 Alex Keybl [:akeybl] 2012-04-25 12:46:20 PDT
Comment on attachment 617862 [details]
new makefile target to streamline the build-deploy-test-build-deploy-test cycle for fennec developers (revised)

[Triage Comment]
Approved for Aurora 14 since this is NPOTB.
Comment 13 Kyle Huey [:khuey] (khuey@mozilla.com) 2012-04-25 20:15:10 PDT
This made the Aurora cutoff, so there's nothing left to do here.

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