Browse Source

Load starlark files from soong

There are a number of instances where we are exporting information
from soong to bazel via soong_injection. This could be more bazel-centric
if the information was instead held in bzl files, and both bazel and
soong read it from there.

Add a starlark package that will run
//build/bazel/constants_exported_to_soong.bzl at initialization time,
and then results can be retreived with GetStarlarkValue.

Since changes to the starlark files mean that soong has to rerun,
add them as ninja deps.

Unfortunately, the starlark code has to be run at runtime rather than
pregenerating their results, because tests run from intellij wouldn't
go through any pregeneration steps. This means that starlark is run
multiple times during the build, once per test package and once per
primary builder invocation. (currently 3, could be reduced to 2 if we
made the symlink forest generation into its own standalone tool) The
starlark code we have so far in this cl is very fast, roughly half a
millisecond, so it's not a big deal for now, but something to keep an
eye on as we add more starlark constants.

Bug: 279095899
Test: go test
Change-Id: I1e7ca1df1d8d67333cbfc46e8396e229820e4476
Cole Faust 1 year ago
parent
commit
c9508aac4c

+ 1 - 0
android/Android.bp

@@ -17,6 +17,7 @@ bootstrap_go_package {
         "soong-remoteexec",
         "soong-response",
         "soong-shared",
+        "soong-starlark",
         "soong-starlark-format",
         "soong-ui-metrics_proto",
         "soong-android-allowlists",

+ 11 - 1
android/ninja_deps.go

@@ -14,7 +14,10 @@
 
 package android
 
-import "sort"
+import (
+	"android/soong/starlark_import"
+	"sort"
+)
 
 func (c *config) addNinjaFileDeps(deps ...string) {
 	for _, dep := range deps {
@@ -40,4 +43,11 @@ type ninjaDepsSingleton struct{}
 
 func (ninjaDepsSingleton) GenerateBuildActions(ctx SingletonContext) {
 	ctx.AddNinjaFileDeps(ctx.Config().ninjaFileDeps()...)
+
+	deps, err := starlark_import.GetNinjaDeps()
+	if err != nil {
+		ctx.Errorf("Error running starlark code: %s", err)
+	} else {
+		ctx.AddNinjaFileDeps(deps...)
+	}
 }

+ 7 - 0
bp2build/bp2build.go

@@ -15,6 +15,7 @@
 package bp2build
 
 import (
+	"android/soong/starlark_import"
 	"fmt"
 	"os"
 	"path/filepath"
@@ -93,6 +94,12 @@ func Codegen(ctx *CodegenContext) *CodegenMetrics {
 		os.Exit(1)
 	}
 	writeFiles(ctx, android.PathForOutput(ctx, bazel.SoongInjectionDirName), injectionFiles)
+	starlarkDeps, err := starlark_import.GetNinjaDeps()
+	if err != nil {
+		fmt.Fprintf(os.Stderr, "%s\n", err)
+		os.Exit(1)
+	}
+	ctx.AddNinjaFileDeps(starlarkDeps...)
 	return &res.metrics
 }
 

+ 9 - 0
cmd/soong_build/queryview.go

@@ -15,6 +15,7 @@
 package main
 
 import (
+	"android/soong/starlark_import"
 	"io/fs"
 	"io/ioutil"
 	"os"
@@ -47,6 +48,14 @@ func createBazelWorkspace(ctx *bp2build.CodegenContext, outDir string, generateF
 		}
 	}
 
+	// Add starlark deps here, so that they apply to both queryview and apibp2build which
+	// both run this function.
+	starlarkDeps, err2 := starlark_import.GetNinjaDeps()
+	if err2 != nil {
+		return err2
+	}
+	ctx.AddNinjaFileDeps(starlarkDeps...)
+
 	return nil
 }
 

+ 1 - 0
go.mod

@@ -6,4 +6,5 @@ require (
 	github.com/google/blueprint v0.0.0
 	google.golang.org/protobuf v0.0.0
 	prebuilts/bazel/common/proto/analysis_v2 v0.0.0
+	go.starlark.net v0.0.0
 )

+ 2 - 0
go.work

@@ -4,6 +4,7 @@ use (
 	.
 	../../external/go-cmp
 	../../external/golang-protobuf
+	../../external/starlark-go
 	../../prebuilts/bazel/common/proto/analysis_v2
 	../../prebuilts/bazel/common/proto/build
 	../blueprint
@@ -16,4 +17,5 @@ replace (
 	google.golang.org/protobuf v0.0.0 => ../../external/golang-protobuf
 	prebuilts/bazel/common/proto/analysis_v2 v0.0.0 => ../../prebuilts/bazel/common/proto/analysis_v2
 	prebuilts/bazel/common/proto/build v0.0.0 => ../../prebuilts/bazel/common/proto/build
+	go.starlark.net v0.0.0 => ../../external/starlark-go
 )

+ 36 - 0
starlark_import/Android.bp

@@ -0,0 +1,36 @@
+// Copyright 2023 Google Inc. All rights reserved.
+//
+// Licensed under the Apache License, Version 2.0 (the "License");
+// you may not use this file except in compliance with the License.
+// You may obtain a copy of the License at
+//
+//     http://www.apache.org/licenses/LICENSE-2.0
+//
+// Unless required by applicable law or agreed to in writing, software
+// distributed under the License is distributed on an "AS IS" BASIS,
+// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+// See the License for the specific language governing permissions and
+// limitations under the License.
+
+package {
+    default_applicable_licenses: ["Android-Apache-2.0"],
+}
+
+bootstrap_go_package {
+    name: "soong-starlark",
+    pkgPath: "android/soong/starlark_import",
+    srcs: [
+        "starlark_import.go",
+        "unmarshal.go",
+    ],
+    testSrcs: [
+        "starlark_import_test.go",
+        "unmarshal_test.go",
+    ],
+    deps: [
+        "go-starlark-starlark",
+        "go-starlark-starlarkstruct",
+        "go-starlark-starlarkjson",
+        "go-starlark-starlarktest",
+    ],
+}

+ 14 - 0
starlark_import/README.md

@@ -0,0 +1,14 @@
+# starlark_import package
+
+This allows soong to read constant information from starlark files. At package initialization
+time, soong will read `build/bazel/constants_exported_to_soong.bzl`, and then make the
+variables from that file available via `starlark_import.GetStarlarkValue()`. So to import
+a new variable, it must be added to `constants_exported_to_soong.bzl` and then it can
+be accessed by name.
+
+Only constant information can be read, since this is not a full bazel execution but a
+standalone starlark interpreter. This means you can't use bazel contructs like `rule`,
+`provider`, `select`, `glob`, etc.
+
+All starlark files that were loaded must be added as ninja deps that cause soong to rerun.
+The loaded files can be retrieved via `starlark_import.GetNinjaDeps()`.

+ 306 - 0
starlark_import/starlark_import.go

@@ -0,0 +1,306 @@
+// Copyright 2023 Google Inc. All rights reserved.
+//
+// Licensed under the Apache License, Version 2.0 (the "License");
+// you may not use this file except in compliance with the License.
+// You may obtain a copy of the License at
+//
+//     http://www.apache.org/licenses/LICENSE-2.0
+//
+// Unless required by applicable law or agreed to in writing, software
+// distributed under the License is distributed on an "AS IS" BASIS,
+// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+// See the License for the specific language governing permissions and
+// limitations under the License.
+
+package starlark_import
+
+import (
+	"fmt"
+	"os"
+	"path/filepath"
+	"sort"
+	"strings"
+	"sync"
+	"time"
+
+	"go.starlark.net/starlark"
+	"go.starlark.net/starlarkjson"
+	"go.starlark.net/starlarkstruct"
+)
+
+func init() {
+	go func() {
+		startTime := time.Now()
+		v, d, err := runStarlarkFile("//build/bazel/constants_exported_to_soong.bzl")
+		endTime := time.Now()
+		//fmt.Fprintf(os.Stderr, "starlark run time: %s\n", endTime.Sub(startTime).String())
+		globalResult.Set(starlarkResult{
+			values:    v,
+			ninjaDeps: d,
+			err:       err,
+			startTime: startTime,
+			endTime:   endTime,
+		})
+	}()
+}
+
+type starlarkResult struct {
+	values    starlark.StringDict
+	ninjaDeps []string
+	err       error
+	startTime time.Time
+	endTime   time.Time
+}
+
+// setOnce wraps a value and exposes Set() and Get() accessors for it.
+// The Get() calls will block until a Set() has been called.
+// A second call to Set() will panic.
+// setOnce must be created using newSetOnce()
+type setOnce[T any] struct {
+	value T
+	lock  sync.Mutex
+	wg    sync.WaitGroup
+	isSet bool
+}
+
+func (o *setOnce[T]) Set(value T) {
+	o.lock.Lock()
+	defer o.lock.Unlock()
+	if o.isSet {
+		panic("Value already set")
+	}
+
+	o.value = value
+	o.isSet = true
+	o.wg.Done()
+}
+
+func (o *setOnce[T]) Get() T {
+	if !o.isSet {
+		o.wg.Wait()
+	}
+	return o.value
+}
+
+func newSetOnce[T any]() *setOnce[T] {
+	result := &setOnce[T]{}
+	result.wg.Add(1)
+	return result
+}
+
+var globalResult = newSetOnce[starlarkResult]()
+
+func GetStarlarkValue[T any](key string) (T, error) {
+	result := globalResult.Get()
+	if result.err != nil {
+		var zero T
+		return zero, result.err
+	}
+	if !result.values.Has(key) {
+		var zero T
+		return zero, fmt.Errorf("a starlark variable by that name wasn't found, did you update //build/bazel/constants_exported_to_soong.bzl?")
+	}
+	return Unmarshal[T](result.values[key])
+}
+
+func GetNinjaDeps() ([]string, error) {
+	result := globalResult.Get()
+	if result.err != nil {
+		return nil, result.err
+	}
+	return result.ninjaDeps, nil
+}
+
+func getTopDir() (string, error) {
+	// It's hard to communicate the top dir to this package in any other way than reading the
+	// arguments directly, because we need to know this at package initialization time. Many
+	// soong constants that we'd like to read from starlark are initialized during package
+	// initialization.
+	for i, arg := range os.Args {
+		if arg == "--top" {
+			if i < len(os.Args)-1 && os.Args[i+1] != "" {
+				return os.Args[i+1], nil
+			}
+		}
+	}
+
+	// When running tests, --top is not passed. Instead, search for the top dir manually
+	cwd, err := os.Getwd()
+	if err != nil {
+		return "", err
+	}
+	for cwd != "/" {
+		if _, err := os.Stat(filepath.Join(cwd, "build/soong/soong_ui.bash")); err == nil {
+			return cwd, nil
+		}
+		cwd = filepath.Dir(cwd)
+	}
+	return "", fmt.Errorf("could not find top dir")
+}
+
+const callerDirKey = "callerDir"
+
+type modentry struct {
+	globals starlark.StringDict
+	err     error
+}
+
+func unsupportedMethod(t *starlark.Thread, fn *starlark.Builtin, _ starlark.Tuple, _ []starlark.Tuple) (starlark.Value, error) {
+	return nil, fmt.Errorf("%sthis file is read by soong, and must therefore be pure starlark and include only constant information. %q is not allowed", t.CallStack().String(), fn.Name())
+}
+
+var builtins = starlark.StringDict{
+	"aspect":     starlark.NewBuiltin("aspect", unsupportedMethod),
+	"glob":       starlark.NewBuiltin("glob", unsupportedMethod),
+	"json":       starlarkjson.Module,
+	"provider":   starlark.NewBuiltin("provider", unsupportedMethod),
+	"rule":       starlark.NewBuiltin("rule", unsupportedMethod),
+	"struct":     starlark.NewBuiltin("struct", starlarkstruct.Make),
+	"select":     starlark.NewBuiltin("select", unsupportedMethod),
+	"transition": starlark.NewBuiltin("transition", unsupportedMethod),
+}
+
+// Takes a module name (the first argument to the load() function) and returns the path
+// it's trying to load, stripping out leading //, and handling leading :s.
+func cleanModuleName(moduleName string, callerDir string) (string, error) {
+	if strings.Count(moduleName, ":") > 1 {
+		return "", fmt.Errorf("at most 1 colon must be present in starlark path: %s", moduleName)
+	}
+
+	// We don't have full support for external repositories, but at least support skylib's dicts.
+	if moduleName == "@bazel_skylib//lib:dicts.bzl" {
+		return "external/bazel-skylib/lib/dicts.bzl", nil
+	}
+
+	localLoad := false
+	if strings.HasPrefix(moduleName, "@//") {
+		moduleName = moduleName[3:]
+	} else if strings.HasPrefix(moduleName, "//") {
+		moduleName = moduleName[2:]
+	} else if strings.HasPrefix(moduleName, ":") {
+		moduleName = moduleName[1:]
+		localLoad = true
+	} else {
+		return "", fmt.Errorf("load path must start with // or :")
+	}
+
+	if ix := strings.LastIndex(moduleName, ":"); ix >= 0 {
+		moduleName = moduleName[:ix] + string(os.PathSeparator) + moduleName[ix+1:]
+	}
+
+	if filepath.Clean(moduleName) != moduleName {
+		return "", fmt.Errorf("load path must be clean, found: %s, expected: %s", moduleName, filepath.Clean(moduleName))
+	}
+	if strings.HasPrefix(moduleName, "../") {
+		return "", fmt.Errorf("load path must not start with ../: %s", moduleName)
+	}
+	if strings.HasPrefix(moduleName, "/") {
+		return "", fmt.Errorf("load path starts with /, use // for a absolute path: %s", moduleName)
+	}
+
+	if localLoad {
+		return filepath.Join(callerDir, moduleName), nil
+	}
+
+	return moduleName, nil
+}
+
+// loader implements load statement. The format of the loaded module URI is
+//
+//	[//path]:base
+//
+// The file path is $ROOT/path/base if path is present, <caller_dir>/base otherwise.
+func loader(thread *starlark.Thread, module string, topDir string, moduleCache map[string]*modentry, moduleCacheLock *sync.Mutex, filesystem map[string]string) (starlark.StringDict, error) {
+	modulePath, err := cleanModuleName(module, thread.Local(callerDirKey).(string))
+	if err != nil {
+		return nil, err
+	}
+	moduleCacheLock.Lock()
+	e, ok := moduleCache[modulePath]
+	if e == nil {
+		if ok {
+			moduleCacheLock.Unlock()
+			return nil, fmt.Errorf("cycle in load graph")
+		}
+
+		// Add a placeholder to indicate "load in progress".
+		moduleCache[modulePath] = nil
+		moduleCacheLock.Unlock()
+
+		childThread := &starlark.Thread{Name: "exec " + module, Load: thread.Load}
+
+		// Cheating for the sake of testing:
+		// propagate starlarktest's Reporter key, otherwise testing
+		// the load function may cause panic in starlarktest code.
+		const testReporterKey = "Reporter"
+		if v := thread.Local(testReporterKey); v != nil {
+			childThread.SetLocal(testReporterKey, v)
+		}
+
+		childThread.SetLocal(callerDirKey, filepath.Dir(modulePath))
+
+		if filesystem != nil {
+			globals, err := starlark.ExecFile(childThread, filepath.Join(topDir, modulePath), filesystem[modulePath], builtins)
+			e = &modentry{globals, err}
+		} else {
+			globals, err := starlark.ExecFile(childThread, filepath.Join(topDir, modulePath), nil, builtins)
+			e = &modentry{globals, err}
+		}
+
+		// Update the cache.
+		moduleCacheLock.Lock()
+		moduleCache[modulePath] = e
+	}
+	moduleCacheLock.Unlock()
+	return e.globals, e.err
+}
+
+// Run runs the given starlark file and returns its global variables and a list of all starlark
+// files that were loaded. The top dir for starlark's // is found via getTopDir().
+func runStarlarkFile(filename string) (starlark.StringDict, []string, error) {
+	topDir, err := getTopDir()
+	if err != nil {
+		return nil, nil, err
+	}
+	return runStarlarkFileWithFilesystem(filename, topDir, nil)
+}
+
+func runStarlarkFileWithFilesystem(filename string, topDir string, filesystem map[string]string) (starlark.StringDict, []string, error) {
+	if !strings.HasPrefix(filename, "//") && !strings.HasPrefix(filename, ":") {
+		filename = "//" + filename
+	}
+	filename, err := cleanModuleName(filename, "")
+	if err != nil {
+		return nil, nil, err
+	}
+	moduleCache := make(map[string]*modentry)
+	moduleCache[filename] = nil
+	moduleCacheLock := &sync.Mutex{}
+	mainThread := &starlark.Thread{
+		Name: "main",
+		Print: func(_ *starlark.Thread, msg string) {
+			// Ignore prints
+		},
+		Load: func(thread *starlark.Thread, module string) (starlark.StringDict, error) {
+			return loader(thread, module, topDir, moduleCache, moduleCacheLock, filesystem)
+		},
+	}
+	mainThread.SetLocal(callerDirKey, filepath.Dir(filename))
+
+	var result starlark.StringDict
+	if filesystem != nil {
+		result, err = starlark.ExecFile(mainThread, filepath.Join(topDir, filename), filesystem[filename], builtins)
+	} else {
+		result, err = starlark.ExecFile(mainThread, filepath.Join(topDir, filename), nil, builtins)
+	}
+	return result, sortedStringKeys(moduleCache), err
+}
+
+func sortedStringKeys(m map[string]*modentry) []string {
+	s := make([]string, 0, len(m))
+	for k := range m {
+		s = append(s, k)
+	}
+	sort.Strings(s)
+	return s
+}

+ 122 - 0
starlark_import/starlark_import_test.go

@@ -0,0 +1,122 @@
+// Copyright 2023 Google Inc. All rights reserved.
+//
+// Licensed under the Apache License, Version 2.0 (the "License");
+// you may not use this file except in compliance with the License.
+// You may obtain a copy of the License at
+//
+//     http://www.apache.org/licenses/LICENSE-2.0
+//
+// Unless required by applicable law or agreed to in writing, software
+// distributed under the License is distributed on an "AS IS" BASIS,
+// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+// See the License for the specific language governing permissions and
+// limitations under the License.
+
+package starlark_import
+
+import (
+	"strings"
+	"testing"
+
+	"go.starlark.net/starlark"
+)
+
+func TestBasic(t *testing.T) {
+	globals, _, err := runStarlarkFileWithFilesystem("a.bzl", "", map[string]string{
+		"a.bzl": `
+my_string = "hello, world!"
+`})
+	if err != nil {
+		t.Error(err)
+		return
+	}
+
+	if globals["my_string"].(starlark.String) != "hello, world!" {
+		t.Errorf("Expected %q, got %q", "hello, world!", globals["my_string"].String())
+	}
+}
+
+func TestLoad(t *testing.T) {
+	globals, _, err := runStarlarkFileWithFilesystem("a.bzl", "", map[string]string{
+		"a.bzl": `
+load("//b.bzl", _b_string = "my_string")
+my_string = "hello, " + _b_string
+`,
+		"b.bzl": `
+my_string = "world!"
+`})
+	if err != nil {
+		t.Error(err)
+		return
+	}
+
+	if globals["my_string"].(starlark.String) != "hello, world!" {
+		t.Errorf("Expected %q, got %q", "hello, world!", globals["my_string"].String())
+	}
+}
+
+func TestLoadRelative(t *testing.T) {
+	globals, ninjaDeps, err := runStarlarkFileWithFilesystem("a.bzl", "", map[string]string{
+		"a.bzl": `
+load(":b.bzl", _b_string = "my_string")
+load("//foo/c.bzl", _c_string = "my_string")
+my_string = "hello, " + _b_string
+c_string = _c_string
+`,
+		"b.bzl": `
+my_string = "world!"
+`,
+		"foo/c.bzl": `
+load(":d.bzl", _d_string = "my_string")
+my_string = "hello, " + _d_string
+`,
+		"foo/d.bzl": `
+my_string = "world!"
+`})
+	if err != nil {
+		t.Error(err)
+		return
+	}
+
+	if globals["my_string"].(starlark.String) != "hello, world!" {
+		t.Errorf("Expected %q, got %q", "hello, world!", globals["my_string"].String())
+	}
+
+	expectedNinjaDeps := []string{
+		"a.bzl",
+		"b.bzl",
+		"foo/c.bzl",
+		"foo/d.bzl",
+	}
+	if !slicesEqual(ninjaDeps, expectedNinjaDeps) {
+		t.Errorf("Expected %v ninja deps, got %v", expectedNinjaDeps, ninjaDeps)
+	}
+}
+
+func TestLoadCycle(t *testing.T) {
+	_, _, err := runStarlarkFileWithFilesystem("a.bzl", "", map[string]string{
+		"a.bzl": `
+load(":b.bzl", _b_string = "my_string")
+my_string = "hello, " + _b_string
+`,
+		"b.bzl": `
+load(":a.bzl", _a_string = "my_string")
+my_string = "hello, " + _a_string
+`})
+	if err == nil || !strings.Contains(err.Error(), "cycle in load graph") {
+		t.Errorf("Expected cycle in load graph, got: %v", err)
+		return
+	}
+}
+
+func slicesEqual[T comparable](a []T, b []T) bool {
+	if len(a) != len(b) {
+		return false
+	}
+	for i := range a {
+		if a[i] != b[i] {
+			return false
+		}
+	}
+	return true
+}

+ 288 - 0
starlark_import/unmarshal.go

@@ -0,0 +1,288 @@
+// Copyright 2023 Google Inc. All rights reserved.
+//
+// Licensed under the Apache License, Version 2.0 (the "License");
+// you may not use this file except in compliance with the License.
+// You may obtain a copy of the License at
+//
+//     http://www.apache.org/licenses/LICENSE-2.0
+//
+// Unless required by applicable law or agreed to in writing, software
+// distributed under the License is distributed on an "AS IS" BASIS,
+// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+// See the License for the specific language governing permissions and
+// limitations under the License.
+
+package starlark_import
+
+import (
+	"fmt"
+	"math"
+	"reflect"
+	"unsafe"
+
+	"go.starlark.net/starlark"
+	"go.starlark.net/starlarkstruct"
+)
+
+func Unmarshal[T any](value starlark.Value) (T, error) {
+	var zero T
+	x, err := UnmarshalReflect(value, reflect.TypeOf(zero))
+	return x.Interface().(T), err
+}
+
+func UnmarshalReflect(value starlark.Value, ty reflect.Type) (reflect.Value, error) {
+	zero := reflect.Zero(ty)
+	var result reflect.Value
+	if ty.Kind() == reflect.Interface {
+		var err error
+		ty, err = typeOfStarlarkValue(value)
+		if err != nil {
+			return zero, err
+		}
+	}
+	if ty.Kind() == reflect.Map {
+		result = reflect.MakeMap(ty)
+	} else {
+		result = reflect.Indirect(reflect.New(ty))
+	}
+
+	switch v := value.(type) {
+	case starlark.String:
+		if result.Type().Kind() != reflect.String {
+			return zero, fmt.Errorf("starlark type was %s, but %s requested", v.Type(), result.Type().Kind().String())
+		}
+		result.SetString(v.GoString())
+	case starlark.Int:
+		signedValue, signedOk := v.Int64()
+		unsignedValue, unsignedOk := v.Uint64()
+		switch result.Type().Kind() {
+		case reflect.Int64:
+			if !signedOk {
+				return zero, fmt.Errorf("starlark int didn't fit in go int64")
+			}
+			result.SetInt(signedValue)
+		case reflect.Int32:
+			if !signedOk || signedValue > math.MaxInt32 || signedValue < math.MinInt32 {
+				return zero, fmt.Errorf("starlark int didn't fit in go int32")
+			}
+			result.SetInt(signedValue)
+		case reflect.Int16:
+			if !signedOk || signedValue > math.MaxInt16 || signedValue < math.MinInt16 {
+				return zero, fmt.Errorf("starlark int didn't fit in go int16")
+			}
+			result.SetInt(signedValue)
+		case reflect.Int8:
+			if !signedOk || signedValue > math.MaxInt8 || signedValue < math.MinInt8 {
+				return zero, fmt.Errorf("starlark int didn't fit in go int8")
+			}
+			result.SetInt(signedValue)
+		case reflect.Int:
+			if !signedOk || signedValue > math.MaxInt || signedValue < math.MinInt {
+				return zero, fmt.Errorf("starlark int didn't fit in go int")
+			}
+			result.SetInt(signedValue)
+		case reflect.Uint64:
+			if !unsignedOk {
+				return zero, fmt.Errorf("starlark int didn't fit in go uint64")
+			}
+			result.SetUint(unsignedValue)
+		case reflect.Uint32:
+			if !unsignedOk || unsignedValue > math.MaxUint32 {
+				return zero, fmt.Errorf("starlark int didn't fit in go uint32")
+			}
+			result.SetUint(unsignedValue)
+		case reflect.Uint16:
+			if !unsignedOk || unsignedValue > math.MaxUint16 {
+				return zero, fmt.Errorf("starlark int didn't fit in go uint16")
+			}
+			result.SetUint(unsignedValue)
+		case reflect.Uint8:
+			if !unsignedOk || unsignedValue > math.MaxUint8 {
+				return zero, fmt.Errorf("starlark int didn't fit in go uint8")
+			}
+			result.SetUint(unsignedValue)
+		case reflect.Uint:
+			if !unsignedOk || unsignedValue > math.MaxUint {
+				return zero, fmt.Errorf("starlark int didn't fit in go uint")
+			}
+			result.SetUint(unsignedValue)
+		default:
+			return zero, fmt.Errorf("starlark type was %s, but %s requested", v.Type(), result.Type().Kind().String())
+		}
+	case starlark.Float:
+		f := float64(v)
+		switch result.Type().Kind() {
+		case reflect.Float64:
+			result.SetFloat(f)
+		case reflect.Float32:
+			if f > math.MaxFloat32 || f < -math.MaxFloat32 {
+				return zero, fmt.Errorf("starlark float didn't fit in go float32")
+			}
+			result.SetFloat(f)
+		default:
+			return zero, fmt.Errorf("starlark type was %s, but %s requested", v.Type(), result.Type().Kind().String())
+		}
+	case starlark.Bool:
+		if result.Type().Kind() != reflect.Bool {
+			return zero, fmt.Errorf("starlark type was %s, but %s requested", v.Type(), result.Type().Kind().String())
+		}
+		result.SetBool(bool(v))
+	case starlark.Tuple:
+		if result.Type().Kind() != reflect.Slice {
+			return zero, fmt.Errorf("starlark type was %s, but %s requested", v.Type(), result.Type().Kind().String())
+		}
+		elemType := result.Type().Elem()
+		// TODO: Add this grow call when we're on go 1.20
+		//result.Grow(v.Len())
+		for i := 0; i < v.Len(); i++ {
+			elem, err := UnmarshalReflect(v.Index(i), elemType)
+			if err != nil {
+				return zero, err
+			}
+			result = reflect.Append(result, elem)
+		}
+	case *starlark.List:
+		if result.Type().Kind() != reflect.Slice {
+			return zero, fmt.Errorf("starlark type was %s, but %s requested", v.Type(), result.Type().Kind().String())
+		}
+		elemType := result.Type().Elem()
+		// TODO: Add this grow call when we're on go 1.20
+		//result.Grow(v.Len())
+		for i := 0; i < v.Len(); i++ {
+			elem, err := UnmarshalReflect(v.Index(i), elemType)
+			if err != nil {
+				return zero, err
+			}
+			result = reflect.Append(result, elem)
+		}
+	case *starlark.Dict:
+		if result.Type().Kind() != reflect.Map {
+			return zero, fmt.Errorf("starlark type was %s, but %s requested", v.Type(), result.Type().Kind().String())
+		}
+		keyType := result.Type().Key()
+		valueType := result.Type().Elem()
+		for _, pair := range v.Items() {
+			key := pair.Index(0)
+			value := pair.Index(1)
+
+			unmarshalledKey, err := UnmarshalReflect(key, keyType)
+			if err != nil {
+				return zero, err
+			}
+			unmarshalledValue, err := UnmarshalReflect(value, valueType)
+			if err != nil {
+				return zero, err
+			}
+
+			result.SetMapIndex(unmarshalledKey, unmarshalledValue)
+		}
+	case *starlarkstruct.Struct:
+		if result.Type().Kind() != reflect.Struct {
+			return zero, fmt.Errorf("starlark type was %s, but %s requested", v.Type(), result.Type().Kind().String())
+		}
+		if result.NumField() != len(v.AttrNames()) {
+			return zero, fmt.Errorf("starlark struct and go struct have different number of fields (%d and %d)", len(v.AttrNames()), result.NumField())
+		}
+		for _, attrName := range v.AttrNames() {
+			attr, err := v.Attr(attrName)
+			if err != nil {
+				return zero, err
+			}
+
+			// TODO(b/279787235): this should probably support tags to rename the field
+			resultField := result.FieldByName(attrName)
+			if resultField == (reflect.Value{}) {
+				return zero, fmt.Errorf("starlark struct had field %s, but requested struct type did not", attrName)
+			}
+			// This hack allows us to change unexported fields
+			resultField = reflect.NewAt(resultField.Type(), unsafe.Pointer(resultField.UnsafeAddr())).Elem()
+			x, err := UnmarshalReflect(attr, resultField.Type())
+			if err != nil {
+				return zero, err
+			}
+			resultField.Set(x)
+		}
+	default:
+		return zero, fmt.Errorf("unimplemented starlark type: %s", value.Type())
+	}
+
+	return result, nil
+}
+
+func typeOfStarlarkValue(value starlark.Value) (reflect.Type, error) {
+	var err error
+	switch v := value.(type) {
+	case starlark.String:
+		return reflect.TypeOf(""), nil
+	case *starlark.List:
+		innerType := reflect.TypeOf("")
+		if v.Len() > 0 {
+			innerType, err = typeOfStarlarkValue(v.Index(0))
+			if err != nil {
+				return nil, err
+			}
+		}
+		for i := 1; i < v.Len(); i++ {
+			innerTypeI, err := typeOfStarlarkValue(v.Index(i))
+			if err != nil {
+				return nil, err
+			}
+			if innerType != innerTypeI {
+				return nil, fmt.Errorf("List must contain elements of entirely the same type, found %v and %v", innerType, innerTypeI)
+			}
+		}
+		return reflect.SliceOf(innerType), nil
+	case *starlark.Dict:
+		keyType := reflect.TypeOf("")
+		valueType := reflect.TypeOf("")
+		keys := v.Keys()
+		if v.Len() > 0 {
+			firstKey := keys[0]
+			keyType, err = typeOfStarlarkValue(firstKey)
+			if err != nil {
+				return nil, err
+			}
+			firstValue, found, err := v.Get(firstKey)
+			if !found {
+				err = fmt.Errorf("value not found")
+			}
+			if err != nil {
+				return nil, err
+			}
+			valueType, err = typeOfStarlarkValue(firstValue)
+			if err != nil {
+				return nil, err
+			}
+		}
+		for _, key := range keys {
+			keyTypeI, err := typeOfStarlarkValue(key)
+			if err != nil {
+				return nil, err
+			}
+			if keyType != keyTypeI {
+				return nil, fmt.Errorf("dict must contain elements of entirely the same type, found %v and %v", keyType, keyTypeI)
+			}
+			value, found, err := v.Get(key)
+			if !found {
+				err = fmt.Errorf("value not found")
+			}
+			if err != nil {
+				return nil, err
+			}
+			valueTypeI, err := typeOfStarlarkValue(value)
+			if valueType.Kind() != reflect.Interface && valueTypeI != valueType {
+				// If we see conflicting value types, change the result value type to an empty interface
+				valueType = reflect.TypeOf([]interface{}{}).Elem()
+			}
+		}
+		return reflect.MapOf(keyType, valueType), nil
+	case starlark.Int:
+		return reflect.TypeOf(0), nil
+	case starlark.Float:
+		return reflect.TypeOf(0.0), nil
+	case starlark.Bool:
+		return reflect.TypeOf(true), nil
+	default:
+		return nil, fmt.Errorf("unimplemented starlark type: %s", value.Type())
+	}
+}

+ 133 - 0
starlark_import/unmarshal_test.go

@@ -0,0 +1,133 @@
+// Copyright 2023 Google Inc. All rights reserved.
+//
+// Licensed under the Apache License, Version 2.0 (the "License");
+// you may not use this file except in compliance with the License.
+// You may obtain a copy of the License at
+//
+//     http://www.apache.org/licenses/LICENSE-2.0
+//
+// Unless required by applicable law or agreed to in writing, software
+// distributed under the License is distributed on an "AS IS" BASIS,
+// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+// See the License for the specific language governing permissions and
+// limitations under the License.
+
+package starlark_import
+
+import (
+	"reflect"
+	"testing"
+
+	"go.starlark.net/starlark"
+)
+
+func createStarlarkValue(t *testing.T, code string) starlark.Value {
+	t.Helper()
+	result, err := starlark.ExecFile(&starlark.Thread{}, "main.bzl", "x = "+code, builtins)
+	if err != nil {
+		panic(err)
+	}
+	return result["x"]
+}
+
+func TestUnmarshallConcreteType(t *testing.T) {
+	x, err := Unmarshal[string](createStarlarkValue(t, `"foo"`))
+	if err != nil {
+		t.Error(err)
+		return
+	}
+	if x != "foo" {
+		t.Errorf(`Expected "foo", got %q`, x)
+	}
+}
+
+func TestUnmarshallConcreteTypeWithInterfaces(t *testing.T) {
+	x, err := Unmarshal[map[string]map[string]interface{}](createStarlarkValue(t,
+		`{"foo": {"foo2": "foo3"}, "bar": {"bar2": ["bar3"]}}`))
+	if err != nil {
+		t.Error(err)
+		return
+	}
+	expected := map[string]map[string]interface{}{
+		"foo": {"foo2": "foo3"},
+		"bar": {"bar2": []string{"bar3"}},
+	}
+	if !reflect.DeepEqual(x, expected) {
+		t.Errorf(`Expected %v, got %v`, expected, x)
+	}
+}
+
+func TestUnmarshall(t *testing.T) {
+	testCases := []struct {
+		input    string
+		expected interface{}
+	}{
+		{
+			input:    `"foo"`,
+			expected: "foo",
+		},
+		{
+			input:    `5`,
+			expected: 5,
+		},
+		{
+			input:    `["foo", "bar"]`,
+			expected: []string{"foo", "bar"},
+		},
+		{
+			input:    `("foo", "bar")`,
+			expected: []string{"foo", "bar"},
+		},
+		{
+			input:    `("foo",5)`,
+			expected: []interface{}{"foo", 5},
+		},
+		{
+			input:    `{"foo": 5, "bar": 10}`,
+			expected: map[string]int{"foo": 5, "bar": 10},
+		},
+		{
+			input:    `{"foo": ["qux"], "bar": []}`,
+			expected: map[string][]string{"foo": {"qux"}, "bar": nil},
+		},
+		{
+			input: `struct(Foo="foo", Bar=5)`,
+			expected: struct {
+				Foo string
+				Bar int
+			}{Foo: "foo", Bar: 5},
+		},
+		{
+			// Unexported fields version of the above
+			input: `struct(foo="foo", bar=5)`,
+			expected: struct {
+				foo string
+				bar int
+			}{foo: "foo", bar: 5},
+		},
+		{
+			input: `{"foo": "foo2", "bar": ["bar2"], "baz": 5, "qux": {"qux2": "qux3"}, "quux": {"quux2": "quux3", "quux4": 5}}`,
+			expected: map[string]interface{}{
+				"foo": "foo2",
+				"bar": []string{"bar2"},
+				"baz": 5,
+				"qux": map[string]string{"qux2": "qux3"},
+				"quux": map[string]interface{}{
+					"quux2": "quux3",
+					"quux4": 5,
+				},
+			},
+		},
+	}
+
+	for _, tc := range testCases {
+		x, err := UnmarshalReflect(createStarlarkValue(t, tc.input), reflect.TypeOf(tc.expected))
+		if err != nil {
+			t.Error(err)
+			continue
+		}
+		if !reflect.DeepEqual(x.Interface(), tc.expected) {
+			t.Errorf(`Expected %#v, got %#v`, tc.expected, x.Interface())
+		}
+	}
+}

+ 14 - 0
tests/bp2build_bazel_test.sh

@@ -53,6 +53,20 @@ EOF
   if [[ "$buildfile_mtime1" != "$buildfile_mtime2" ]]; then
     fail "BUILD.bazel was updated even though contents are same"
   fi
+
+  # Force bp2build to rerun by updating the timestamp of the constants_exported_to_soong.bzl file.
+  touch build/bazel/constants_exported_to_soong.bzl
+
+  run_soong bp2build
+  local -r buildfile_mtime3=$(stat -c "%y" out/soong/bp2build/pkg/BUILD.bazel)
+  local -r marker_mtime3=$(stat -c "%y" out/soong/bp2build_workspace_marker)
+
+  if [[ "$marker_mtime2" == "$marker_mtime3" ]]; then
+    fail "Expected bp2build marker file to change"
+  fi
+  if [[ "$buildfile_mtime2" != "$buildfile_mtime3" ]]; then
+    fail "BUILD.bazel was updated even though contents are same"
+  fi
 }
 
 # Tests that blueprint files that are deleted are not present when the

+ 2 - 2
tests/persistent_bazel_test.sh

@@ -73,8 +73,8 @@ function test_bazel_failure {
 
   USE_PERSISTENT_BAZEL=1 run_soong nothing 1>out/failurelog.txt 2>&1 && fail "Expected build failure" || true
 
-  if ! grep -sq "'build/bazel/rules' is not a package" out/failurelog.txt ; then
-    fail "Expected error to contain 'build/bazel/rules' is not a package, instead got:\n$(cat out/failurelog.txt)"
+  if ! grep -sq "cannot load //build/bazel/rules/common/api_constants.bzl" out/failurelog.txt ; then
+    fail "Expected error to contain 'cannot load //build/bazel/rules/common/api_constants.bzl', instead got:\n$(cat out/failurelog.txt)"
   fi
 
   kill $(cat out/bazel/output/server/server.pid.txt) 2>/dev/null || true