Merge "Move apex module installation into Soong"
diff --git a/android/neverallow.go b/android/neverallow.go
index 04366d3..b36bf04 100644
--- a/android/neverallow.go
+++ b/android/neverallow.go
@@ -214,8 +214,11 @@
 	return []Rule{
 		NeverAllow().
 			ModuleType("makefile_goal").
+			// TODO(b/33691272): remove this after migrating seapp to Soong
+			Without("product_out_path", "obj/ETC/plat_seapp_contexts_intermediates/plat_seapp_contexts").
+			Without("product_out_path", "obj/ETC/plat_seapp_neverallows_intermediates/plat_seapp_neverallows").
 			WithoutMatcher("product_out_path", Regexp("^boot[0-9a-zA-Z.-]*[.]img$")).
-			Because("Only boot images may be imported as a makefile goal."),
+			Because("Only boot images and seapp contexts may be imported as a makefile goal."),
 	}
 }
 
diff --git a/android/neverallow_test.go b/android/neverallow_test.go
index 35aadd8..edda244 100644
--- a/android/neverallow_test.go
+++ b/android/neverallow_test.go
@@ -293,7 +293,7 @@
 			`),
 		},
 		expectedErrors: []string{
-			"Only boot images may be imported as a makefile goal.",
+			"Only boot images and seapp contexts may be imported as a makefile goal.",
 		},
 	},
 }
diff --git a/cmd/sbox/sbox.go b/cmd/sbox/sbox.go
index c7f3f6a..4fa7486 100644
--- a/cmd/sbox/sbox.go
+++ b/cmd/sbox/sbox.go
@@ -164,7 +164,7 @@
 		if useSubDir {
 			localTempDir = filepath.Join(localTempDir, strconv.Itoa(i))
 		}
-		depFile, err := runCommand(command, localTempDir)
+		depFile, err := runCommand(command, localTempDir, i)
 		if err != nil {
 			// Running the command failed, keep the temporary output directory around in
 			// case a user wants to inspect it for debugging purposes.  Soong will delete
@@ -194,6 +194,28 @@
 	return nil
 }
 
+// createCommandScript will create and return an exec.Cmd that runs rawCommand.
+//
+// rawCommand is executed via a script in the sandbox.
+// tempDir is the temporary where the script is created.
+// toDirInSandBox is the path containing the script in the sbox environment.
+// toDirInSandBox is the path containing the script in the sbox environment.
+// seed is a unique integer used to distinguish different scripts that might be at location.
+//
+// returns an exec.Cmd that can be ran from within sbox context if no error, or nil if error.
+// caller must ensure script is cleaned up if function succeeds.
+//
+func createCommandScript(rawCommand string, tempDir, toDirInSandbox string, seed int) (*exec.Cmd, error) {
+	scriptName := fmt.Sprintf("sbox_command.%d.bash", seed)
+	scriptPathAndName := joinPath(tempDir, scriptName)
+	err := os.WriteFile(scriptPathAndName, []byte(rawCommand), 0644)
+	if err != nil {
+		return nil, fmt.Errorf("failed to write command %s... to %s",
+			rawCommand[0:40], scriptPathAndName)
+	}
+	return exec.Command("bash", joinPath(toDirInSandbox, filepath.Base(scriptName))), nil
+}
+
 // readManifest reads an sbox manifest from a textproto file.
 func readManifest(file string) (*sbox_proto.Manifest, error) {
 	manifestData, err := ioutil.ReadFile(file)
@@ -213,7 +235,7 @@
 
 // runCommand runs a single command from a manifest.  If the command references the
 // __SBOX_DEPFILE__ placeholder it returns the name of the depfile that was used.
-func runCommand(command *sbox_proto.Command, tempDir string) (depFile string, err error) {
+func runCommand(command *sbox_proto.Command, tempDir string, commandIndex int) (depFile string, err error) {
 	rawCommand := command.GetCommand()
 	if rawCommand == "" {
 		return "", fmt.Errorf("command is required")
@@ -255,7 +277,11 @@
 		return "", err
 	}
 
-	cmd := exec.Command("bash", "-c", rawCommand)
+	cmd, err := createCommandScript(rawCommand, tempDir, pathToTempDirInSbox, commandIndex)
+	if err != nil {
+		return "", err
+	}
+
 	buf := &bytes.Buffer{}
 	cmd.Stdin = os.Stdin
 	cmd.Stdout = buf
diff --git a/java/base.go b/java/base.go
index 71b5ac8..d9b1260 100644
--- a/java/base.go
+++ b/java/base.go
@@ -1066,7 +1066,7 @@
 	j.compiledSrcJars = srcJars
 
 	enableSharding := false
-	var headerJarFileWithoutJarjar android.Path
+	var headerJarFileWithoutDepsOrJarjar android.Path
 	if ctx.Device() && !ctx.Config().IsEnvFalse("TURBINE_ENABLED") && !deps.disableTurbine {
 		if j.properties.Javac_shard_size != nil && *(j.properties.Javac_shard_size) > 0 {
 			enableSharding = true
@@ -1076,7 +1076,7 @@
 			// allow for the use of annotation processors that do function correctly
 			// with sharding enabled. See: b/77284273.
 		}
-		headerJarFileWithoutJarjar, j.headerJarFile =
+		headerJarFileWithoutDepsOrJarjar, j.headerJarFile =
 			j.compileJavaHeader(ctx, uniqueSrcFiles, srcJars, deps, flags, jarName, kotlinJars)
 		if ctx.Failed() {
 			return
@@ -1105,7 +1105,9 @@
 		}
 
 		if enableSharding {
-			flags.classpath = append(flags.classpath, headerJarFileWithoutJarjar)
+			if headerJarFileWithoutDepsOrJarjar != nil {
+				flags.classpath = append(classpath{headerJarFileWithoutDepsOrJarjar}, flags.classpath...)
+			}
 			shardSize := int(*(j.properties.Javac_shard_size))
 			var shardSrcs []android.Paths
 			if len(uniqueSrcFiles) > 0 {
@@ -1508,7 +1510,7 @@
 
 func (j *Module) compileJavaHeader(ctx android.ModuleContext, srcFiles, srcJars android.Paths,
 	deps deps, flags javaBuilderFlags, jarName string,
-	extraJars android.Paths) (headerJar, jarjarHeaderJar android.Path) {
+	extraJars android.Paths) (headerJar, jarjarAndDepsHeaderJar android.Path) {
 
 	var jars android.Paths
 	if len(srcFiles) > 0 || len(srcJars) > 0 {
@@ -1519,6 +1521,7 @@
 			return nil, nil
 		}
 		jars = append(jars, turbineJar)
+		headerJar = turbineJar
 	}
 
 	jars = append(jars, extraJars...)
@@ -1532,20 +1535,19 @@
 	combinedJar := android.PathForModuleOut(ctx, "turbine-combined", jarName)
 	TransformJarsToJar(ctx, combinedJar, "for turbine", jars, android.OptionalPath{},
 		false, nil, []string{"META-INF/TRANSITIVE"})
-	headerJar = combinedJar
-	jarjarHeaderJar = combinedJar
+	jarjarAndDepsHeaderJar = combinedJar
 
 	if j.expandJarjarRules != nil {
 		// Transform classes.jar into classes-jarjar.jar
 		jarjarFile := android.PathForModuleOut(ctx, "turbine-jarjar", jarName)
-		TransformJarJar(ctx, jarjarFile, headerJar, j.expandJarjarRules)
-		jarjarHeaderJar = jarjarFile
+		TransformJarJar(ctx, jarjarFile, jarjarAndDepsHeaderJar, j.expandJarjarRules)
+		jarjarAndDepsHeaderJar = jarjarFile
 		if ctx.Failed() {
 			return nil, nil
 		}
 	}
 
-	return headerJar, jarjarHeaderJar
+	return headerJar, jarjarAndDepsHeaderJar
 }
 
 func (j *Module) instrument(ctx android.ModuleContext, flags javaBuilderFlags,
diff --git a/java/java_test.go b/java/java_test.go
index c039f72..6e4e673 100644
--- a/java/java_test.go
+++ b/java/java_test.go
@@ -988,11 +988,11 @@
 		}
 		`)
 
-	barHeaderJar := filepath.Join("out", "soong", ".intermediates", "bar", "android_common", "turbine-combined", "bar.jar")
+	barHeaderJar := filepath.Join("out", "soong", ".intermediates", "bar", "android_common", "turbine", "bar.jar")
 	for i := 0; i < 3; i++ {
 		barJavac := ctx.ModuleForTests("bar", "android_common").Description("javac" + strconv.Itoa(i))
-		if !strings.Contains(barJavac.Args["classpath"], barHeaderJar) {
-			t.Errorf("bar javac classpath %v does not contain %q", barJavac.Args["classpath"], barHeaderJar)
+		if !strings.HasPrefix(barJavac.Args["classpath"], "-classpath "+barHeaderJar+":") {
+			t.Errorf("bar javac classpath %v does start with %q", barJavac.Args["classpath"], barHeaderJar)
 		}
 	}
 }
diff --git a/java/systemserver_classpath_fragment.go b/java/systemserver_classpath_fragment.go
index d75635c..e263cc4 100644
--- a/java/systemserver_classpath_fragment.go
+++ b/java/systemserver_classpath_fragment.go
@@ -125,6 +125,12 @@
 	// TODO(b/203233647): Add better mechanism to make it optional.
 	_, unknown = android.RemoveFromList("car-frameworks-service-module", unknown)
 
+	// This module is optional, so it is not present in all products.
+	// (See PRODUCT_ISOLATED_COMPILATION_ENABLED.)
+	// So ignore it even if it is not in PRODUCT_APEX_SYSTEM_SERVER_JARS.
+	// TODO(b/203233647): Add better mechanism to make it optional.
+	_, unknown = android.RemoveFromList("service-compos", unknown)
+
 	// TODO(satayev): for apex_test we want to include all contents unconditionally to classpaths
 	// config. However, any test specific jars would not be present in ApexSystemServerJars. Instead,
 	// we should check if we are creating a config for apex_test via ApexInfo and amend the values.
diff --git a/mk2rbc/cmd/mk2rbc.go b/mk2rbc/cmd/mk2rbc.go
index 7bb0246..bb5a680 100644
--- a/mk2rbc/cmd/mk2rbc.go
+++ b/mk2rbc/cmd/mk2rbc.go
@@ -46,8 +46,6 @@
 	dryRun   = flag.Bool("dry_run", false, "dry run")
 	recurse  = flag.Bool("convert_dependents", false, "convert all dependent files")
 	mode     = flag.String("mode", "", `"backup" to back up existing files, "write" to overwrite them`)
-	noWarn   = flag.Bool("no_warnings", false, "don't warn about partially failed conversions")
-	verbose  = flag.Bool("v", false, "print summary")
 	errstat  = flag.Bool("error_stat", false, "print error statistics")
 	traceVar = flag.String("trace", "", "comma-separated list of variables to trace")
 	// TODO(asmundak): this option is for debugging
@@ -75,7 +73,6 @@
 	flagAlias("root", "d")
 	flagAlias("dry_run", "n")
 	flagAlias("convert_dependents", "r")
-	flagAlias("no_warnings", "w")
 	flagAlias("error_stat", "e")
 }
 
@@ -207,9 +204,9 @@
 		}
 	}
 
-	printStats()
 	if *errstat {
 		errorLogger.printStatistics()
+		printStats()
 	}
 	if !ok {
 		os.Exit(1)
@@ -329,17 +326,16 @@
 	}()
 
 	mk2starRequest := mk2rbc.Request{
-		MkFile:             mkFile,
-		Reader:             nil,
-		RootDir:            *rootDir,
-		OutputDir:          *outputTop,
-		OutputSuffix:       *suffix,
-		TracedVariables:    tracedVariables,
-		TraceCalls:         *traceCalls,
-		WarnPartialSuccess: !*noWarn,
-		SourceFS:           os.DirFS(*rootDir),
-		MakefileFinder:     makefileFinder,
-		ErrorLogger:        errorLogger,
+		MkFile:          mkFile,
+		Reader:          nil,
+		RootDir:         *rootDir,
+		OutputDir:       *outputTop,
+		OutputSuffix:    *suffix,
+		TracedVariables: tracedVariables,
+		TraceCalls:      *traceCalls,
+		SourceFS:        os.DirFS(*rootDir),
+		MakefileFinder:  makefileFinder,
+		ErrorLogger:     errorLogger,
 	}
 	ss, err := mk2rbc.Convert(mk2starRequest)
 	if err != nil {
@@ -417,9 +413,6 @@
 
 func printStats() {
 	var sortedFiles []string
-	if *noWarn && !*verbose {
-		return
-	}
 	for p := range converted {
 		sortedFiles = append(sortedFiles, p)
 	}
@@ -435,27 +428,22 @@
 			nOk++
 		}
 	}
-	if !*noWarn {
-		if nPartial > 0 {
-			fmt.Fprintf(os.Stderr, "Conversion was partially successful for:\n")
-			for _, f := range sortedFiles {
-				if ss := converted[f]; ss != nil && ss.HasErrors() {
-					fmt.Fprintln(os.Stderr, "  ", f)
-				}
-			}
-		}
-
-		if nFailed > 0 {
-			fmt.Fprintf(os.Stderr, "Conversion failed for files:\n")
-			for _, f := range sortedFiles {
-				if converted[f] == nil {
-					fmt.Fprintln(os.Stderr, "  ", f)
-				}
+	if nPartial > 0 {
+		fmt.Fprintf(os.Stderr, "Conversion was partially successful for:\n")
+		for _, f := range sortedFiles {
+			if ss := converted[f]; ss != nil && ss.HasErrors() {
+				fmt.Fprintln(os.Stderr, "  ", f)
 			}
 		}
 	}
-	if *verbose && (nPartial > 0 || nFailed > 0) {
-		fmt.Fprintln(os.Stderr, "Succeeded: ", nOk, " Partial: ", nPartial, " Failed: ", nFailed)
+
+	if nFailed > 0 {
+		fmt.Fprintf(os.Stderr, "Conversion failed for files:\n")
+		for _, f := range sortedFiles {
+			if converted[f] == nil {
+				fmt.Fprintln(os.Stderr, "  ", f)
+			}
+		}
 	}
 }
 
@@ -468,8 +456,8 @@
 	data map[string]datum
 }
 
-func (ebt errorSink) NewError(sourceFile string, sourceLine int, node parser.Node, message string, args ...interface{}) {
-	fmt.Fprintf(os.Stderr, "%s:%d ", sourceFile, sourceLine)
+func (ebt errorSink) NewError(el mk2rbc.ErrorLocation, node parser.Node, message string, args ...interface{}) {
+	fmt.Fprint(os.Stderr, el, ": ")
 	fmt.Fprintf(os.Stderr, message, args...)
 	fmt.Fprintln(os.Stderr)
 	if !*errstat {
diff --git a/mk2rbc/expr.go b/mk2rbc/expr.go
index 8279d2e..ec0b279 100644
--- a/mk2rbc/expr.go
+++ b/mk2rbc/expr.go
@@ -18,8 +18,6 @@
 	"fmt"
 	"strconv"
 	"strings"
-
-	mkparser "android/soong/androidmk/parser"
 )
 
 // Represents an expression in the Starlark code. An expression has
@@ -106,7 +104,7 @@
 		format += "%s" + strings.ReplaceAll(chunk, "%", "%%")
 	}
 	gctx.writef("%q %% ", format)
-	emitarg := func(arg starlarkExpr) {
+	emitArg := func(arg starlarkExpr) {
 		if arg.typ() == starlarkTypeList {
 			gctx.write(`" ".join(`)
 			arg.emit(gctx)
@@ -116,12 +114,12 @@
 		}
 	}
 	if len(xi.args) == 1 {
-		emitarg(xi.args[0])
+		emitArg(xi.args[0])
 	} else {
 		sep := "("
 		for _, arg := range xi.args {
 			gctx.write(sep)
-			emitarg(arg)
+			emitArg(arg)
 			sep = ", "
 		}
 		gctx.write(")")
@@ -224,9 +222,9 @@
 		s.expr.emit(ctx)
 		ctx.write("))")
 	case starlarkTypeBool:
-		ctx.write("((")
+		ctx.write(`("true" if (`)
 		s.expr.emit(ctx)
-		ctx.write(`) ? "true" : "")`)
+		ctx.write(`) else "")`)
 	case starlarkTypeVoid:
 		ctx.write(`""`)
 	default:
@@ -285,20 +283,33 @@
 }
 
 func (eq *eqExpr) emit(gctx *generationContext) {
-	emitSimple := func(expr starlarkExpr) {
-		if eq.isEq {
-			gctx.write("not ")
-		}
-		expr.emit(gctx)
+	var stringOperand string
+	var otherOperand starlarkExpr
+	if s, ok := maybeString(eq.left); ok {
+		stringOperand = s
+		otherOperand = eq.right
+	} else if s, ok := maybeString(eq.right); ok {
+		stringOperand = s
+		otherOperand = eq.left
 	}
-	// Are we checking that a variable is empty?
-	if isEmptyString(eq.left) {
-		emitSimple(eq.right)
-		return
-	} else if isEmptyString(eq.right) {
-		emitSimple(eq.left)
-		return
 
+	// If we've identified one of the operands as being a string literal, check
+	// for some special cases we can do to simplify the resulting expression.
+	if otherOperand != nil {
+		if stringOperand == "" {
+			if eq.isEq {
+				gctx.write("not ")
+			}
+			otherOperand.emit(gctx)
+			return
+		}
+		if stringOperand == "true" && otherOperand.typ() == starlarkTypeBool {
+			if !eq.isEq {
+				gctx.write("not ")
+			}
+			otherOperand.emit(gctx)
+			return
+		}
 	}
 
 	if eq.left.typ() != eq.right.typ() {
@@ -414,7 +425,7 @@
 	return &v
 }
 
-// concatExpr generates epxr1 + expr2 + ... + exprN in Starlark.
+// concatExpr generates expr1 + expr2 + ... + exprN in Starlark.
 type concatExpr struct {
 	items []starlarkExpr
 }
@@ -607,8 +618,8 @@
 }
 
 type badExpr struct {
-	node    mkparser.Node
-	message string
+	errorLocation ErrorLocation
+	message       string
 }
 
 func (b *badExpr) eval(_ map[string]starlarkExpr) (res starlarkExpr, same bool) {
@@ -617,15 +628,15 @@
 	return
 }
 
-func (b *badExpr) emit(_ *generationContext) {
-	panic("implement me")
+func (b *badExpr) emit(gctx *generationContext) {
+	gctx.emitConversionError(b.errorLocation, b.message)
 }
 
 func (_ *badExpr) typ() starlarkType {
 	return starlarkTypeUnknown
 }
 
-func (b *badExpr) emitListVarCopy(gctx *generationContext) {
+func (_ *badExpr) emitListVarCopy(_ *generationContext) {
 	panic("implement me")
 }
 
diff --git a/mk2rbc/mk2rbc.go b/mk2rbc/mk2rbc.go
index b87ab41..c55300e 100644
--- a/mk2rbc/mk2rbc.go
+++ b/mk2rbc/mk2rbc.go
@@ -102,6 +102,7 @@
 	"addsuffix":                           {baseName + ".addsuffix", starlarkTypeList, hiddenArgNone},
 	"copy-files":                          {baseName + ".copy_files", starlarkTypeList, hiddenArgNone},
 	"dir":                                 {baseName + ".dir", starlarkTypeList, hiddenArgNone},
+	"dist-for-goals":                      {baseName + ".mkdist_for_goals", starlarkTypeVoid, hiddenArgGlobal},
 	"enforce-product-packages-exist":      {baseName + ".enforce_product_packages_exist", starlarkTypeVoid, hiddenArgNone},
 	"error":                               {baseName + ".mkerror", starlarkTypeVoid, hiddenArgNone},
 	"findstring":                          {"!findstring", starlarkTypeInt, hiddenArgNone},
@@ -156,23 +157,31 @@
 
 // Conversion request parameters
 type Request struct {
-	MkFile             string    // file to convert
-	Reader             io.Reader // if set, read input from this stream instead
-	RootDir            string    // root directory path used to resolve included files
-	OutputSuffix       string    // generated Starlark files suffix
-	OutputDir          string    // if set, root of the output hierarchy
-	ErrorLogger        ErrorLogger
-	TracedVariables    []string // trace assignment to these variables
-	TraceCalls         bool
-	WarnPartialSuccess bool
-	SourceFS           fs.FS
-	MakefileFinder     MakefileFinder
+	MkFile          string    // file to convert
+	Reader          io.Reader // if set, read input from this stream instead
+	RootDir         string    // root directory path used to resolve included files
+	OutputSuffix    string    // generated Starlark files suffix
+	OutputDir       string    // if set, root of the output hierarchy
+	ErrorLogger     ErrorLogger
+	TracedVariables []string // trace assignment to these variables
+	TraceCalls      bool
+	SourceFS        fs.FS
+	MakefileFinder  MakefileFinder
 }
 
 // ErrorLogger prints errors and gathers error statistics.
 // Its NewError function is called on every error encountered during the conversion.
 type ErrorLogger interface {
-	NewError(sourceFile string, sourceLine int, node mkparser.Node, text string, args ...interface{})
+	NewError(el ErrorLocation, node mkparser.Node, text string, args ...interface{})
+}
+
+type ErrorLocation struct {
+	MkFile string
+	MkLine int
+}
+
+func (el ErrorLocation) String() string {
+	return fmt.Sprintf("%s:%d", el.MkFile, el.MkLine)
 }
 
 // Derives module name for a given file. It is base name
@@ -248,10 +257,6 @@
 		node.emit(gctx)
 	}
 
-	if ss.hasErrors && ss.warnPartialSuccess {
-		gctx.newLine()
-		gctx.writef("%s(%q, %q)", cfnWarning, filepath.Base(ss.mkFile), "partially successful conversion")
-	}
 	if gctx.starScript.traceCalls {
 		gctx.newLine()
 		gctx.writef(`print("<%s")`, gctx.starScript.mkFile)
@@ -306,6 +311,10 @@
 	gctx.writef("%*s", 2*gctx.indentLevel, "")
 }
 
+func (gctx *generationContext) emitConversionError(el ErrorLocation, message string) {
+	gctx.writef(`rblf.mk2rbc_error("%s", %q)`, el, message)
+}
+
 type knownVariable struct {
 	name      string
 	class     varClass
@@ -370,18 +379,17 @@
 
 // Information about the generated Starlark script.
 type StarlarkScript struct {
-	mkFile             string
-	moduleName         string
-	mkPos              scanner.Position
-	nodes              []starlarkNode
-	inherited          []*moduleInfo
-	hasErrors          bool
-	topDir             string
-	traceCalls         bool // print enter/exit each init function
-	warnPartialSuccess bool
-	sourceFS           fs.FS
-	makefileFinder     MakefileFinder
-	nodeLocator        func(pos mkparser.Pos) int
+	mkFile         string
+	moduleName     string
+	mkPos          scanner.Position
+	nodes          []starlarkNode
+	inherited      []*moduleInfo
+	hasErrors      bool
+	topDir         string
+	traceCalls     bool // print enter/exit each init function
+	sourceFS       fs.FS
+	makefileFinder MakefileFinder
+	nodeLocator    func(pos mkparser.Pos) int
 }
 
 func (ss *StarlarkScript) newNode(node starlarkNode) {
@@ -560,7 +568,7 @@
 		return
 	}
 	_, isTraced := ctx.tracedVariables[name]
-	asgn := &assignmentNode{lhs: lhs, mkValue: a.Value, isTraced: isTraced}
+	asgn := &assignmentNode{lhs: lhs, mkValue: a.Value, isTraced: isTraced, location: ctx.errorLocation(a)}
 	if lhs.valueType() == starlarkTypeUnknown {
 		// Try to divine variable type from the RHS
 		asgn.value = ctx.parseMakeString(a, a.Value)
@@ -1023,10 +1031,10 @@
 func (ctx *parseContext) newBadExpr(node mkparser.Node, text string, args ...interface{}) starlarkExpr {
 	message := fmt.Sprintf(text, args...)
 	if ctx.errorLogger != nil {
-		ctx.errorLogger.NewError(ctx.script.mkFile, ctx.script.nodeLocator(node.Pos()), node, text, args...)
+		ctx.errorLogger.NewError(ctx.errorLocation(node), node, text, args...)
 	}
 	ctx.script.hasErrors = true
-	return &badExpr{node, message}
+	return &badExpr{errorLocation: ctx.errorLocation(node), message: message}
 }
 
 func (ctx *parseContext) parseCompare(cond *mkparser.Directive) starlarkExpr {
@@ -1544,18 +1552,14 @@
 // records that the given node failed to be converted and includes an explanatory message
 func (ctx *parseContext) errorf(failedNode mkparser.Node, message string, args ...interface{}) {
 	if ctx.errorLogger != nil {
-		ctx.errorLogger.NewError(ctx.script.mkFile, ctx.script.nodeLocator(failedNode.Pos()), failedNode, message, args...)
+		ctx.errorLogger.NewError(ctx.errorLocation(failedNode), failedNode, message, args...)
 	}
-	message = fmt.Sprintf(message, args...)
-	ctx.insertComment(fmt.Sprintf("# MK2RBC TRANSLATION ERROR: %s", message))
-	ctx.carryAsComment(failedNode)
-
+	ctx.receiver.newNode(&exprNode{ctx.newBadExpr(failedNode, message, args...)})
 	ctx.script.hasErrors = true
 }
 
 func (ctx *parseContext) wrapBadExpr(xBad *badExpr) {
-	ctx.insertComment(fmt.Sprintf("# MK2RBC TRANSLATION ERROR: %s", xBad.message))
-	ctx.carryAsComment(xBad.node)
+	ctx.receiver.newNode(&exprNode{xBad})
 }
 
 func (ctx *parseContext) loadedModulePath(path string) string {
@@ -1621,6 +1625,10 @@
 	return ok
 }
 
+func (ctx *parseContext) errorLocation(node mkparser.Node) ErrorLocation {
+	return ErrorLocation{ctx.script.mkFile, ctx.script.nodeLocator(node.Pos())}
+}
+
 func (ss *StarlarkScript) String() string {
 	return NewGenerateContext(ss).emit()
 }
@@ -1659,14 +1667,13 @@
 		return nil, fmt.Errorf("bad makefile %s", req.MkFile)
 	}
 	starScript := &StarlarkScript{
-		moduleName:         moduleNameForFile(req.MkFile),
-		mkFile:             req.MkFile,
-		topDir:             req.RootDir,
-		traceCalls:         req.TraceCalls,
-		warnPartialSuccess: req.WarnPartialSuccess,
-		sourceFS:           req.SourceFS,
-		makefileFinder:     req.MakefileFinder,
-		nodeLocator:        func(pos mkparser.Pos) int { return parser.Unpack(pos).Line },
+		moduleName:     moduleNameForFile(req.MkFile),
+		mkFile:         req.MkFile,
+		topDir:         req.RootDir,
+		traceCalls:     req.TraceCalls,
+		sourceFS:       req.SourceFS,
+		makefileFinder: req.MakefileFinder,
+		nodeLocator:    func(pos mkparser.Pos) int { return parser.Unpack(pos).Line },
 	}
 	ctx := newParseContext(starScript, nodes)
 	ctx.outputSuffix = req.OutputSuffix
diff --git a/mk2rbc/mk2rbc_test.go b/mk2rbc/mk2rbc_test.go
index c2c4654..dfdf274 100644
--- a/mk2rbc/mk2rbc_test.go
+++ b/mk2rbc/mk2rbc_test.go
@@ -105,15 +105,12 @@
 PRODUCT_NAME := $(call foo1, bar)
 PRODUCT_NAME := $(call foo0)
 `,
-		expected: `# MK2RBC TRANSLATION ERROR: cannot handle invoking foo1
-# PRODUCT_NAME := $(call foo1, bar)
-# MK2RBC TRANSLATION ERROR: cannot handle invoking foo0
-# PRODUCT_NAME := $(call foo0)
-load("//build/make/core:product_config.rbc", "rblf")
+		expected: `load("//build/make/core:product_config.rbc", "rblf")
 
 def init(g, handle):
   cfg = rblf.cfg(handle)
-  rblf.warning("product.mk", "partially successful conversion")
+  rblf.mk2rbc_error("product.mk:2", "cannot handle invoking foo1")
+  rblf.mk2rbc_error("product.mk:3", "cannot handle invoking foo0")
 `,
 	},
 	{
@@ -207,15 +204,11 @@
     $(info foo)
 endef
 `,
-		expected: `# MK2RBC TRANSLATION ERROR: define is not supported: some-macro
-# define  some-macro
-#     $(info foo)
-# endef
-load("//build/make/core:product_config.rbc", "rblf")
+		expected: `load("//build/make/core:product_config.rbc", "rblf")
 
 def init(g, handle):
   cfg = rblf.cfg(handle)
-  rblf.warning("product.mk", "partially successful conversion")
+  rblf.mk2rbc_error("product.mk:2", "define is not supported: some-macro")
 `,
 	},
 	{
@@ -282,9 +275,7 @@
     # Comment
     pass
   else:
-    # MK2RBC TRANSLATION ERROR: cannot set predefined variable TARGET_COPY_OUT_RECOVERY to "foo", its value should be "recovery"
-    pass
-  rblf.warning("product.mk", "partially successful conversion")
+    rblf.mk2rbc_error("product.mk:5", "cannot set predefined variable TARGET_COPY_OUT_RECOVERY to \"foo\", its value should be \"recovery\"")
 `,
 	},
 	{
@@ -368,6 +359,21 @@
 `,
 	},
 	{
+		desc:   "ifeq with $(NATIVE_COVERAGE)",
+		mkname: "product.mk",
+		in: `
+ifeq ($(NATIVE_COVERAGE),true)
+endif
+`,
+		expected: `load("//build/make/core:product_config.rbc", "rblf")
+
+def init(g, handle):
+  cfg = rblf.cfg(handle)
+  if g.get("NATIVE_COVERAGE", False):
+    pass
+`,
+	},
+	{
 		desc:   "Check filter result",
 		mkname: "product.mk",
 		in: `
@@ -689,6 +695,7 @@
 $(call enforce-product-packages-exist, foo)
 $(call require-artifacts-in-path, foo, bar)
 $(call require-artifacts-in-path-relaxed, foo, bar)
+$(call dist-for-goals, goal, from:to)
 `,
 		expected: `load("//build/make/core:product_config.rbc", "rblf")
 
@@ -698,6 +705,7 @@
   rblf.enforce_product_packages_exist("foo")
   rblf.require_artifacts_in_path("foo", "bar")
   rblf.require_artifacts_in_path_relaxed("foo", "bar")
+  rblf.mkdist_for_goals(g, "goal", "from:to")
 `,
 	},
 	{
@@ -857,9 +865,7 @@
   rblf.soong_config_namespace(g, "cvd")
   rblf.soong_config_set(g, "cvd", "launch_configs", "cvd_config_auto.json")
   rblf.soong_config_append(g, "cvd", "grub_config", "grub.cfg")
-  # MK2RBC TRANSLATION ERROR: SOONG_CONFIG_ variables cannot be referenced, use soong_config_get instead: SOONG_CONFIG_cvd_grub_config
-  # x := $(SOONG_CONFIG_cvd_grub_config)
-  rblf.warning("product.mk", "partially successful conversion")
+  rblf.mk2rbc_error("product.mk:7", "SOONG_CONFIG_ variables cannot be referenced, use soong_config_get instead: SOONG_CONFIG_cvd_grub_config")
 `,
 	}, {
 		desc:   "soong namespace accesses",
@@ -1046,15 +1052,11 @@
 		in: `
 foo: foo.c
 	gcc -o $@ $*`,
-		expected: `# MK2RBC TRANSLATION ERROR: unsupported line rule:       foo: foo.c
-#gcc -o $@ $*
-# rule:       foo: foo.c
-# gcc -o $@ $*
-load("//build/make/core:product_config.rbc", "rblf")
+		expected: `load("//build/make/core:product_config.rbc", "rblf")
 
 def init(g, handle):
   cfg = rblf.cfg(handle)
-  rblf.warning("product.mk", "partially successful conversion")
+  rblf.mk2rbc_error("product.mk:2", "unsupported line rule:       foo: foo.c\n#gcc -o $@ $*")
 `,
 	},
 	{
@@ -1062,14 +1064,27 @@
 		mkname: "product.mk",
 		in: `
 override FOO:=`,
-		expected: `# MK2RBC TRANSLATION ERROR: cannot handle override directive
-# override FOO :=
-load("//build/make/core:product_config.rbc", "rblf")
+		expected: `load("//build/make/core:product_config.rbc", "rblf")
 
 def init(g, handle):
   cfg = rblf.cfg(handle)
+  rblf.mk2rbc_error("product.mk:2", "cannot handle override directive")
   g["override FOO"] = ""
-  rblf.warning("product.mk", "partially successful conversion")
+`,
+	},
+	{
+		desc:   "Bad expression",
+		mkname: "build/product.mk",
+		in: `
+ifeq (,$(call foobar))
+endif
+`,
+		expected: `load("//build/make/core:product_config.rbc", "rblf")
+
+def init(g, handle):
+  cfg = rblf.cfg(handle)
+  if rblf.mk2rbc_error("build/product.mk:2", "cannot handle invoking foobar"):
+    pass
 `,
 	},
 }
@@ -1079,6 +1094,7 @@
 	class varClass
 	starlarkType
 }{
+	{"NATIVE_COVERAGE", VarClassSoong, starlarkTypeBool},
 	{"PRODUCT_NAME", VarClassConfig, starlarkTypeString},
 	{"PRODUCT_MODEL", VarClassConfig, starlarkTypeString},
 	{"PRODUCT_PACKAGES", VarClassConfig, starlarkTypeList},
@@ -1140,13 +1156,12 @@
 		t.Run(test.desc,
 			func(t *testing.T) {
 				ss, err := Convert(Request{
-					MkFile:             test.mkname,
-					Reader:             bytes.NewBufferString(test.in),
-					RootDir:            ".",
-					OutputSuffix:       ".star",
-					WarnPartialSuccess: true,
-					SourceFS:           fs,
-					MakefileFinder:     &testMakefileFinder{fs: fs},
+					MkFile:         test.mkname,
+					Reader:         bytes.NewBufferString(test.in),
+					RootDir:        ".",
+					OutputSuffix:   ".star",
+					SourceFS:       fs,
+					MakefileFinder: &testMakefileFinder{fs: fs},
 				})
 				if err != nil {
 					t.Error(err)
diff --git a/mk2rbc/node.go b/mk2rbc/node.go
index 3fe1a17..d38299d 100644
--- a/mk2rbc/node.go
+++ b/mk2rbc/node.go
@@ -183,6 +183,7 @@
 	value    starlarkExpr
 	mkValue  *mkparser.MakeString
 	flavor   assignmentFlavor
+	location ErrorLocation
 	isTraced bool
 	previous *assignmentNode
 }
@@ -223,18 +224,6 @@
 	}
 
 	gctx.newLine()
-	if bad, ok := in.expr.(*badExpr); ok {
-		gctx.write("# MK2STAR ERROR converting:")
-		gctx.newLine()
-		gctx.writef("#   %s", bad.node.Dump())
-		gctx.newLine()
-		gctx.writef("# %s", bad.message)
-		gctx.newLine()
-		// The init function emits a warning if the conversion was not
-		// fullly successful, so here we (arbitrarily) take the false path.
-		gctx.writef("%sFalse:", ifElif)
-		return
-	}
 	gctx.write(ifElif)
 	in.expr.emit(gctx)
 	gctx.write(":")
diff --git a/mk2rbc/variable.go b/mk2rbc/variable.go
index ded07fe..6b67a7c 100644
--- a/mk2rbc/variable.go
+++ b/mk2rbc/variable.go
@@ -228,10 +228,9 @@
 			if actualValue == expectedValue {
 				return
 			}
-			gctx.writef("# MK2RBC TRANSLATION ERROR: cannot set predefined variable %s to %q, its value should be %q",
-				pv.name(), actualValue, expectedValue)
-			gctx.newLine()
-			gctx.write("pass")
+			gctx.emitConversionError(asgn.location,
+				fmt.Sprintf("cannot set predefined variable %s to %q, its value should be %q",
+					pv.name(), actualValue, expectedValue))
 			gctx.starScript.hasErrors = true
 			return
 		}
diff --git a/scripts/build-mainline-modules.sh b/scripts/build-mainline-modules.sh
index f183c05..1e3f6ce 100755
--- a/scripts/build-mainline-modules.sh
+++ b/scripts/build-mainline-modules.sh
@@ -17,7 +17,6 @@
 MODULES_SDK_AND_EXPORTS=(
   art-module-sdk
   art-module-test-exports
-  compos-module-sdk
   conscrypt-module-host-exports
   conscrypt-module-sdk
   conscrypt-module-test-exports
diff --git a/scripts/unpack-prebuilt-apex.sh b/scripts/unpack-prebuilt-apex.sh
index 1acdeb5..f34a480 100755
--- a/scripts/unpack-prebuilt-apex.sh
+++ b/scripts/unpack-prebuilt-apex.sh
@@ -29,8 +29,6 @@
 shift 4
 REQUIRED_PATHS=$@
 
-set -x 1
-
 rm -fr $OUTPUT_DIR
 mkdir -p $OUTPUT_DIR
 
diff --git a/scripts/update_out b/scripts/update_out
new file mode 100755
index 0000000..d3950cf
--- /dev/null
+++ b/scripts/update_out
@@ -0,0 +1,21 @@
+#! /bin/bash
+# Run given command application and update the contents of a given file.
+# Will not change the file if its contents has not changed.
+[[ $# -gt 1 ]] || { echo "Usage: ${0##*/} FILE COMMAND" >&2; exit 1; }
+set -u
+declare -r outfile="$1"
+shift
+if [[ ! -f $outfile ]]; then
+	$@ >$outfile
+	exit
+fi
+
+declare -r newout=${outfile}.new
+$@ >$newout
+rc=$?
+if cmp -s $newout $outfile; then
+	rm $newout
+else
+	mv -f $newout $outfile
+fi
+exit $rc