Refactor Android.mk generation

Now, instead of combining multiple binaries into a single BUILD_PREBUILT
definition, use separate instances for every module variant. This fixes
HOST vs HOST_CROSS prebuilts, and should be saner overall. From make,
these should look the same, we're only just using one instance of
prebuilt_internal per BUILD_PREBUILT call instead of multiple.

With that simplification, we don't have to store as much state, and can
directly write into the buffer.

Also switch from io.WriteString to fmt.Fprintln, which will require
fewer explicit string concatentations, and we don't need to worry about
newlines.

Allow the module-provided functions to return errors.

Change-Id: If30453b21fa21387f635626618d8fabfc78e6859

4 files changed, 107 insertions, 117 deletions

diff --git a/cc/androidmk.go b/cc/androidmk.go
index 11d7614b..08cdbdcf 100644
--- a/cc/androidmk.go
+++ b/cc/androidmk.go
@@ -15,67 +15,73 @@
 package cc
 
 import (
+	"fmt"
 	"io"
 	"strings"
 
 	"android/soong/common"
 )
 
-func (c *CCLibrary) AndroidMk() (ret common.AndroidMkData) {
+func (c *CCLibrary) AndroidMk() (ret common.AndroidMkData, err error) {
 	if c.static() {
 		ret.Class = "STATIC_LIBRARIES"
 	} else {
 		ret.Class = "SHARED_LIBRARIES"
 	}
 	ret.OutputFile = c.outputFile()
-	ret.Extra = func(name, prefix string, outputFile common.Path, arch common.Arch) (ret []string) {
-		exportedIncludes := c.exportedFlags()
-		for i := range exportedIncludes {
-			exportedIncludes[i] = strings.TrimPrefix(exportedIncludes[i], "-I")
+	ret.Extra = func(w io.Writer, outputFile common.Path) error {
+		exportedIncludes := []string{}
+		for _, flag := range c.exportedFlags() {
+			if flag != "" {
+				exportedIncludes = append(exportedIncludes, strings.TrimPrefix(flag, "-I"))
+			}
 		}
 		if len(exportedIncludes) > 0 {
-			ret = append(ret, "LOCAL_EXPORT_C_INCLUDE_DIRS := "+strings.Join(exportedIncludes, " "))
+			fmt.Fprintln(w, "LOCAL_EXPORT_C_INCLUDE_DIRS :=", strings.Join(exportedIncludes, " "))
 		}
 
-		ret = append(ret, "LOCAL_MODULE_SUFFIX := "+outputFile.Ext())
-		ret = append(ret, "LOCAL_SHARED_LIBRARIES_"+arch.ArchType.String()+" := "+strings.Join(c.savedDepNames.SharedLibs, " "))
+		fmt.Fprintln(w, "LOCAL_MODULE_SUFFIX :=", outputFile.Ext())
+		if len(c.savedDepNames.SharedLibs) > 0 {
+			fmt.Fprintln(w, "LOCAL_SHARED_LIBRARIES :=", strings.Join(c.savedDepNames.SharedLibs, " "))
+		}
 
 		if c.Properties.Relative_install_path != "" {
-			ret = append(ret, "LOCAL_MODULE_RELATIVE_PATH := "+c.Properties.Relative_install_path)
+			fmt.Fprintln(w, "LOCAL_MODULE_RELATIVE_PATH :=", c.Properties.Relative_install_path)
 		}
 
 		// These are already included in LOCAL_SHARED_LIBRARIES
-		ret = append(ret, "LOCAL_CXX_STL := none")
-		ret = append(ret, "LOCAL_SYSTEM_SHARED_LIBRARIES :=")
+		fmt.Fprintln(w, "LOCAL_CXX_STL := none")
+		fmt.Fprintln(w, "LOCAL_SYSTEM_SHARED_LIBRARIES :=")
 
-		return
+		return nil
 	}
 	return
 }
 
-func (c *ccObject) AndroidMk() (ret common.AndroidMkData) {
+func (c *ccObject) AndroidMk() (ret common.AndroidMkData, err error) {
 	ret.OutputFile = c.outputFile()
-	ret.Custom = func(w io.Writer, name, prefix string) {
+	ret.Custom = func(w io.Writer, name, prefix string) error {
 		out := c.outputFile().Path()
 
-		io.WriteString(w, "$("+prefix+"TARGET_OUT_INTERMEDIATE_LIBRARIES)/"+name+objectExtension+": "+out.String()+" | $(ACP)\n")
-		io.WriteString(w, "\t$(copy-file-to-target)\n")
+		fmt.Fprintln(w, "\n$("+prefix+"OUT_INTERMEDIATE_LIBRARIES)/"+name+objectExtension+":", out.String(), "| $(ACP)")
+		fmt.Fprintln(w, "\t$(copy-file-to-target)")
+
+		return nil
 	}
 	return
 }
 
-func (c *CCBinary) AndroidMk() (ret common.AndroidMkData) {
+func (c *CCBinary) AndroidMk() (ret common.AndroidMkData, err error) {
 	ret.Class = "EXECUTABLES"
-	ret.Extra = func(name, prefix string, outputFile common.Path, arch common.Arch) []string {
-		ret := []string{
-			"LOCAL_CXX_STL := none",
-			"LOCAL_SYSTEM_SHARED_LIBRARIES :=",
-			"LOCAL_SHARED_LIBRARIES_" + arch.ArchType.String() + " += " + strings.Join(c.savedDepNames.SharedLibs, " "),
-		}
+	ret.Extra = func(w io.Writer, outputFile common.Path) error {
+		fmt.Fprintln(w, "LOCAL_CXX_STL := none")
+		fmt.Fprintln(w, "LOCAL_SYSTEM_SHARED_LIBRARIES :=")
+		fmt.Fprintln(w, "LOCAL_SHARED_LIBRARIES :=", strings.Join(c.savedDepNames.SharedLibs, " "))
 		if c.Properties.Relative_install_path != "" {
-			ret = append(ret, "LOCAL_MODULE_RELATIVE_PATH_"+arch.ArchType.String()+" := "+c.Properties.Relative_install_path)
+			fmt.Fprintln(w, "LOCAL_MODULE_RELATIVE_PATH :=", c.Properties.Relative_install_path)
 		}
-		return ret
+
+		return nil
 	}
 	ret.OutputFile = c.outputFile()
 	return
diff --git a/common/androidmk.go b/common/androidmk.go
index 86efb042..f041b597 100644
--- a/common/androidmk.go
+++ b/common/androidmk.go
@@ -16,6 +16,7 @@ package common
 
 import (
 	"bytes"
+	"fmt"
 	"io"
 	"io/ioutil"
 	"os"
@@ -32,16 +33,16 @@ func init() {
 }
 
 type AndroidMkDataProvider interface {
-	AndroidMk() AndroidMkData
+	AndroidMk() (AndroidMkData, error)
 }
 
 type AndroidMkData struct {
 	Class      string
 	OutputFile OptionalPath
 
-	Custom func(w io.Writer, name, prefix string)
+	Custom func(w io.Writer, name, prefix string) error
 
-	Extra func(name, prefix string, outputFile Path, arch Arch) []string
+	Extra func(w io.Writer, outputFile Path) error
 }
 
 func AndroidMkSingleton() blueprint.Singleton {
@@ -87,8 +88,8 @@ func (c *androidMkSingleton) GenerateBuildActions(ctx blueprint.SingletonContext
 func translateAndroidMk(ctx blueprint.SingletonContext, mkFile string, mods []AndroidModule) error {
 	buf := &bytes.Buffer{}
 
-	io.WriteString(buf, "LOCAL_PATH := $(TOP)\n")
-	io.WriteString(buf, "LOCAL_MODULE_MAKEFILE := $(lastword $(MAKEFILE_LIST))\n")
+	fmt.Fprintln(buf, "LOCAL_PATH := $(TOP)")
+	fmt.Fprintln(buf, "LOCAL_MODULE_MAKEFILE := $(lastword $(MAKEFILE_LIST))")
 
 	for _, mod := range mods {
 		err := translateAndroidMkModule(ctx, buf, mod)
@@ -122,112 +123,83 @@ func translateAndroidMk(ctx blueprint.SingletonContext, mkFile string, mods []An
 }
 
 func translateAndroidMkModule(ctx blueprint.SingletonContext, w io.Writer, mod blueprint.Module) error {
-	if mod != ctx.PrimaryModule(mod) {
-		// These will be handled by the primary module
-		return nil
-	}
-
 	name := ctx.ModuleName(mod)
 
-	type hostClass struct {
-		host     bool
-		class    string
-		multilib string
+	provider, ok := mod.(AndroidMkDataProvider)
+	if !ok {
+		return nil
 	}
 
-	type archSrc struct {
-		arch  Arch
-		src   Path
-		extra []string
+	amod := mod.(AndroidModule).base()
+	data, err := provider.AndroidMk()
+	if err != nil {
+		return err
 	}
 
-	srcs := make(map[hostClass][]archSrc)
-	var modules []hostClass
-
-	ctx.VisitAllModuleVariants(mod, func(m blueprint.Module) {
-		provider, ok := m.(AndroidMkDataProvider)
-		if !ok {
-			return
-		}
-
-		amod := m.(AndroidModule).base()
-		data := provider.AndroidMk()
-
-		if !amod.Enabled() {
-			return
-		}
+	if !amod.Enabled() {
+		return err
+	}
 
-		arch := amod.commonProperties.CompileArch
+	hostCross := false
+	if amod.Host() && amod.HostType() != CurrentHostType() {
+		hostCross = true
+	}
 
+	if data.Custom != nil {
 		prefix := ""
-		if amod.HostOrDevice() == Host {
-			if arch.ArchType != ctx.Config().(Config).HostArches[amod.HostType()][0].ArchType {
-				prefix = "2ND_"
+		if amod.Host() {
+			if hostCross {
+				prefix = "HOST_CROSS_"
+			} else {
+				prefix = "HOST_"
+			}
+			if amod.Arch().ArchType != ctx.Config().(Config).HostArches[amod.HostType()][0].ArchType {
+				prefix = "2ND_" + prefix
 			}
 		} else {
-			if arch.ArchType != ctx.Config().(Config).DeviceArches[0].ArchType {
-				prefix = "2ND_"
+			prefix = "TARGET_"
+			if amod.Arch().ArchType != ctx.Config().(Config).DeviceArches[0].ArchType {
+				prefix = "2ND_" + prefix
 			}
 		}
 
-		if data.Custom != nil {
-			data.Custom(w, name, prefix)
-			return
-		}
-
-		if !data.OutputFile.Valid() {
-			return
-		}
-
-		hC := hostClass{
-			host:     amod.HostOrDevice() == Host,
-			class:    data.Class,
-			multilib: amod.commonProperties.Compile_multilib,
-		}
-
-		src := archSrc{
-			arch: arch,
-			src:  data.OutputFile.Path(),
-		}
-
-		if data.Extra != nil {
-			src.extra = data.Extra(name, prefix, src.src, arch)
-		}
-
-		if srcs[hC] == nil {
-			modules = append(modules, hC)
-		}
-		srcs[hC] = append(srcs[hC], src)
-	})
+		return data.Custom(w, name, prefix)
+	}
 
-	for _, hC := range modules {
-		archSrcs := srcs[hC]
+	if !data.OutputFile.Valid() {
+		return err
+	}
 
-		io.WriteString(w, "\ninclude $(CLEAR_VARS)\n")
-		io.WriteString(w, "LOCAL_MODULE := "+name+"\n")
-		io.WriteString(w, "LOCAL_MODULE_CLASS := "+hC.class+"\n")
-		io.WriteString(w, "LOCAL_MULTILIB := "+hC.multilib+"\n")
+	fmt.Fprintln(w, "\ninclude $(CLEAR_VARS)")
+	fmt.Fprintln(w, "LOCAL_MODULE :=", name)
+	fmt.Fprintln(w, "LOCAL_MODULE_CLASS :=", data.Class)
+	fmt.Fprintln(w, "LOCAL_MULTILIB :=", amod.commonProperties.Compile_multilib)
+	fmt.Fprintln(w, "LOCAL_SRC_FILES :=", data.OutputFile.String())
 
-		printed := make(map[string]bool)
-		for _, src := range archSrcs {
-			io.WriteString(w, "LOCAL_SRC_FILES_"+src.arch.ArchType.String()+" := "+src.src.String()+"\n")
+	archStr := amod.Arch().ArchType.String()
+	if amod.Host() {
+		if hostCross {
+			fmt.Fprintln(w, "LOCAL_MODULE_HOST_CROSS_ARCH :=", archStr)
+		} else {
+			fmt.Fprintln(w, "LOCAL_MODULE_HOST_ARCH :=", archStr)
 
-			for _, extra := range src.extra {
-				if !printed[extra] {
-					printed[extra] = true
-					io.WriteString(w, extra+"\n")
-				}
-			}
+			// TODO: this isn't true for every module, only dependencies of ACP
+			fmt.Fprintln(w, "LOCAL_ACP_UNAVAILABLE := true")
 		}
+		fmt.Fprintln(w, "LOCAL_MODULE_HOST_OS :=", amod.HostType().String())
+		fmt.Fprintln(w, "LOCAL_IS_HOST_MODULE := true")
+	} else {
+		fmt.Fprintln(w, "LOCAL_MODULE_TARGET_ARCH :=", archStr)
+	}
 
-		if hC.host {
-			// TODO: this isn't true for every module
-			io.WriteString(w, "LOCAL_ACP_UNAVAILABLE := true\n")
-
-			io.WriteString(w, "LOCAL_IS_HOST_MODULE := true\n")
+	if data.Extra != nil {
+		err = data.Extra(w, data.OutputFile.Path())
+		if err != nil {
+			return err
 		}
-		io.WriteString(w, "include $(BUILD_PREBUILT)\n")
 	}
 
-	return nil
+	fmt.Fprintln(w, "include $(BUILD_PREBUILT)")
+
+	return err
 }
diff --git a/common/module.go b/common/module.go
index ad2ba4e7..844db7f6 100644
--- a/common/module.go
+++ b/common/module.go
@@ -255,6 +255,14 @@ func (a *AndroidModuleBase) HostType() HostType {
 	return a.commonProperties.CompileHostType
 }
 
+func (a *AndroidModuleBase) Host() bool {
+	return a.HostOrDevice().Host()
+}
+
+func (a *AndroidModuleBase) Arch() Arch {
+	return a.commonProperties.CompileArch
+}
+
 func (a *AndroidModuleBase) HostSupported() bool {
 	return a.commonProperties.HostOrDeviceSupported == HostSupported ||
 		a.commonProperties.HostOrDeviceSupported == HostAndDeviceSupported &&
diff --git a/java/androidmk.go b/java/androidmk.go
index bc81054f..6d4dddf9 100644
--- a/java/androidmk.go
+++ b/java/androidmk.go
@@ -15,17 +15,21 @@
 package java
 
 import (
+	"fmt"
+
 	"android/soong/common"
 )
 
-func (*JavaLibrary) AndroidMk() (ret common.AndroidMkData) {
+func (*JavaLibrary) AndroidMk() (ret common.AndroidMkData, err error) {
 	ret.Class = "JAVA_LIBRARIES"
 	// TODO
+	err = fmt.Errorf("Not yet implemented")
 	return
 }
 
-func (*JavaPrebuilt) AndroidMk() (ret common.AndroidMkData) {
+func (*JavaPrebuilt) AndroidMk() (ret common.AndroidMkData, err error) {
 	ret.Class = "JAVA_LIBRARIES"
 	// TODO
+	err = fmt.Errorf("Not yet implemented")
 	return
 }