소스 검색

Allow dynamically calculated inherit-product path

Bug: 193566316
Test: internal
Change-Id: Iaa7b68cf459f9a694ae9d37a32c9372cf8a8335a
Sasha Smundak 2 년 전
부모
커밋
6609ba7664
7개의 변경된 파일474개의 추가작업 그리고 54개의 파일을 삭제
  1. 41 0
      mk2rbc/cmd/mk2rbc.go
  2. 121 0
      mk2rbc/find_mockfs.go
  3. 135 26
      mk2rbc/mk2rbc.go
  4. 81 8
      mk2rbc/mk2rbc_test.go
  5. 86 17
      mk2rbc/node.go
  6. 5 0
      mk2rbc/types.go
  7. 5 3
      mk2rbc/variable.go

+ 41 - 0
mk2rbc/cmd/mk2rbc.go

@@ -21,10 +21,12 @@
 package main
 
 import (
+	"bufio"
 	"flag"
 	"fmt"
 	"io/ioutil"
 	"os"
+	"os/exec"
 	"path/filepath"
 	"regexp"
 	"runtime/debug"
@@ -78,6 +80,7 @@ func init() {
 var backupSuffix string
 var tracedVariables []string
 var errorLogger = errorsByType{data: make(map[string]datum)}
+var makefileFinder = &LinuxMakefileFinder{}
 
 func main() {
 	flag.Usage = func() {
@@ -302,6 +305,8 @@ func convertOne(mkFile string) (ok bool) {
 		TracedVariables:    tracedVariables,
 		TraceCalls:         *traceCalls,
 		WarnPartialSuccess: *warn,
+		SourceFS:           os.DirFS(*rootDir),
+		MakefileFinder:     makefileFinder,
 	}
 	if *errstat {
 		mk2starRequest.ErrorLogger = errorLogger
@@ -504,3 +509,39 @@ func stringsWithFreq(items []string, topN int) (string, int) {
 	}
 	return res, len(sorted)
 }
+
+type LinuxMakefileFinder struct {
+	cachedRoot      string
+	cachedMakefiles []string
+}
+
+func (l *LinuxMakefileFinder) Find(root string) []string {
+	if l.cachedMakefiles != nil && l.cachedRoot == root {
+		return l.cachedMakefiles
+	}
+	l.cachedRoot = root
+	l.cachedMakefiles = make([]string, 0)
+
+	// Return all *.mk files but not in hidden directories.
+
+	// NOTE(asmundak): as it turns out, even the WalkDir (which is an _optimized_ directory tree walker)
+	// is about twice slower than running `find` command (14s vs 6s on the internal Android source tree).
+	common_args := []string{"!", "-type", "d", "-name", "*.mk", "!", "-path", "*/.*/*"}
+	if root != "" {
+		common_args = append([]string{root}, common_args...)
+	}
+	cmd := exec.Command("/usr/bin/find", common_args...)
+	stdout, err := cmd.StdoutPipe()
+	if err == nil {
+		err = cmd.Start()
+	}
+	if err != nil {
+		panic(fmt.Errorf("cannot get the output from %s: %s", cmd, err))
+	}
+	scanner := bufio.NewScanner(stdout)
+	for scanner.Scan() {
+		l.cachedMakefiles = append(l.cachedMakefiles, strings.TrimPrefix(scanner.Text(), "./"))
+	}
+	stdout.Close()
+	return l.cachedMakefiles
+}

+ 121 - 0
mk2rbc/find_mockfs.go

@@ -0,0 +1,121 @@
+package mk2rbc
+
+import (
+	"io/fs"
+	"os"
+	"path/filepath"
+	"time"
+)
+
+// Mock FS. Maps a directory name to an array of entries.
+// An entry implements fs.DirEntry, fs.FIleInfo and fs.File interface
+type FindMockFS struct {
+	dirs map[string][]myFileInfo
+}
+
+func (m FindMockFS) locate(name string) (myFileInfo, bool) {
+	if name == "." {
+		return myFileInfo{".", true}, true
+	}
+	dir := filepath.Dir(name)
+	base := filepath.Base(name)
+	if entries, ok := m.dirs[dir]; ok {
+		for _, e := range entries {
+			if e.name == base {
+				return e, true
+			}
+		}
+	}
+	return myFileInfo{}, false
+}
+
+func (m FindMockFS) create(name string, isDir bool) {
+	dir := filepath.Dir(name)
+	m.dirs[dir] = append(m.dirs[dir], myFileInfo{filepath.Base(name), isDir})
+}
+
+func (m FindMockFS) Stat(name string) (fs.FileInfo, error) {
+	if fi, ok := m.locate(name); ok {
+		return fi, nil
+	}
+	return nil, os.ErrNotExist
+}
+
+type myFileInfo struct {
+	name  string
+	isDir bool
+}
+
+func (m myFileInfo) Info() (fs.FileInfo, error) {
+	panic("implement me")
+}
+
+func (m myFileInfo) Size() int64 {
+	panic("implement me")
+}
+
+func (m myFileInfo) Mode() fs.FileMode {
+	panic("implement me")
+}
+
+func (m myFileInfo) ModTime() time.Time {
+	panic("implement me")
+}
+
+func (m myFileInfo) Sys() interface{} {
+	return nil
+}
+
+func (m myFileInfo) Stat() (fs.FileInfo, error) {
+	return m, nil
+}
+
+func (m myFileInfo) Read(bytes []byte) (int, error) {
+	panic("implement me")
+}
+
+func (m myFileInfo) Close() error {
+	panic("implement me")
+}
+
+func (m myFileInfo) Name() string {
+	return m.name
+}
+
+func (m myFileInfo) IsDir() bool {
+	return m.isDir
+}
+
+func (m myFileInfo) Type() fs.FileMode {
+	return m.Mode()
+}
+
+func (m FindMockFS) Open(name string) (fs.File, error) {
+	panic("implement me")
+}
+
+func (m FindMockFS) ReadDir(name string) ([]fs.DirEntry, error) {
+	if d, ok := m.dirs[name]; ok {
+		var res []fs.DirEntry
+		for _, e := range d {
+			res = append(res, e)
+		}
+		return res, nil
+	}
+	return nil, os.ErrNotExist
+}
+
+func NewFindMockFS(files []string) FindMockFS {
+	myfs := FindMockFS{make(map[string][]myFileInfo)}
+	for _, f := range files {
+		isDir := false
+		for f != "." {
+			if _, ok := myfs.locate(f); !ok {
+				myfs.create(f, isDir)
+			}
+			isDir = true
+			f = filepath.Dir(f)
+		}
+	}
+	return myfs
+}

+ 135 - 26
mk2rbc/mk2rbc.go

@@ -27,6 +27,7 @@ import (
 	"bytes"
 	"fmt"
 	"io"
+	"io/fs"
 	"io/ioutil"
 	"os"
 	"path/filepath"
@@ -104,6 +105,7 @@ var knownFunctions = map[string]struct {
 	"match-prefix":                        {"!match-prefix", starlarkTypeUnknown},       // internal macro
 	"match-word":                          {"!match-word", starlarkTypeUnknown},         // internal macro
 	"match-word-in-list":                  {"!match-word-in-list", starlarkTypeUnknown}, // internal macro
+	"my-dir":                              {"!my-dir", starlarkTypeString},
 	"patsubst":                            {baseName + ".mkpatsubst", starlarkTypeString},
 	"produce_copy_files":                  {baseName + ".produce_copy_files", starlarkTypeList},
 	"require-artifacts-in-path":           {baseName + ".require_artifacts_in_path", starlarkTypeVoid},
@@ -136,6 +138,8 @@ type Request struct {
 	TracedVariables    []string // trace assignment to these variables
 	TraceCalls         bool
 	WarnPartialSuccess bool
+	SourceFS           fs.FS
+	MakefileFinder     MakefileFinder
 }
 
 // An error sink allowing to gather error statistics.
@@ -149,7 +153,8 @@ type ErrorMonitorCB interface {
 func moduleNameForFile(mkFile string) string {
 	base := strings.TrimSuffix(filepath.Base(mkFile), filepath.Ext(mkFile))
 	// TODO(asmundak): what else can be in the product file names?
-	return strings.ReplaceAll(base, "-", "_")
+	return strings.NewReplacer("-", "_", ".", "_").Replace(base)
+
 }
 
 func cloneMakeString(mkString *mkparser.MakeString) *mkparser.MakeString {
@@ -241,7 +246,7 @@ func (gctx *generationContext) emitPreamble() {
 			sc.moduleLocalName = m
 			continue
 		}
-		if !sc.loadAlways {
+		if sc.optional {
 			uri += "|init"
 		}
 		gctx.newLine()
@@ -342,11 +347,13 @@ type StarlarkScript struct {
 	moduleName         string
 	mkPos              scanner.Position
 	nodes              []starlarkNode
-	inherited          []*inheritedModule
+	inherited          []*moduleInfo
 	hasErrors          bool
 	topDir             string
 	traceCalls         bool // print enter/exit each init function
 	warnPartialSuccess bool
+	sourceFS           fs.FS
+	makefileFinder     MakefileFinder
 }
 
 func (ss *StarlarkScript) newNode(node starlarkNode) {
@@ -379,13 +386,15 @@ type parseContext struct {
 	receiver         nodeReceiver // receptacle for the generated starlarkNode's
 	receiverStack    []nodeReceiver
 	outputDir        string
+	dependentModules map[string]*moduleInfo
 }
 
 func newParseContext(ss *StarlarkScript, nodes []mkparser.Node) *parseContext {
+	topdir, _ := filepath.Split(filepath.Join(ss.topDir, "foo"))
 	predefined := []struct{ name, value string }{
 		{"SRC_TARGET_DIR", filepath.Join("build", "make", "target")},
 		{"LOCAL_PATH", filepath.Dir(ss.mkFile)},
-		{"TOPDIR", ss.topDir},
+		{"TOPDIR", topdir},
 		// TODO(asmundak): maybe read it from build/make/core/envsetup.mk?
 		{"TARGET_COPY_OUT_SYSTEM", "system"},
 		{"TARGET_COPY_OUT_SYSTEM_OTHER", "system_other"},
@@ -428,6 +437,7 @@ func newParseContext(ss *StarlarkScript, nodes []mkparser.Node) *parseContext {
 		moduleNameCount:  make(map[string]int),
 		builtinMakeVars:  map[string]starlarkExpr{},
 		variables:        make(map[string]variable),
+		dependentModules: make(map[string]*moduleInfo),
 	}
 	ctx.pushVarAssignments()
 	for _, item := range predefined {
@@ -619,16 +629,12 @@ func (ctx *parseContext) buildConcatExpr(a *mkparser.Assignment) *concatExpr {
 	return xConcat
 }
 
-func (ctx *parseContext) newInheritedModule(v mkparser.Node, pathExpr starlarkExpr, loadAlways bool) *inheritedModule {
-	var path string
-	x, _ := pathExpr.eval(ctx.builtinMakeVars)
-	s, ok := x.(*stringLiteralExpr)
-	if !ok {
-		ctx.errorf(v, "inherit-product/include argument is too complex")
-		return nil
+func (ctx *parseContext) newDependentModule(path string, optional bool) *moduleInfo {
+	modulePath := ctx.loadedModulePath(path)
+	if mi, ok := ctx.dependentModules[modulePath]; ok {
+		mi.optional = mi.optional || optional
+		return mi
 	}
-
-	path = s.literal
 	moduleName := moduleNameForFile(path)
 	moduleLocalName := "_" + moduleName
 	n, found := ctx.moduleNameCount[moduleName]
@@ -636,27 +642,124 @@ func (ctx *parseContext) newInheritedModule(v mkparser.Node, pathExpr starlarkEx
 		moduleLocalName += fmt.Sprintf("%d", n)
 	}
 	ctx.moduleNameCount[moduleName] = n + 1
-	ln := &inheritedModule{
-		path:            ctx.loadedModulePath(path),
+	mi := &moduleInfo{
+		path:            modulePath,
 		originalPath:    path,
-		moduleName:      moduleName,
 		moduleLocalName: moduleLocalName,
-		loadAlways:      loadAlways,
+		optional:        optional,
+	}
+	ctx.dependentModules[modulePath] = mi
+	ctx.script.inherited = append(ctx.script.inherited, mi)
+	return mi
+}
+
+func (ctx *parseContext) handleSubConfig(
+	v mkparser.Node, pathExpr starlarkExpr, loadAlways bool, processModule func(inheritedModule)) {
+	pathExpr, _ = pathExpr.eval(ctx.builtinMakeVars)
+
+	// In a simple case, the name of a module to inherit/include is known statically.
+	if path, ok := maybeString(pathExpr); ok {
+		if strings.Contains(path, "*") {
+			if paths, err := fs.Glob(ctx.script.sourceFS, path); err == nil {
+				for _, p := range paths {
+					processModule(inheritedStaticModule{ctx.newDependentModule(p, !loadAlways), loadAlways})
+				}
+			} else {
+				ctx.errorf(v, "cannot glob wildcard argument")
+			}
+		} else {
+			processModule(inheritedStaticModule{ctx.newDependentModule(path, !loadAlways), loadAlways})
+		}
+		return
+	}
+
+	// If module path references variables (e.g., $(v1)/foo/$(v2)/device-config.mk), find all the paths in the
+	// source tree that may be a match and the corresponding variable values. For instance, if the source tree
+	// contains vendor1/foo/abc/dev.mk and vendor2/foo/def/dev.mk, the first one will be inherited when
+	// (v1, v2) == ('vendor1', 'abc'), and the second one when (v1, v2) == ('vendor2', 'def').
+	// We then emit the code that loads all of them, e.g.:
+	//    load("//vendor1/foo/abc:dev.rbc", _dev1_init="init")
+	//    load("//vendor2/foo/def/dev.rbc", _dev2_init="init")
+	// And then inherit it as follows:
+	//    _e = {
+	//       "vendor1/foo/abc/dev.mk": ("vendor1/foo/abc/dev", _dev1_init),
+	//       "vendor2/foo/def/dev.mk": ("vendor2/foo/def/dev", _dev_init2) }.get("%s/foo/%s/dev.mk" % (v1, v2))
+	//    if _e:
+	//       rblf.inherit(handle, _e[0], _e[1])
+	//
+	var matchingPaths []string
+	varPath, ok := pathExpr.(*interpolateExpr)
+	if !ok {
+		ctx.errorf(v, "inherit-product/include argument is too complex")
+		return
+	}
+
+	pathPattern := []string{varPath.chunks[0]}
+	for _, chunk := range varPath.chunks[1:] {
+		if chunk != "" {
+			pathPattern = append(pathPattern, chunk)
+		}
+	}
+	if pathPattern[0] != "" {
+		matchingPaths = ctx.findMatchingPaths(pathPattern)
+	} else {
+		// Heuristics -- if pattern starts from top, restrict it to the directories where
+		// we know inherit-product uses dynamically calculated path.
+		for _, t := range []string{"vendor/qcom", "vendor/google_devices"} {
+			pathPattern[0] = t
+			matchingPaths = append(matchingPaths, ctx.findMatchingPaths(pathPattern)...)
+		}
+	}
+	// Safeguard against $(call inherit-product,$(PRODUCT_PATH))
+	const maxMatchingFiles = 100
+	if len(matchingPaths) > maxMatchingFiles {
+		ctx.errorf(v, "there are >%d files matching the pattern, please rewrite it", maxMatchingFiles)
+		return
+	}
+	res := inheritedDynamicModule{*varPath, []*moduleInfo{}, loadAlways}
+	for _, p := range matchingPaths {
+		// A product configuration files discovered dynamically may attempt to inherit
+		// from another one which does not exist in this source tree. Prevent load errors
+		// by always loading the dynamic files as optional.
+		res.candidateModules = append(res.candidateModules, ctx.newDependentModule(p, true))
+	}
+	processModule(res)
+}
+
+func (ctx *parseContext) findMatchingPaths(pattern []string) []string {
+	files := ctx.script.makefileFinder.Find(ctx.script.topDir)
+	if len(pattern) == 0 {
+		return files
+	}
+
+	// Create regular expression from the pattern
+	s_regexp := "^" + regexp.QuoteMeta(pattern[0])
+	for _, s := range pattern[1:] {
+		s_regexp += ".*" + regexp.QuoteMeta(s)
+	}
+	s_regexp += "$"
+	rex := regexp.MustCompile(s_regexp)
+
+	// Now match
+	var res []string
+	for _, p := range files {
+		if rex.MatchString(p) {
+			res = append(res, p)
+		}
 	}
-	ctx.script.inherited = append(ctx.script.inherited, ln)
-	return ln
+	return res
 }
 
 func (ctx *parseContext) handleInheritModule(v mkparser.Node, pathExpr starlarkExpr, loadAlways bool) {
-	if im := ctx.newInheritedModule(v, pathExpr, loadAlways); im != nil {
+	ctx.handleSubConfig(v, pathExpr, loadAlways, func(im inheritedModule) {
 		ctx.receiver.newNode(&inheritNode{im})
-	}
+	})
 }
 
 func (ctx *parseContext) handleInclude(v mkparser.Node, pathExpr starlarkExpr, loadAlways bool) {
-	if ln := ctx.newInheritedModule(v, pathExpr, loadAlways); ln != nil {
-		ctx.receiver.newNode(&includeNode{ln})
-	}
+	ctx.handleSubConfig(v, pathExpr, loadAlways, func(im inheritedModule) {
+		ctx.receiver.newNode(&includeNode{im})
+	})
 }
 
 func (ctx *parseContext) handleVariable(v *mkparser.Variable) {
@@ -1091,9 +1194,10 @@ func (ctx *parseContext) parseReference(node mkparser.Node, ref *mkparser.MakeSt
 		}
 		expr.name = words[0].Dump()
 		if len(words) < 2 {
-			return expr
+			args = &mkparser.MakeString{}
+		} else {
+			args = words[1]
 		}
-		args = words[1]
 	}
 	if kf, found := knownFunctions[expr.name]; found {
 		expr.returnType = kf.returnType
@@ -1103,6 +1207,8 @@ func (ctx *parseContext) parseReference(node mkparser.Node, ref *mkparser.MakeSt
 	switch expr.name {
 	case "word":
 		return ctx.parseWordFunc(node, args)
+	case "my-dir":
+		return &variableRefExpr{ctx.addVariable("LOCAL_PATH"), true}
 	case "subst", "patsubst":
 		return ctx.parseSubstFunc(node, expr.name, args)
 	default:
@@ -1285,6 +1391,7 @@ func (ss *StarlarkScript) String() string {
 }
 
 func (ss *StarlarkScript) SubConfigFiles() []string {
+
 	var subs []string
 	for _, src := range ss.inherited {
 		subs = append(subs, src.originalPath)
@@ -1322,6 +1429,8 @@ func Convert(req Request) (*StarlarkScript, error) {
 		topDir:             req.RootDir,
 		traceCalls:         req.TraceCalls,
 		warnPartialSuccess: req.WarnPartialSuccess,
+		sourceFS:           req.SourceFS,
+		makefileFinder:     req.MakefileFinder,
 	}
 	ctx := newParseContext(starScript, nodes)
 	ctx.outputSuffix = req.OutputSuffix

+ 81 - 8
mk2rbc/mk2rbc_test.go

@@ -16,6 +16,8 @@ package mk2rbc
 
 import (
 	"bytes"
+	"io/fs"
+	"path/filepath"
 	"strings"
 	"testing"
 )
@@ -100,10 +102,13 @@ def init(g, handle):
 		desc:   "Unknown function",
 		mkname: "product.mk",
 		in: `
-PRODUCT_NAME := $(call foo, bar)
+PRODUCT_NAME := $(call foo1, bar)
+PRODUCT_NAME := $(call foo0)
 `,
-		expected: `# MK2RBC TRANSLATION ERROR: cannot handle invoking foo
-# PRODUCT_NAME := $(call foo, bar)
+		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")
 
 def init(g, handle):
@@ -130,7 +135,7 @@ def init(g, handle):
     rblf.inherit(handle, "part", _part_init)
   else:
     # Comment
-    rblf.inherit(handle, "./part", _part_init)
+    rblf.inherit(handle, "part", _part_init)
 `,
 	},
 	{
@@ -144,7 +149,7 @@ load(":part.star|init", _part_init = "init")
 
 def init(g, handle):
   cfg = rblf.cfg(handle)
-  if _part_init != None:
+  if _part_init:
     rblf.inherit(handle, "part", _part_init)
 `,
 	},
@@ -160,7 +165,7 @@ else
 endif
 `,
 		expected: `load("//build/make/core:product_config.rbc", "rblf")
-load(":part.star", _part_init = "init")
+load(":part.star|init", _part_init = "init")
 
 def init(g, handle):
   cfg = rblf.cfg(handle)
@@ -176,8 +181,7 @@ def init(g, handle):
 		desc:   "Synonymous inherited configurations",
 		mkname: "path/product.mk",
 		in: `
-$(call inherit-product, foo/font.mk)
-$(call inherit-product, bar/font.mk)
+$(call inherit-product, */font.mk)
 `,
 		expected: `load("//build/make/core:product_config.rbc", "rblf")
 load("//foo:font.star", _font_init = "init")
@@ -254,6 +258,8 @@ def init(g, handle):
 		in: `
 ifdef PRODUCT_NAME
 # Comment
+else
+  TARGET_COPY_OUT_VENDOR := foo
 endif
 `,
 		expected: `load("//build/make/core:product_config.rbc", "rblf")
@@ -263,6 +269,10 @@ def init(g, handle):
   if g.get("PRODUCT_NAME") != None:
     # Comment
     pass
+  else:
+    # MK2RBC TRANSLATION ERROR: cannot set predefined variable TARGET_COPY_OUT_VENDOR to "foo", its value should be "||VENDOR-PATH-PH||"
+    pass
+  rblf.warning("product.mk", "partially successful conversion")
 `,
 	},
 	{
@@ -840,6 +850,30 @@ def init(g, handle):
     g["V2"] = g.get("PRODUCT_ADB_KEYS", "")
     g["PRODUCT_ADB_KEYS"] = "foo"
     g["V3"] = g["PRODUCT_ADB_KEYS"]
+`,
+	},
+	{
+		desc:   "Dynamic inherit path",
+		mkname: "product.mk",
+		in: `
+MY_PATH=foo
+$(call inherit-product,vendor/$(MY_PATH)/cfg.mk)
+`,
+		expected: `load("//build/make/core:product_config.rbc", "rblf")
+load("//vendor/foo1:cfg.star|init", _cfg_init = "init")
+load("//vendor/bar/baz:cfg.star|init", _cfg1_init = "init")
+
+def init(g, handle):
+  cfg = rblf.cfg(handle)
+  g["MY_PATH"] = "foo"
+  _entry = {
+    "vendor/foo1/cfg.mk": ("_cfg", _cfg_init),
+    "vendor/bar/baz/cfg.mk": ("_cfg1", _cfg1_init),
+  }.get("vendor/%s/cfg.mk" % g["MY_PATH"])
+  (_varmod, _varmod_init) = _entry if _entry else (None, None)
+  if not _varmod_init:
+    rblf.mkerror("cannot")
+  rblf.inherit(handle, _varmod, _varmod_init)
 `,
 	},
 }
@@ -865,10 +899,47 @@ var known_variables = []struct {
 	{"PLATFORM_LIST", VarClassSoong, starlarkTypeList}, // TODO(asmundak): make it local instead of soong
 }
 
+type testMakefileFinder struct {
+	fs    fs.FS
+	root  string
+	files []string
+}
+
+func (t *testMakefileFinder) Find(root string) []string {
+	if t.files != nil || root == t.root {
+		return t.files
+	}
+	t.files = make([]string, 0)
+	fs.WalkDir(t.fs, root, func(path string, d fs.DirEntry, err error) error {
+		if err != nil {
+			return err
+		}
+		if d.IsDir() {
+			base := filepath.Base(path)
+			if base[0] == '.' && len(base) > 1 {
+				return fs.SkipDir
+			}
+			return nil
+		}
+		if strings.HasSuffix(path, ".mk") {
+			t.files = append(t.files, path)
+		}
+		return nil
+	})
+	return t.files
+}
+
 func TestGood(t *testing.T) {
 	for _, v := range known_variables {
 		KnownVariables.NewVariable(v.name, v.class, v.starlarkType)
 	}
+	fs := NewFindMockFS([]string{
+		"vendor/foo1/cfg.mk",
+		"vendor/bar/baz/cfg.mk",
+		"part.mk",
+		"foo/font.mk",
+		"bar/font.mk",
+	})
 	for _, test := range testCases {
 		t.Run(test.desc,
 			func(t *testing.T) {
@@ -878,6 +949,8 @@ func TestGood(t *testing.T) {
 					RootDir:            ".",
 					OutputSuffix:       ".star",
 					WarnPartialSuccess: true,
+					SourceFS:           fs,
+					MakefileFinder:     &testMakefileFinder{fs: fs},
 				})
 				if err != nil {
 					t.Error(err)

+ 86 - 17
mk2rbc/node.go

@@ -42,24 +42,85 @@ func (c *commentNode) emit(gctx *generationContext) {
 	}
 }
 
-type inheritedModule struct {
+type moduleInfo struct {
 	path            string // Converted Starlark file path
 	originalPath    string // Makefile file path
-	moduleName      string
 	moduleLocalName string
-	loadAlways      bool
+	optional        bool
 }
 
-func (im inheritedModule) name() string {
-	return MakePath2ModuleName(im.originalPath)
+func (im moduleInfo) entryName() string {
+	return im.moduleLocalName + "_init"
 }
 
-func (im inheritedModule) entryName() string {
-	return im.moduleLocalName + "_init"
+type inheritedModule interface {
+	name() string
+	entryName() string
+	emitSelect(gctx *generationContext)
+	isLoadAlways() bool
+}
+
+type inheritedStaticModule struct {
+	*moduleInfo
+	loadAlways bool
+}
+
+func (im inheritedStaticModule) name() string {
+	return fmt.Sprintf("%q", MakePath2ModuleName(im.originalPath))
+}
+
+func (im inheritedStaticModule) emitSelect(_ *generationContext) {
+}
+
+func (im inheritedStaticModule) isLoadAlways() bool {
+	return im.loadAlways
+}
+
+type inheritedDynamicModule struct {
+	path             interpolateExpr
+	candidateModules []*moduleInfo
+	loadAlways       bool
+}
+
+func (i inheritedDynamicModule) name() string {
+	return "_varmod"
+}
+
+func (i inheritedDynamicModule) entryName() string {
+	return i.name() + "_init"
+}
+
+func (i inheritedDynamicModule) emitSelect(gctx *generationContext) {
+	gctx.newLine()
+	gctx.writef("_entry = {")
+	gctx.indentLevel++
+	for _, mi := range i.candidateModules {
+		gctx.newLine()
+		gctx.writef(`"%s": (%q, %s),`, mi.originalPath, mi.moduleLocalName, mi.entryName())
+	}
+	gctx.indentLevel--
+	gctx.newLine()
+	gctx.write("}.get(")
+	i.path.emit(gctx)
+	gctx.write(")")
+	gctx.newLine()
+	gctx.writef("(%s, %s) = _entry if _entry else (None, None)", i.name(), i.entryName())
+	if i.loadAlways {
+		gctx.newLine()
+		gctx.writef("if not %s:", i.entryName())
+		gctx.indentLevel++
+		gctx.newLine()
+		gctx.write(`rblf.mkerror("cannot")`)
+		gctx.indentLevel--
+	}
+}
+
+func (i inheritedDynamicModule) isLoadAlways() bool {
+	return i.loadAlways
 }
 
 type inheritNode struct {
-	*inheritedModule
+	module inheritedModule
 }
 
 func (inn *inheritNode) emit(gctx *generationContext) {
@@ -68,32 +129,40 @@ func (inn *inheritNode) emit(gctx *generationContext) {
 	// Conditional case:
 	//    if <module>_init != None:
 	//      same as above
+	inn.module.emitSelect(gctx)
+
+	name := inn.module.name()
+	entry := inn.module.entryName()
 	gctx.newLine()
-	if inn.loadAlways {
-		gctx.writef("%s(handle, %q, %s)", cfnInherit, inn.name(), inn.entryName())
+	if inn.module.isLoadAlways() {
+		gctx.writef("%s(handle, %s, %s)", cfnInherit, name, entry)
 		return
 	}
-	gctx.writef("if %s != None:", inn.entryName())
+
+	gctx.writef("if %s:", entry)
 	gctx.indentLevel++
 	gctx.newLine()
-	gctx.writef("%s(handle, %q, %s)", cfnInherit, inn.name(), inn.entryName())
+	gctx.writef("%s(handle, %s, %s)", cfnInherit, name, entry)
 	gctx.indentLevel--
 }
 
 type includeNode struct {
-	*inheritedModule
+	module inheritedModule
 }
 
 func (inn *includeNode) emit(gctx *generationContext) {
+	inn.module.emitSelect(gctx)
+	entry := inn.module.entryName()
 	gctx.newLine()
-	if inn.loadAlways {
-		gctx.writef("%s(g, handle)", inn.entryName())
+	if inn.module.isLoadAlways() {
+		gctx.writef("%s(g, handle)", entry)
 		return
 	}
-	gctx.writef("if %s != None:", inn.entryName())
+
+	gctx.writef("if %s != None:", entry)
 	gctx.indentLevel++
 	gctx.newLine()
-	gctx.writef("%s(g, handle)", inn.entryName())
+	gctx.writef("%s(g, handle)", entry)
 	gctx.indentLevel--
 }
 

+ 5 - 0
mk2rbc/types.go

@@ -58,3 +58,8 @@ func (s ScopeBase) Call(_ string, _ []string) []string {
 func (s ScopeBase) SetFunc(_ string, _ func([]string) []string) {
 	panic("implement me")
 }
+
+// Used to find all makefiles in the source tree
+type MakefileFinder interface {
+	Find(root string) []string
+}

+ 5 - 3
mk2rbc/variable.go

@@ -16,7 +16,6 @@ package mk2rbc
 
 import (
 	"fmt"
-	"os"
 	"strings"
 )
 
@@ -222,15 +221,18 @@ func (pv predefinedVariable) emitGet(gctx *generationContext, _ bool) {
 	pv.value.emit(gctx)
 }
 
-func (pv predefinedVariable) emitSet(_ *generationContext, asgn *assignmentNode) {
+func (pv predefinedVariable) emitSet(gctx *generationContext, asgn *assignmentNode) {
 	if expectedValue, ok1 := maybeString(pv.value); ok1 {
 		actualValue, ok2 := maybeString(asgn.value)
 		if ok2 {
 			if actualValue == expectedValue {
 				return
 			}
-			fmt.Fprintf(os.Stderr, "cannot set predefined variable %s to %q, its value should be %q",
+			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.starScript.hasErrors = true
 			return
 		}
 	}