The default bug view has changed. See this FAQ.

mobile/android/installer: dependency build

RESOLVED FIXED in mozilla15

Status

()

Core
Build Config
RESOLVED FIXED
5 years ago
5 years ago

People

(Reporter: joey, Assigned: khuey)

Tracking

unspecified
mozilla15
Points:
---

Firefox Tracking Flags

(Not tracked)

Details

Attachments

(1 attachment)

(Reporter)

Description

5 years ago
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.
Blocks: 748452
Assignee: nobody → khuey
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.
Status: NEW → RESOLVED
Last Resolved: 5 years ago
Resolution: --- → WONTFIX
Created attachment 624135 [details] [diff] [review]
Patch
Attachment #624135 - Flags: review?(ted.mielczarek)
Status: RESOLVED → REOPENED
Resolution: WONTFIX → ---
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)
Attachment #624135 - Flags: review?(ted.mielczarek) → review+
https://hg.mozilla.org/mozilla-central/rev/5b580c92e98a
Status: REOPENED → RESOLVED
Last Resolved: 5 years ago5 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla15
You need to log in before you can comment on or make changes to this bug.