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

Status: RESOLVED FIXED

(Firefox Build System :: General, defect)

Description

[(not currently active) Ted Mielczarek]

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)]

Attachment: new makefile target to streamline the build-deploy-test-build-deploy-test cycle for fennec developers

Comment 2

[Mike Hommey [:glandium]]

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]]

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

[(not currently active) Ted Mielczarek]

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

[(not currently active) Ted Mielczarek]

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)]

Attachment: 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

[(not currently active) Ted Mielczarek]

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)]

Attachment: 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

[[e-mail] (:bhearsum)]

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)]

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)]

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]]

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 (Exited; not receiving bugmail, old account, do not use)]

This made the Aurora cutoff, so there's nothing left to do here.