Closed Bug 746741 Opened 8 years ago Closed 8 years ago

add makefile targets to encapsulate rebuild/repackage/install steps on android

Categories

(Firefox Build System :: General, defect)

ARM
Android
defect
Not set

Tracking

(firefox14 fixed)

RESOLVED FIXED
mozilla14
Tracking Status
firefox14 --- fixed

People

(Reporter: ted, Assigned: joduinn)

References

Details

Attachments

(2 files, 1 obsolete file)

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.
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?
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 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.)
Attachment #617405 - Flags: feedback?(ted.mielczarek) → feedback+
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.
Assignee: nobody → joduinn
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.
Attachment #617735 - Flags: review?(ted.mielczarek)
Attachment #617405 - Attachment is obsolete: true
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.
Attachment #617735 - Flags: review?(ted.mielczarek) → review+
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
Attachment #617862 - Flags: checkin? → checkin+
Target Milestone: --- → mozilla14
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
Attachment #617862 - Flags: approval-mozilla-aurora?
all done, landed on m-c. Flagging to land on aurora; per akeybl, this is now ok to close as FIXED.
Status: NEW → RESOLVED
Closed: 8 years ago
Resolution: --- → FIXED
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.
Attachment #617862 - Flags: approval-mozilla-aurora? → approval-mozilla-aurora+
This made the Aurora cutoff, so there's nothing left to do here.
Product: Core → Firefox Build System
You need to log in before you can comment on or make changes to this bug.