Merge changes from topics "dist_for_goals", "mk2star"

* changes:
  Generate runtime conversion diagnostics
  Convert dist-for-goals.
This commit is contained in:
Treehugger Robot 2021-11-18 22:04:52 +00:00 committed by Gerrit Code Review
commit f18bedf1c3
6 changed files with 126 additions and 146 deletions

View file

@ -46,8 +46,6 @@ var (
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 @@ func init() {
flagAlias("root", "d")
flagAlias("dry_run", "n")
flagAlias("convert_dependents", "r")
flagAlias("no_warnings", "w")
flagAlias("error_stat", "e")
}
@ -207,9 +204,9 @@ func main() {
}
}
printStats()
if *errstat {
errorLogger.printStatistics()
printStats()
}
if !ok {
os.Exit(1)
@ -336,7 +333,6 @@ func convertOne(mkFile string) (ok bool) {
OutputSuffix: *suffix,
TracedVariables: tracedVariables,
TraceCalls: *traceCalls,
WarnPartialSuccess: !*noWarn,
SourceFS: os.DirFS(*rootDir),
MakefileFinder: makefileFinder,
ErrorLogger: errorLogger,
@ -417,9 +413,6 @@ func writeGenerated(path string, contents string) error {
func printStats() {
var sortedFiles []string
if *noWarn && !*verbose {
return
}
for p := range converted {
sortedFiles = append(sortedFiles, p)
}
@ -435,7 +428,6 @@ func printStats() {
nOk++
}
}
if !*noWarn {
if nPartial > 0 {
fmt.Fprintf(os.Stderr, "Conversion was partially successful for:\n")
for _, f := range sortedFiles {
@ -454,10 +446,6 @@ func printStats() {
}
}
}
if *verbose && (nPartial > 0 || nFailed > 0) {
fmt.Fprintln(os.Stderr, "Succeeded: ", nOk, " Partial: ", nPartial, " Failed: ", nFailed)
}
}
type datum struct {
count int
@ -468,8 +456,8 @@ type errorSink struct {
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 {

View file

@ -18,8 +18,6 @@ import (
"fmt"
"strconv"
"strings"
mkparser "android/soong/androidmk/parser"
)
// Represents an expression in the Starlark code. An expression has
@ -106,7 +104,7 @@ func (xi *interpolateExpr) emit(gctx *generationContext) {
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 @@ func (xi *interpolateExpr) emit(gctx *generationContext) {
}
}
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(")")
@ -427,7 +425,7 @@ func newStringListExpr(items []string) *listExpr {
return &v
}
// concatExpr generates epxr1 + expr2 + ... + exprN in Starlark.
// concatExpr generates expr1 + expr2 + ... + exprN in Starlark.
type concatExpr struct {
items []starlarkExpr
}
@ -620,7 +618,7 @@ func (cx *callExpr) emitListVarCopy(gctx *generationContext) {
}
type badExpr struct {
node mkparser.Node
errorLocation ErrorLocation
message string
}
@ -630,15 +628,15 @@ func (b *badExpr) eval(_ map[string]starlarkExpr) (res starlarkExpr, same bool)
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")
}

View file

@ -102,6 +102,7 @@ var knownFunctions = map[string]struct {
"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},
@ -164,7 +165,6 @@ type Request struct {
ErrorLogger ErrorLogger
TracedVariables []string // trace assignment to these variables
TraceCalls bool
WarnPartialSuccess bool
SourceFS fs.FS
MakefileFinder MakefileFinder
}
@ -172,7 +172,16 @@ type Request struct {
// 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 @@ func (gctx *generationContext) emit() string {
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 @@ func (gctx *generationContext) newLine() {
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
@ -378,7 +387,6 @@ type StarlarkScript struct {
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
@ -560,7 +568,7 @@ func (ctx *parseContext) handleAssignment(a *mkparser.Assignment) {
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) parseCondition(check *mkparser.Directive) starlarkNode
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 @@ func (ctx *parseContext) carryAsComment(failedNode mkparser.Node) {
// 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 @@ func (ctx *parseContext) hasNamespaceVar(namespaceName string, varName string) b
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()
}
@ -1663,7 +1671,6 @@ func Convert(req Request) (*StarlarkScript, error) {
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 },

View file

@ -105,15 +105,12 @@ def init(g, handle):
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 @@ define some-macro
$(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 @@ def init(g, handle):
# 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\"")
`,
},
{
@ -704,6 +695,7 @@ $(call enforce-product-packages-exist,)
$(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")
@ -713,6 +705,7 @@ def init(g, handle):
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")
`,
},
{
@ -872,9 +865,7 @@ def init(g, handle):
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",
@ -1061,15 +1052,11 @@ def init(g, handle):
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 $@ $*")
`,
},
{
@ -1077,14 +1064,27 @@ def init(g, handle):
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
`,
},
}
@ -1160,7 +1160,6 @@ func TestGood(t *testing.T) {
Reader: bytes.NewBufferString(test.in),
RootDir: ".",
OutputSuffix: ".star",
WarnPartialSuccess: true,
SourceFS: fs,
MakefileFinder: &testMakefileFinder{fs: fs},
})

View file

@ -183,6 +183,7 @@ type assignmentNode struct {
value starlarkExpr
mkValue *mkparser.MakeString
flavor assignmentFlavor
location ErrorLocation
isTraced bool
previous *assignmentNode
}
@ -223,18 +224,6 @@ func (in *ifNode) emit(gctx *generationContext) {
}
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(":")

View file

@ -228,10 +228,9 @@ func (pv predefinedVariable) emitSet(gctx *generationContext, asgn *assignmentNo
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
}