748467 - mobile/android/installer: dependency build

Status: RESOLVED FIXED

(Firefox Build System :: General, defect)

Description

[Joey Armstrong [:joey]]

dir: mobile/android/installer

Multiple attempts at a dependency build in this directory will generate ~200k of log output.  Track down where time is being spent and add make dependencies so work will only be done when needed.

Comment 1

[Kyle Huey (Exited; not receiving bugmail, old account, do not use)]

This bug basically boils down to "make package" is slow.  There aren't any obvious wins here, and I don't see anything we can achieve in the timeframe we're looking for.

Comment 2

[Kyle Huey (Exited; not receiving bugmail, old account, do not use)]

Attachment: Patch

Comment 3

[(not currently active) Ted Mielczarek]

Comment on attachment 624135 [details] [diff] [review]
Patch

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

This is kind of ugly but the concept is sound.

::: mobile/android/installer/Makefile.in
@@ +95,5 @@
>  endif
>  DEFINES += -DBINPATH=$(BINPATH)
>  
>  ifdef MOZ_PKG_MANIFEST_P
> +$(MOZ_PKG_MANIFEST): $(MOZ_PKG_MANIFEST_P) $(GLOBAL_DEPS)

Did we ever find out if this was needed for multi-locale builds? If so, you might want to stick $(if $(IS_LANGUAGE_REPACK),FORCE) in the deps here.

::: toolkit/mozapps/installer/packager-deps.py
@@ +1,4 @@
> +import os, sys
> +
> +filename = sys.argv[1]
> +with open(filename, 'r') as f:

You could import fileinput and use for line in fileinput.input(). (Handy module that reads stdin/whatever you pass on the cmdline.)

@@ +2,5 @@
> +
> +filename = sys.argv[1]
> +with open(filename, 'r') as f:
> +    for l in f.readlines():
> +        l.strip()

l = l.strip() (strip doesn't modify the original)

@@ +3,5 @@
> +filename = sys.argv[1]
> +with open(filename, 'r') as f:
> +    for l in f.readlines():
> +        l.strip()
> +        if l[:4] != "bin/":

if not l.startswith("bin/"):

This will work for mobile, but it kind of makes me sad. It won't work on OS X, because @BINPATH@ gets turned into the bundle name there, so maybe you should ifdef out this stuff if someone happens to use it where $(OS_TARGET) is Darwin?

::: toolkit/mozapps/installer/packager.mk
@@ +887,5 @@
>  	@echo "Compressing..."
>  	cd $(DIST) && $(MAKE_PACKAGE)
>  
> +ifdef MOZ_FAST_PACKAGE
> +MAKE_PACKAGE_DEPS = $(wildcard $(subst * , ,$(addprefix $(DIST)/bin/,$(shell $(PYTHON) $(topsrcdir)/toolkit/mozapps/installer/packager-deps.py $(MOZ_PKG_MANIFEST)))))

Why are you substing out the *? $(wildcard) will expand it for you.

@@ +894,5 @@
> +endif
> +
> +make-package: $(MAKE_PACKAGE_DEPS)
> +	$(MAKE) make-package-internal
> +	@touch $@

$(TOUCH)

Comment 4

[Ed Morley [:emorley]]

https://hg.mozilla.org/mozilla-central/rev/5b580c92e98a