855003 - Need a build target for packaging the emulator

Status: RESOLVED FIXED

(Firefox OS Graveyard :: GonkIntegration, defect)

Description

[Jonathan Griffin (:jgriffin)]

We need to create a B2G build target for packaging up all the emulator files that are needed to run the emulator elsewhere (i.e., the current contents of emulator.zip).

This is needed to allow buildbot to automatically generate these packages for us.

Comment 1

[Jonathan Griffin (:jgriffin)]

FTW, I said in an IRC conversation with catlee that the build target would be:

./build.sh package-emulator

which would make an emulator.zip file in out/host.

Comment 2

[Jonathan Griffin (:jgriffin)]

Attachment: gonk-misc patch to add 'package-emulator' target

This adds a 'package-emulator' build target to B2G, that will create a standalone emulator.zip package from the build.  It codifies logic that currently resides in a couple of different shell scripts in Jenkins.

The package it produces is 121MB, which is quite an improvement over the 500MB+ packages that we currently make.

Comment 3

[John Ford [:jhford] CET/CEST Berlin Time]

(In reply to Jonathan Griffin (:jgriffin) from comment #2)
> Created attachment 730293 [details] [diff] [review]
> gonk-misc patch to add 'package-emulator' target
> 
> This adds a 'package-emulator' build target to B2G, that will create a
> standalone emulator.zip package from the build.  It codifies logic that
> currently resides in a couple of different shell scripts in Jenkins.
> 
> The package it produces is 121MB, which is quite an improvement over the
> 500MB+ packages that we currently make.

Any reason why this needs to be in gonk-misc/Android.mk?  Let's put it as a script in B2G.git/scripts instead.  I don't know the frequency with which we'll be running this packaging logic, but we can get a decent performance improvement by using gnu tar's --transform option to avoid staging the files with cp.  I'm not sure if infozip has a --transform equivalent or if using zip files is a hard requirement.

Comment 4

[Jonathan Griffin (:jgriffin)]

It lives in Android.mk so it can access vars like $HOST_OUT and $PRODUCT_OUT, which otherwise we'd have to guess.  Is there a reason it shouldn't live in Android.mk?

Comment 5

[John Ford [:jhford] CET/CEST Berlin Time]

(In reply to Jonathan Griffin (:jgriffin) from comment #4)
> It lives in Android.mk so it can access vars like $HOST_OUT and
> $PRODUCT_OUT, which otherwise we'd have to guess.  Is there a reason it
> shouldn't live in Android.mk?

It's adding a bunch of variables and scripting to the makefile.  Doing so gets messy quickly.  You can grab these variables using "get_build_var" from build/envsetup.sh in a script.

~/b2g/B2G $ cat get-variable.sh
#!/bin/bash
set -e
. setup.sh
PRODUCT_OUT=$(get_build_var PRODUCT_OUT)
HOST_OUT=$(get_build_var HOST_OUT)
echo HOST_OUT: $HOST_OUT PRODUCT_OUT: $PRODUCT_OUT
~/b2g/B2G $ ./get-variable.sh
<snip warnings about llvm-gcc, etc>
HOST_OUT: out/host/darwin-x86 PRODUCT_OUT: out/target/product/generic

Is there any other information that you'd need from the build system?

Comment 6

[Jonathan Griffin (:jgriffin)]

Ah, thanks.  I'll rewrite this as a script, then.

Comment 7

[Jonathan Griffin (:jgriffin)]

Attachment: B2G script to package emulator

Comment 8

[John Ford [:jhford] CET/CEST Berlin Time]

Comment on attachment 733136 [details] [diff] [review]
B2G script to package emulator

master: fc4a2b87306d22d094f3d8114b5b8e903059ffc1

I didn't have an emulator build handy, so I used a shortened list of files which didn't include anything generated by a build, and verified that {PRODUCT,HOST}_OUT worked.

A minor nit, the 'out' directory hard code can be avoided with:

diff --git a/scripts/package-emulator.sh b/scripts/package-emulator.sh
index 96a7e15..2cf713c 100755
--- a/scripts/package-emulator.sh
+++ b/scripts/package-emulator.sh
@@ -5,5 +5,5 @@ cd ..
 PRODUCT_OUT=$(get_build_var PRODUCT_OUT)
 HOST_OUT=$(get_build_var HOST_OUT)
-TOP=$(pwd)
+OUT_DIR=$(get_abs_build_var OUT_DIR)

 EMULATOR_FILES=(\
@@ -26,5 +26,5 @@ EMULATOR_FILES=(\
        ${PRODUCT_OUT}/hardware-qemu.ini)

-EMULATOR_ARCHIVE="${TOP}/out/emulator.tar.gz"
+EMULATOR_ARCHIVE="${OUT_DIR}/emulator.tar.gz"

 echo "Creating emulator archive at $EMULATOR_ARCHIVE"

Comment 9

[John Ford [:jhford] CET/CEST Berlin Time]

Landed diff in comment 8
master: b8aa6b4f02ec83b4c7e0ea221e778826b111a911

Comment 10

[Jonathan Griffin (:jgriffin)]

Thanks jhford.

Catlee, this adds a 'package-emulator.sh' script to the 'scripts' directory of the B2G repo; you can call this to package up the emulator as out/emulator.tar.gz.

Comment 11

[Chris AtLee [:catlee]]

doh

why wasn't this moved into a build target? the current implementation means we have to special case packaging for emulators

Comment 12

[Jonathan Griffin (:jgriffin)]

(In reply to Chris AtLee [:catlee] from comment #11)
> doh
> 
> why wasn't this moved into a build target? the current implementation means
> we have to special case packaging for emulators

mwu didn't want it as a build target.

Comment 13

[Jonathan Griffin (:jgriffin)]

Sorry, it was jhford that axed the build target idea.  I'll let him explain the reasoning.

Comment 14

[John Ford [:jhford] CET/CEST Berlin Time]

It's 11:30pm, I'm exhausted and have a flight in 4 hours, so i hope this is coherent.

It added a lot of variables and phony recipes which makes the makefile messier.  I'm not seeing any package target definitions anywhere else.  Where is the code that is defined for other devices that makes this the emulator a special case?

Comment 15

[Chris AtLee [:catlee]]

'build.sh' produces packages for most targets that are uploadable as the final build artifacts.

In some cases, we need to call specific targets in build.sh to produce specific packages, e.g. 
https://hg.mozilla.org/mozilla-central/file/c74f44828165/b2g/config/panda/config.json#l7

means we call 'build.sh boottarball systemtarball userdatatarball package-tests' for panda builds.

having a build.sh target for emulator packages would greatly simply the integration with build automation, we could use the hooks that already exist.

could 'package-emulator' be added as a build.sh target that calls this new script?

Comment 16

[Dylan Oliver [:doliver]]

Mike, :jhford is on PTO this week. Can you provide input on Chris' suggestion in comment #15?

Comment 17

[John Ford [:jhford] CET/CEST Berlin Time]

(In reply to Chris AtLee [:catlee] from comment #15)
> could 'package-emulator' be added as a build.sh target that calls this new
> script?

That sounds reasonable.  gonk-misc/Android.mk is a good place for the rule to live.  It might make life easier if the script is moved from B2G.git to gonk-misc.

It might be worthwhile to adapt the script to optionally be able to use the build system variables passed in as environment variables when invoked from the rule.

Comment 18

[Jonathan Griffin (:jgriffin)]

Attachment: ./build.sh target for package-emulator

If this lands, I'll remove package-emulator.sh from B2G.git.

Comment 19

[John Ford [:jhford] CET/CEST Berlin Time]

Comment on attachment 743698 [details] [diff] [review]
./build.sh target for package-emulator

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

This looks good, assuming the switch to using OUT_DIR instead of hardcoding $(pwd)/out/...

::: package-emulator.sh
@@ +22,5 @@
> +	${PRODUCT_OUT}/system.img \
> +	${PRODUCT_OUT}/userdata.img \
> +	${PRODUCT_OUT}/ramdisk.img)
> +
> +EMULATOR_ARCHIVE="${TOP}/out/emulator.tar.gz"

Instead of hardcoding the /out/ directory, can you instead use the OUT_DIR build system variable?

e.g.:
EMULATOR_ARCHIVE="${OUT_DIR}/emulator.tar.gz"

Comment 20

[Michael Wu [:mwu]]

Comment on attachment 743698 [details] [diff] [review]
./build.sh target for package-emulator

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

This looks like it could be trivially done correctly in Android.mk. The original patch was definitely overcomplicated, but this belongs in a makefile rule if you're only calling tar. Don't use phony for this though - make it dependent on the files that it's sticking into the tarball.

Comment 21

[Jonathan Griffin (:jgriffin)]

Attachment: ./build.sh target for package-emulator

Review comments addressed.

Comment 22

[Michael Wu [:mwu]]

Comment on attachment 743736 [details] [diff] [review]
./build.sh target for package-emulator

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

::: Android.mk
@@ +304,5 @@
> +	$(PRODUCT_OUT)/system.img \
> +	$(PRODUCT_OUT)/userdata.img \
> +	$(PRODUCT_OUT)/ramdisk.img
> +$(EMULATOR_FILES): package-emulator
> +package-emulator:

$(EMULATOR_FILES): package-emulator makes those files dependent on package-emulator.

I think you meant something like:

package-emulator: $(EMULATOR_FILES)

or something like this, which is more "correct":

EMULATOR_ARCHIVE:="$(OUT_DIR)/emulator.tar.gz"
package-emulator: $(EMULATOR_ARCHIVE)
$(EMULATOR_ARCHIVE): $(EMULATOR_FILES)

And if you do this, you can use automatic variables to specify the input and output files. http://www.gnu.org/software/make/manual/html_node/Automatic-Variables.html

Comment 23

[Jonathan Griffin (:jgriffin)]

Attachment: ./build.sh target for package-emulator

Cool, thanks, I'm improving my makefile fu.

Comment 24

[Michael Wu [:mwu]]

Comment on attachment 743879 [details] [diff] [review]
./build.sh target for package-emulator

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

Looks good. I think this belongs in the emulator device directory rather than gonk-misc, but we can move it later. (we don't currently have a emulator device directory)

::: Android.mk
@@ +307,5 @@
> +EMULATOR_ARCHIVE:="$(OUT_DIR)/emulator.tar.gz"
> +package-emulator: $(EMULATOR_ARCHIVE)
> +$(EMULATOR_ARCHIVE): $(EMULATOR_FILES)
> +	echo "Creating emulator archive at $@" && \
> +	rm -rf $@ && \

rm -f might be better since we're not expecting emulator.tar.gz to be a directory.

Comment 25

[Jonathan Griffin (:jgriffin)]

https://github.com/mozilla-b2g/gonk-misc/commit/212dcdd38e84020fcd230d1038aa6abea170c7a2