Note: There are a few cases of duplicates in user autocompletion which are being worked on.

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

RESOLVED FIXED in Firefox 14

Status

()

Core
Build Config
RESOLVED FIXED
5 years ago
5 years ago

People

(Reporter: ted, Assigned: joduinn)

Tracking

Trunk
mozilla14
ARM
Android
Points:
---

Firefox Tracking Flags

(firefox14 fixed)

Details

Attachments

(2 attachments, 1 obsolete attachment)

(Reporter)

Description

5 years ago
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.
Created attachment 617405 [details] [diff] [review]
new makefile target to streamline the build-deploy-test-build-deploy-test cycle for fennec developers
Attachment #617405 - Flags: feedback?(ted.mielczarek)
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
(Reporter)

Comment 4

5 years ago
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+
(Reporter)

Comment 5

5 years ago
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.
(Reporter)

Updated

5 years ago
Assignee: nobody → joduinn
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.
Attachment #617735 - Flags: review?(ted.mielczarek)
(Reporter)

Updated

5 years ago
Attachment #617405 - Attachment is obsolete: true
(Reporter)

Comment 7

5 years ago
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+
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
Attachment #617862 - Flags: review+
Attachment #617862 - Flags: checkin?
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+

Updated

5 years ago
Target Milestone: --- → mozilla14
Blocks: 748452
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
Last Resolved: 5 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.
status-firefox14: --- → fixed
You need to log in before you can comment on or make changes to this bug.