Apparently our dependencies for Java code are not working properly, and result in lots of Java files being recompiled every time. We should figure out what's going on there and fix it.

http://mxr.mozilla.org/mozilla-central/source/mobile/android/base/Makefile.in http://mxr.mozilla.org/mozilla-central/source/embedding/android/Makefile.in

Minor optimization: Add a --build-increment option in make-platform.py and the double-$(shell) + commands will be a little more efficient. Also add a call to $(checkIfEmpty,ANDROID_VERSION_CODE) to detect processing errors. ifeq (,$(ANDROID_VERSION_CODE)) # increment the version code by 1 so xul fennec will win any compatability ties ANDROID_VERSION_CODE=$(shell echo `$(PYTHON) $(topsrcdir)/toolkit/xre/make-platformini.py --print-buildid | cut -c1-10` + 1 | bc) endif

per mtg w/joey, khuey: cleaning up bug dependencies.

bug 747540:

bug 751156: robocop dependency build should be a nop

bug 751156: robocop dependency build should be a nop

Comment on attachment 698743 [details] [diff] [review] * * * Added library makefile java-build.mk as a place to gather java logic. Creation & removal of classes/ and res/ moved to library routines. export:: target for installing res/ resource files moved into j-b.mk. Dependencies added to only copy when sources are modified. Triggers are definition of RES_FILES= and/or JAVA_FILES=. [note] /robocop/ can also make use of this logic but omitted due to retry/failures logged on android. Cleanups: - replaced preq and target use with $< and $@ to remove duplication and shorten long lines. - Removed special case .mkdir.done target, no longer needed. - Remove dups from GENERATED_DIRS macro - remove trailing whitespace from assignments - insert a whitespace between operator and line continuation to follow coding style guide

Comment on attachment 698743 [details] [diff] [review] * * * Review of attachment 698743 [details] [diff] [review]: ----------------------------------------------------------------- ::: config/makefiles/java-build.mk @@ +8,5 @@ > +ifndef INCLUDED_JAVA_BUILD_MK #{ > + > +ifdef RES_FILES #{ > +GENERATED_DIRS += res > +GARBAGE += .deps-copy-res I think you mean .deps-copy-java-res here? @@ +15,5 @@ > + > +# nop-build: only copy res/ files when needed > +.deps-copy-java-res: $(foreach fyl,$(RES_FILES),$(srcdir)/$(fyl)) $(call mkdir_deps,res) > + $(call copy_dir,$(srcdir)/res,$(CURDIR)/res) > + @$(TOUCH) $@ Is there a benefit to copying the whole res/ directory when any file in srcdir/res changes? Would something like this work instead to copy only the files that have changed? export:: $(RES_FILES) $(RES_FILES): %: $(srcdir)/% cp $< $@ Also, what happens if a file in srcdir is removed?

(In reply to Michael Shal [:mshal] from comment #8) > Comment on attachment 698743 [details] [diff] [review] > * * * > > Review of attachment 698743 [details] [diff] [review]: > ----------------------------------------------------------------- > > ::: config/makefiles/java-build.mk > @@ +8,5 @@ > > +ifndef INCLUDED_JAVA_BUILD_MK #{ > > + > > +ifdef RES_FILES #{ > > +GENERATED_DIRS += res > > +GARBAGE += .deps-copy-res > > I think you mean .deps-copy-java-res here? > > @@ +15,5 @@ > > + > > +# nop-build: only copy res/ files when needed > > +.deps-copy-java-res: $(foreach fyl,$(RES_FILES),$(srcdir)/$(fyl)) $(call mkdir_deps,res) > > + $(call copy_dir,$(srcdir)/res,$(CURDIR)/res) > > + @$(TOUCH) $@ > > Is there a benefit to copying the whole res/ directory when any file in > srcdir/res changes? Would something like this work instead to copy only the > files that have changed? My preference would be to simply use a one liner like rsync to only copy content as needed and be done with it. The only benefit I can for copying an entire directory w/o dependencies would be constant overhead for shells spawned. Maybe tar use has been preferred so syntax can be later replaced by an nsinstall.py call ? > export:: $(RES_FILES) > > $(RES_FILES): %: $(srcdir)/% > cp $< $@ > > Also, what happens if a file in srcdir is removed? rsync --delete would also handle this case properly but yes any of the raw copy operations will retain files in the destination until clean or clobber has been run. That may be why some of the install commands were written as $(RM); tar so the directory copy would be clean.

mshal review: declare a local var to fix dep filename typo

Comment on attachment 699332 [details] [diff] [review] fix dependencies for Java builds Review of attachment 699332 [details] [diff] [review]: ----------------------------------------------------------------- ::: config/makefiles/autotargets.mk @@ -68,5 @@ > -ifndef NOWARN_AUTOTARGETS # { > - @echo "WARNING: $(MKDIR) -dot- requested by $(MAKE) -C $(CURDIR) $(MAKECMDGOALS)" > - @$(TOUCH) -t 198001030000 $@ > -endif #} > - Why remove this? ::: config/makefiles/java-build.mk @@ +15,5 @@ > + > +export:: $(res-dep) > + > +# nop-build: only copy res/ files when needed > +$(res-dep): $(foreach fyl,$(RES_FILES),$(srcdir)/$(fyl)) $(call mkdir_deps,res) $(addprefix $(srcdir)/,$(RES_FILES)) @@ +18,5 @@ > +# nop-build: only copy res/ files when needed > +$(res-dep): $(foreach fyl,$(RES_FILES),$(srcdir)/$(fyl)) $(call mkdir_deps,res) > + $(call copy_dir,$(srcdir)/res,$(CURDIR)/res) > + @$(TOUCH) $@ > +endif #} I'm not sure this is not going to create problems @@ +22,5 @@ > +endif #} > + > + > +ifdef JAVAFILES #{ > +GENERATED_DIRS += classes This doesn't sound right, considering the directory is "classes" only because that's what is given to the -d argument to $(JAVAC). If you're going to make java build rules, make the $(JAVAC) rules part of it.

(In reply to Mike Hommey [:glandium] from comment #11) > Comment on attachment 699332 [details] [diff] [review] > fix dependencies for Java builds > > Review of attachment 699332 [details] [diff] [review]: > ----------------------------------------------------------------- > > ::: config/makefiles/autotargets.mk > @@ -68,5 @@ > > -ifndef NOWARN_AUTOTARGETS # { > > - @echo "WARNING: $(MKDIR) -dot- requested by $(MAKE) -C $(CURDIR) $(MAKECMDGOALS)" > > - @$(TOUCH) -t 198001030000 $@ > > -endif #} > > - > > Why remove this? The rule was added initially as a stub to trap creation requests w/o a directory argument. Failure mode no longer being reported by a recent clean build. > ::: config/makefiles/java-build.mk > @@ +15,5 @@ > > + > > +export:: $(res-dep) > > + > > +# nop-build: only copy res/ files when needed > > +$(res-dep): $(foreach fyl,$(RES_FILES),$(srcdir)/$(fyl)) $(call mkdir_deps,res) > > $(addprefix $(srcdir)/,$(RES_FILES)) heh yes argument list length should not be a problem here :) > @@ +18,5 @@ > > +# nop-build: only copy res/ files when needed > > +$(res-dep): $(foreach fyl,$(RES_FILES),$(srcdir)/$(fyl)) $(call mkdir_deps,res) > > + $(call copy_dir,$(srcdir)/res,$(CURDIR)/res) > > + @$(TOUCH) $@ > > +endif #} > > I'm not sure this is not going to create problems Any problems will be due to the list of copied resources not being maintained correct ? If dynamic gathering is not an option due to overhead - a unit test or checkup script would be able to catch anything that falls through the cracks. Added deps will fix the nop build problem of having to blindly copy the directory just in case. > @@ +22,5 @@ > > +endif #} > > + > > + > > +ifdef JAVAFILES #{ > > +GENERATED_DIRS += classes > > This doesn't sound right, considering the directory is "classes" only > because that's what is given to the -d argument to $(JAVAC). If you're going > to make java build rules, make the $(JAVAC) rules part of it. Yes more of the java logic can be refactored out as part of a later revision. This logic is simply laying the groundwork to create a library makefile where java logic can be relocated gradually rather than in bulk. The only dep required at the moment is asynchronous creation of classes/ when $(SOURCE) is defined.

> > > +# nop-build: only copy res/ files when needed > > > +$(res-dep): $(foreach fyl,$(RES_FILES),$(srcdir)/$(fyl)) $(call mkdir_deps,res) > > > + $(call copy_dir,$(srcdir)/res,$(CURDIR)/res) > > > + @$(TOUCH) $@ > > > +endif #} > > > > I'm not sure this is not going to create problems > > Any problems will be due to the list of copied resources not being > maintained correct ? Well $(if $(IS_LANGUAGE_REPACK),FORCE) should probably be added for good measure.

Comment on attachment 701128 [details] [diff] [review] fix dependencies for Java builds [note] probably should create a sutagent-specific bug for this patch and use the java dep problem but as a container. nop builds within sutagent/ directories now succeed adding no processing overhead on subsequent calls. mshal: Declare a local var to hold the resource dependency timestamp so it can be used consistently {GARBAGE += $(ts)}. Replace $(foreach loop) with $(addprefix). No long argument lists or filename quoting will be needed. Added IS_LANGUAGE_REPACK + FORCE to the java-build.mk export:: target so the library can copy files when modified or force a full copy for l10n builds. A few cosmetic makefile cleanups per the coding standards page. Remove trailing whitespace, whitespace inserted between operators and line continuation, etc.

Comment on attachment 701128 [details] [diff] [review] fix dependencies for Java builds Review of attachment 701128 [details] [diff] [review]: ----------------------------------------------------------------- ::: config/makefiles/autotargets.mk @@ -68,5 @@ > -ifndef NOWARN_AUTOTARGETS # { > - @echo "WARNING: $(MKDIR) -dot- requested by $(MAKE) -C $(CURDIR) $(MAKECMDGOALS)" > - @$(TOUCH) -t 198001030000 $@ > -endif #} > - > > Why remove this? > > The rule was added initially as a stub to trap creation requests w/o a > directory argument. Failure mode no longer being reported by a recent clean > build. It's not good practice to make changes to the build systems that are unrelated to the bug. There are a lot of unrelated changes in this patch. At the very least, please do them in a separate patch, although a separate bug would be better. ::: config/makefiles/java-build.mk @@ +28,5 @@ > +endif #} > + > + > +ifdef JAVAFILES #{ > +GENERATED_DIRS += classes Unless you move the java compilation rules, this should stay in each Makefile.in.

(In reply to Mike Hommey [:glandium] from comment #16) > Comment on attachment 701128 [details] [diff] [review] > fix dependencies for Java builds > > Review of attachment 701128 [details] [diff] [review]: > ----------------------------------------------------------------- > > ::: config/makefiles/autotargets.mk > @@ -68,5 @@ > > -ifndef NOWARN_AUTOTARGETS # { > > - @echo "WARNING: $(MKDIR) -dot- requested by $(MAKE) -C $(CURDIR) $(MAKECMDGOALS)" > > - @$(TOUCH) -t 198001030000 $@ > > -endif #} > > - > > > > Why remove this? > > > > The rule was added initially as a stub to trap creation requests w/o a > > directory argument. Failure mode no longer being reported by a recent clean > > build. > > It's not good practice to make changes to the build systems that are > unrelated to the bug. There are a lot of unrelated changes in this patch. At > the very least, please do them in a separate patch, although a separate bug > would be better. Ok fair enough > ::: config/makefiles/java-build.mk > @@ +28,5 @@ > > +endif #} > > + > > + > > +ifdef JAVAFILES #{ > > +GENERATED_DIRS += classes > > Unless you move the java compilation rules, this should stay in each > Makefile.in. Why needlessly make the problem space larger, more complex ? If logic is moved gradually rather than all at once there will be a far greater chance for breakage and affected scope will be much smaller. Rules can be moved into the library makefile later {even unused} as an initial step. Then change makefiles-by-directory one by one until the have all transitioned. Adding rules and widening patch scope will only delay deployment.

(In reply to Joey Armstrong [:joey] from comment #17) > (In reply to Mike Hommey [:glandium] from comment #16) > > Comment on attachment 701128 [details] [diff] [review] > > fix dependencies for Java builds > > > > Review of attachment 701128 [details] [diff] [review]: > > ----------------------------------------------------------------- > > > +ifdef JAVAFILES #{ > > > +GENERATED_DIRS += classes > > > > Unless you move the java compilation rules, this should stay in each > > Makefile.in. > > Why needlessly make the problem space larger, more complex ? If logic is > moved gradually rather than all at once there will be a far greater chance > for breakage and affected scope will be much smaller. reverse(text) - if rules are gathered into java-build.mk and all makefiles are updated in one patch the chance of breakage will be much higher than adding unused rules then later updating individual directories to use library logic for building rather than inlined makefile logic.

(In reply to Joey Armstrong [:joey] from comment #17) > Why needlessly make the problem space larger, more complex ? If logic is > moved gradually rather than all at once there will be a far greater chance > for breakage and affected scope will be much smaller. > > Rules can be moved into the library makefile later {even unused} as an > initial step. Then change makefiles-by-directory one by one until the have > all transitioned. Adding rules and widening patch scope will only delay > deployment. ... which is why you should just leave the classes directory alone, and leave it in GARBAGE_DIRS, where it currently is.

(In reply to Mike Hommey [:glandium] from comment #19) > (In reply to Joey Armstrong [:joey] from comment #17) > > Why needlessly make the problem space larger, more complex ? If logic is > > moved gradually rather than all at once there will be a far greater chance > > for breakage and affected scope will be much smaller. > > > > Rules can be moved into the library makefile later {even unused} as an > > initial step. Then change makefiles-by-directory one by one until the have > > all transitioned. Adding rules and widening patch scope will only delay > > deployment. > > ... which is why you should just leave the classes directory alone, and > leave it in GARBAGE_DIRS, where it currently is. Why ? That answer would require retaining export:: targets that duplicate logic in the sandbox makefiles along with retaining a few explicit inlined "nsinstall -D classes" calls. These deps are part of the reason why nop builds are not functioning properly. The patch is refactoring java logic into a dedicated library. Allows removing duplicate logic for class creation/removal and addresses the nop build problem. Not sure why any of these would be an issue.

(In reply to Joey Armstrong [:joey] from comment #20) > Why ? That answer would require retaining export:: targets that duplicate > logic in the sandbox makefiles along with retaining a few explicit inlined > "nsinstall -D classes" calls. These deps are part of the reason why nop > builds are not functioning properly. Oh I see. Then replace put classes in GENERATED_DIRS, but keep that in the individual makefiles.

(In reply to Mike Hommey [:glandium] from comment #21) > (In reply to Joey Armstrong [:joey] from comment #20) > > Why ? That answer would require retaining export:: targets that duplicate > > logic in the sandbox makefiles along with retaining a few explicit inlined > > "nsinstall -D classes" calls. These deps are part of the reason why nop > > builds are not functioning properly. > > Oh I see. Then replace put classes in GENERATED_DIRS, but keep that in the > individual makefiles. Why duplicate the setting within all makefiles that compile java source when the assignment can be made in one place within a central library ?

(In reply to Joey Armstrong [:joey] from comment #22) > Why duplicate the setting within all makefiles that compile java source when > the assignment can be made in one place within a central library ? Because unless you centralize the rules, it's a lie.

(In reply to Mike Hommey [:glandium] from comment #23) > (In reply to Joey Armstrong [:joey] from comment #22) > > Why duplicate the setting within all makefiles that compile java source when > > the assignment can be made in one place within a central library ? > > Because unless you centralize the rules, it's a lie. That is exactly what the patch is setting up. A central makefile to contain java logic, and in this case handle directory and resource creation. The part this patch is not doing is attempting to move everything java related in one monolithic patch. Think of it as a form of staging, java-build.mk is a place where everything java related will eventually end up. For the initial pass start small/non-invasive. Only move creation of the directory and resource classes. Afterward target rules, etc can be refactored and isolated into the library.

must have bzexported the wrong sandbox last time. .mkdir.done target rule removal was moved to another patch.

Comment on attachment 702974 [details] [diff] [review] fix dependencies for java build Refresh patch to remove .mkdir.dep target moved into patch for bug 830351. Common logic for processing res{ource}/ and classes/ directory refactored into java-build.mk. Related nop build problems corrected by adding deps to only copy content when modified.

Comment on attachment 702974 [details] [diff] [review] fix dependencies for java build Review of attachment 702974 [details] [diff] [review]: ----------------------------------------------------------------- whatever

(In reply to Mike Hommey [:glandium] from comment #28) > Comment on attachment 702974 [details] [diff] [review] > fix dependencies for java build > > Review of attachment 702974 [details] [diff] [review]: > ----------------------------------------------------------------- > > whatever Well is there a good reason for not pulling the logic out of individual makefiles and placing it within a central library ?

(In reply to Joey Armstrong [:joey] from comment #29) > Well is there a good reason for not pulling the logic out of individual > makefiles and placing it within a central library ? For the last time, because unless the rules doing $(JAVAC) -d classes move as well, putting GENERATED_DIRS = classes in a central place is a lie. But I don't care anymore.

(In reply to Mike Hommey [:glandium] from comment #30) > (In reply to Joey Armstrong [:joey] from comment #29) > > Well is there a good reason for not pulling the logic out of individual > > makefiles and placing it within a central library ? > > For the last time, because unless the rules doing $(JAVAC) -d classes move > as well, putting GENERATED_DIRS = classes in a central place is a lie. > But I don't care anymore. Well I guess we just have a difference of opinion on this because this dep is simply an element of the java build and can be handled independent of any rule attempting to make use of it. Also breaking the dependency between classes/ and $(JAVAC) -d would help save some file stats by no longer having to list classes/ as a pre-requisite.

inbound changeset: 8104d8262f22

It's [leave open], adjusting (see https://wiki.mozilla.org/Tree_Rules/Inbound#Please_do_the_following_after_pushing_to_inbound) :-)

Comment on attachment 728214 [details] [diff] [review] fix dependencies for java builds Added a javac rule in java-build.mk to generalize compiling files assigned to JAVAFILES=. Target rule uses an added dependency to limit recompiles to source changes. Pay attention to IS_LANGUAGE_REPACK so work can be FORCE'd when necessary Added target 'java-pre-process' used to synchronize PP_TARGET (*.java.in->*.java) with compiling generated files. Added several targets to GARBAGE so the clean target can function properly. Added a few missing deps in Makefile.in that should have prompted classes.dex to regenerate. robocop: added an explicit rule to generate R.java that will correct a chicken-n-egg problem when building in a clean directory. Replaced hardcoded elements within target rules, use autovariables ($<) to remove redundancy. Makefile cleanup: removed export: res/ and related makefile logic. An earlier patch moved this logic into java-build.sh and pays attention to RES_FILES=. Inlined export target now redundant and will force extra work. config/makefiles/autotargets.mk o logic originally from bug 750303 with usage/l10n related dep failures removed. o added: fully populate GARBAGE_DIRS from GENERATED_DIRS* so make clean will work properly. o added: when no goal is specified assume the .default targets from rules.mk. This is likely what caused l10n repack failures for 750303.

Comment on attachment 728214 [details] [diff] [review] fix dependencies for java builds >diff --git a/build/mobile/sutagent/android/Makefile.in b/build/mobile/sutagent/android/Makefile.in >--- a/build/mobile/sutagent/android/Makefile.in >+++ b/build/mobile/sutagent/android/Makefile.in > GARBAGE += \ > AndroidManifest.xml \ > classes.dex \ >+ R.java \ > sutAgentAndroid.apk \ > sutAgentAndroid.ap_ \ > sutAgentAndroid-unsigned-unaligned.apk \ > sutAgentAndroid-unaligned.apk \ > $(NULL) How is R.java GARBAGE here? It looks like R.java is checked in, not generated (but, see below...) >diff --git a/build/mobile/sutagent/android/fencp/Makefile.in b/build/mobile/sutagent/android/fencp/Makefile.in >--- a/build/mobile/sutagent/android/fencp/Makefile.in >+++ b/build/mobile/sutagent/android/fencp/Makefile.in >+# Should R.java be checked in ? Seems strange that it is, given that it says it is autogenerated and should not be modified. Should these all be deleted from version control and replaced by rules to generate them? >diff --git a/config/makefiles/autotargets.mk b/config/makefiles/autotargets.mk >--- a/config/makefiles/autotargets.mk >+++ b/config/makefiles/autotargets.mk >+GENERATED_DIRS += $(foreach tgt,$(_mcg_),$(value GENERATED_DIRS_$(tgt))) >+GENERATED_DIRS := $(call expand,GENERATED_DIRS) >+GENERATED_DIRS := $(strip $(sort $(GENERATED_DIRS))) >+ >+# Derive a full list for clean/... >+GARBAGE_DIRS := $(strip $(sort \ >+ $(GARBAGE_DIRS) \ >+ $(foreach var,$(filter GENERATED_DIRS%,$(.VARIABLES)),$(call expand,$(var))) \ >+ )) >+ >+# directory deps are timestamp based to only create when needed >+AUTO_DEPS += $(if $(value GENERATED_DIRS),$(call mkdir_deps,GENERATED_DIRS)) Call the above chunk of code part A... >diff --git a/config/makefiles/java-build.mk b/config/makefiles/java-build.mk >--- a/config/makefiles/java-build.mk >+++ b/config/makefiles/java-build.mk >@@ -2,39 +2,77 @@ > # vim:set ts=8 sw=8 sts=8 noet: > # > # This Source Code Form is subject to the terms of the Mozilla Public > # License, v. 2.0. If a copy of the MPL was not distributed with this file, > # You can obtain one at http://mozilla.org/MPL/2.0/. > > ifndef INCLUDED_JAVA_BUILD_MK #{ > >+ >+########################################################################### >+## Resource Files >+########################################################################### > ifdef RES_FILES #{ >-res-dep := .deps-copy-java-res >+res-dep := .deps/.last-update-res-deps >+GARBAGE += $(res-dep) > >-GENERATED_DIRS += res >-GARBAGE += $(res-dep) >+GENERATED_DIRS_export += res .deps > > export:: $(res-dep) > > res-dep-preqs := \ > $(addprefix $(srcdir)/,$(RES_FILES)) \ >- $(call mkdir_deps,res) \ >+ $(call mkdir_deps,res .deps) \ > $(if $(IS_LANGUAGE_REPACK),FORCE) \ > $(NULL) ... And this chunk part B. How come we call mkdir_deps for "res" and ".deps" explicitly here in part B? Doesn't 'GENERATED_DIRS_export += res .deps' mean that they will be called for mkdir_deps as part of the AUTO_DEPS line in part A? I think mkdir_deps should only be done once - while it doesn't break to call it multiple times, it's confusing what is actually needed here, and who (as in, which Makefile / dependency list) is responsible for creating them. > > # nop-build: only copy res/ files when needed > $(res-dep): $(res-dep-preqs) > $(call copy_dir,$(srcdir)/res,$(CURDIR)/res) > @$(TOUCH) $@ > endif #} > > >+########################################################################### >+## Java source/compiles >+## >+## JAVAFILES: A list of java source to compile. >+## >+## *.java.in => *.java >+## To pre-process *.java.in prior to compiling declare a PP_TARGET (~rules.mk) >+## and assign ${var}_TARGET := java-pre-process >+## >+## PP_TARGETS += java-stuff >+## java-stuff := $(wildcard $(srcdir)/*.java.in) >+## java-stuff_TARGET := java-pre-process >+########################################################################### > ifdef JAVAFILES #{ >-GENERATED_DIRS += classes > >-export:: classes >-classes: $(call mkdir_deps,classes) >-endif #} >+javac-dep := .deps/.last-update-javac-dep >+GARBAGE += $(javac-dep) >+ >+CLASSDIR ?= classes >+GENERATED_DIRS_export += classes .deps >+ >+# TODO: parameterize target but keep logic simple for now. >+libs:: java-pre-process $(javac-dep) >+ >+javac-dep-preqs := \ >+ $(JAVAFILES) \ >+ $(call mkdir_deps,$(CLASSDIR) .deps) \ >+ $(if $(IS_LANGUAGE_REPACK),FORCE) \ >+ $(NULL) Similar issue here with mkdir_deps in javac-dep-preqs and as a byproduct of GENERATED_DIRS_export. Also a nit - you seem to declare CLASSDIR ?= classes here (is this intended to be overridden by frontend Makefiles?), but then you use "CLASSDIR" in some places, and "classes" in others. I don't have a preference, but it should be consistent. (If you go with CLASSDIR, maybe "res" should be "RES ?= res" above as well?)

I don't believe this conflicts with the patches in bug 854530, but just in case...

(In reply to Gregory Szorc [:gps] from comment #38) > I don't believe this conflicts with the patches in bug 854530, but just in > case... It doesn't really conflict, but it's definitely in the same space. It's possible we want Bug 854530 to output a backend.mk that uses these rules.

Comment on attachment 728214 [details] [diff] [review] fix dependencies for java builds Review of attachment 728214 [details] [diff] [review]: ----------------------------------------------------------------- I don't have much to add over what mshal said. Cancelling review until next round. At that point, it should be r+. ::: build/mobile/robocop/Makefile.in @@ +106,1 @@ > classes.dex: $(java-tests-dep) While we're here, can we combine these lines? Alternatively, we could put the deps in a variable. ::: build/mobile/sutagent/android/fencp/Makefile.in @@ +8,5 @@ > VPATH = @srcdir@ > > include $(DEPTH)/config/autoconf.mk > > +# Should R.java be checked in ? nalexander says it should not. Let's kill it and set up appropriate deps. Wait for a follow-up bug? ::: config/makefiles/autotargets.mk @@ +47,5 @@ > # Extract directory path from a dependency file. > mkdir_stem =$(foreach val,$(getargv),$(subst /.mkdir.done,$(NULL),$(val))) > > +# := expansion for function use > +expand = $(eval func-expand-$(1) := $($(1)))$(func-expand-$(1)) Please document this in a sentence or two.

Have not looked at this for a long time and Nick was doing more recent dependency work in this area.