Use strings instead of simpleNinjaStrings where possible am: 95bec3331c

Original change: https://android-review.googlesource.com/c/platform/build/blueprint/+/2813840

Change-Id: I729a500def4b5f8cfe51e8a2461df90a9ea90c80
Signed-off-by: Automerger Merge Worker <android-build-automerger-merge-worker@system.gserviceaccount.com>

bootstrap/command.go

@@ -135,7 +135,7 @@ func RunBlueprint(args Args, stopBefore StopBefore, ctx *blueprint.Context, conf
 	}
 
 	const outFilePermissions = 0666
-	var out io.StringWriter
+	var out blueprint.StringWriterWriter
 	var f *os.File
 	var buf *bufio.Writer
 
@@ -145,7 +145,7 @@ func RunBlueprint(args Args, stopBefore StopBefore, ctx *blueprint.Context, conf
 		if err := os.WriteFile(joinPath(ctx.SrcDir(), args.OutFile), []byte(nil), outFilePermissions); err != nil {
 			return nil, fmt.Errorf("error writing empty Ninja file: %s", err)
 		}
-		out = io.Discard.(io.StringWriter)
+		out = io.Discard.(blueprint.StringWriterWriter)
 	} else {
 		f, err := os.OpenFile(joinPath(ctx.SrcDir(), args.OutFile), os.O_WRONLY|os.O_CREATE|os.O_TRUNC, outFilePermissions)
 		if err != nil {

context.go

@@ -2786,11 +2786,16 @@ func jsonModuleWithActionsFromModuleInfo(m *moduleInfo) *JsonModule {
 	var actions []JSONAction
 	for _, bDef := range m.actionDefs.buildDefs {
 		a := JSONAction{
-			Inputs: append(
+			Inputs: append(append(append(
-				getNinjaStringsWithNilPkgNames(bDef.Inputs),
+				bDef.InputStrings,
+				bDef.ImplicitStrings...),
+				getNinjaStringsWithNilPkgNames(bDef.Inputs)...),
 				getNinjaStringsWithNilPkgNames(bDef.Implicits)...),
-			Outputs: append(
+
-				getNinjaStringsWithNilPkgNames(bDef.Outputs),
+			Outputs: append(append(append(
+				bDef.OutputStrings,
+				bDef.ImplicitOutputStrings...),
+				getNinjaStringsWithNilPkgNames(bDef.Outputs)...),
 				getNinjaStringsWithNilPkgNames(bDef.ImplicitOutputs)...),
 		}
 		if d, ok := bDef.Variables["description"]; ok {
@@ -3992,6 +3997,9 @@ func (c *Context) AllTargets() (map[string]string, error) {
 				}
 				targets[outputValue] = ruleName
 			}
+			for _, output := range append(buildDef.OutputStrings, buildDef.ImplicitOutputStrings...) {
+				targets[output] = ruleName
+			}
 		}
 		return nil
 	}
@@ -4214,7 +4222,7 @@ func (c *Context) SingletonName(singleton Singleton) string {
 // WriteBuildFile writes the Ninja manifest text for the generated build
 // actions to w.  If this is called before PrepareBuildActions successfully
 // completes then ErrBuildActionsNotReady is returned.
-func (c *Context) WriteBuildFile(w io.StringWriter) error {
+func (c *Context) WriteBuildFile(w StringWriterWriter) error {
 	var err error
 	pprof.Do(c.Context, pprof.Labels("blueprint", "WriteBuildFile"), func(ctx context.Context) {
 		if !c.buildActionsReady {
@@ -4696,7 +4704,7 @@ type phonyCandidate struct {
 // keyForPhonyCandidate gives a unique identifier for a set of deps.
 // If any of the deps use a variable, we return an empty string to signal
 // that this set of deps is ineligible for extraction.
-func keyForPhonyCandidate(deps []*ninjaString) string {
+func keyForPhonyCandidate(deps []*ninjaString, stringDeps []string) string {
 	hasher := sha256.New()
 	for _, d := range deps {
 		if len(d.Variables()) != 0 {
@@ -4704,6 +4712,9 @@ func keyForPhonyCandidate(deps []*ninjaString) string {
 		}
 		io.WriteString(hasher, d.Value(nil))
 	}
+	for _, d := range stringDeps {
+		io.WriteString(hasher, d)
+	}
 	return base64.RawURLEncoding.EncodeToString(hasher.Sum(nil))
 }
 
@@ -4713,7 +4724,7 @@ func keyForPhonyCandidate(deps []*ninjaString) string {
 // (and phonyCandidate#first.OrderOnly) will be replaced with phonyCandidate#phony.Outputs
 func scanBuildDef(wg *sync.WaitGroup, candidates *sync.Map, phonyCount *atomic.Uint32, b *buildDef) {
 	defer wg.Done()
-	key := keyForPhonyCandidate(b.OrderOnly)
+	key := keyForPhonyCandidate(b.OrderOnly, b.OrderOnlyStrings)
 	if key == "" {
 		return
 	}
@@ -4726,17 +4737,20 @@ func scanBuildDef(wg *sync.WaitGroup, candidates *sync.Map, phonyCount *atomic.U
 			// extract it as a phony output
 			phonyCount.Add(1)
 			m.phony = &buildDef{
-				Rule:     Phony,
+				Rule:          Phony,
-				Outputs:  []*ninjaString{simpleNinjaString("dedup-" + key)},
+				OutputStrings: []string{"dedup-" + key},
-				Inputs:   m.first.OrderOnly, //we could also use b.OrderOnly
+				Inputs:        m.first.OrderOnly, //we could also use b.OrderOnly
-				Optional: true,
+				InputStrings:  m.first.OrderOnlyStrings,
+				Optional:      true,
 			}
 			// the previously recorded build-def, which first had these deps as its
 			// order-only deps, should now use this phony output instead
-			m.first.OrderOnly = m.phony.Outputs
+			m.first.OrderOnlyStrings = m.phony.OutputStrings
+			m.first.OrderOnly = nil
 			m.first = nil
 		})
-		b.OrderOnly = m.phony.Outputs
+		b.OrderOnlyStrings = m.phony.OutputStrings
+		b.OrderOnly = nil
 	}
 }
 
@@ -4753,7 +4767,7 @@ func (c *Context) deduplicateOrderOnlyDeps(infos []*moduleInfo) *localBuildActio
 	wg := sync.WaitGroup{}
 	for _, info := range infos {
 		for _, b := range info.actionDefs.buildDefs {
-			if len(b.OrderOnly) > 0 {
+			if len(b.OrderOnly) > 0 || len(b.OrderOnlyStrings) > 0 {
 				wg.Add(1)
 				go scanBuildDef(&wg, &candidates, &phonyCount, b)
 			}
@@ -4774,7 +4788,7 @@ func (c *Context) deduplicateOrderOnlyDeps(infos []*moduleInfo) *localBuildActio
 	c.EventHandler.Do("sort_phony_builddefs", func() {
 		// sorting for determinism, the phony output names are stable
 		sort.Slice(phonys, func(i int, j int) bool {
-			return phonys[i].Outputs[0].Value(nil) < phonys[j].Outputs[0].Value(nil)
+			return phonys[i].OutputStrings[0] < phonys[j].OutputStrings[0]
 		})
 	})
 

context_test.go

@@ -1159,18 +1159,11 @@ func TestPackageIncludes(t *testing.T) {
 }
 
 func TestDeduplicateOrderOnlyDeps(t *testing.T) {
-	outputs := func(names ...string) []*ninjaString {
-		r := make([]*ninjaString, len(names))
-		for i, name := range names {
-			r[i] = simpleNinjaString(name)
-		}
-		return r
-	}
 	b := func(output string, inputs []string, orderOnlyDeps []string) *buildDef {
 		return &buildDef{
-			Outputs:   outputs(output),
+			OutputStrings:    []string{output},
-			Inputs:    outputs(inputs...),
+			InputStrings:     inputs,
-			OrderOnly: outputs(orderOnlyDeps...),
+			OrderOnlyStrings: orderOnlyDeps,
 		}
 	}
 	m := func(bs ...*buildDef) *moduleInfo {
@@ -1179,7 +1172,7 @@ func TestDeduplicateOrderOnlyDeps(t *testing.T) {
 	type testcase struct {
 		modules        []*moduleInfo
 		expectedPhonys []*buildDef
-		conversions    map[string][]*ninjaString
+		conversions    map[string][]string
 	}
 	testCases := []testcase{{
 		modules: []*moduleInfo{
@@ -1189,9 +1182,9 @@ func TestDeduplicateOrderOnlyDeps(t *testing.T) {
 		expectedPhonys: []*buildDef{
 			b("dedup-GKw-c0PwFokMUQ6T-TUmEWnZ4_VlQ2Qpgw-vCTT0-OQ", []string{"d"}, nil),
 		},
-		conversions: map[string][]*ninjaString{
+		conversions: map[string][]string{
-			"A": outputs("dedup-GKw-c0PwFokMUQ6T-TUmEWnZ4_VlQ2Qpgw-vCTT0-OQ"),
+			"A": []string{"dedup-GKw-c0PwFokMUQ6T-TUmEWnZ4_VlQ2Qpgw-vCTT0-OQ"},
-			"B": outputs("dedup-GKw-c0PwFokMUQ6T-TUmEWnZ4_VlQ2Qpgw-vCTT0-OQ"),
+			"B": []string{"dedup-GKw-c0PwFokMUQ6T-TUmEWnZ4_VlQ2Qpgw-vCTT0-OQ"},
 		},
 	}, {
 		modules: []*moduleInfo{
@@ -1205,10 +1198,10 @@ func TestDeduplicateOrderOnlyDeps(t *testing.T) {
 			m(b("C", nil, []string{"a"})),
 		},
 		expectedPhonys: []*buildDef{b("dedup-ypeBEsobvcr6wjGzmiPcTaeG7_gUfE5yuYB3ha_uSLs", []string{"a"}, nil)},
-		conversions: map[string][]*ninjaString{
+		conversions: map[string][]string{
-			"A": outputs("dedup-ypeBEsobvcr6wjGzmiPcTaeG7_gUfE5yuYB3ha_uSLs"),
+			"A": []string{"dedup-ypeBEsobvcr6wjGzmiPcTaeG7_gUfE5yuYB3ha_uSLs"},
-			"B": outputs("b"),
+			"B": []string{"b"},
-			"C": outputs("dedup-ypeBEsobvcr6wjGzmiPcTaeG7_gUfE5yuYB3ha_uSLs"),
+			"C": []string{"dedup-ypeBEsobvcr6wjGzmiPcTaeG7_gUfE5yuYB3ha_uSLs"},
 		},
 	}, {
 		modules: []*moduleInfo{
@@ -1220,11 +1213,11 @@ func TestDeduplicateOrderOnlyDeps(t *testing.T) {
 		expectedPhonys: []*buildDef{
 			b("dedup--44g_C5MPySMYMOb1lLzwTRymLuXe4tNWQO4UFViBgM", []string{"a", "b"}, nil),
 			b("dedup-9F3lHN7zCZFVHkHogt17VAR5lkigoAdT9E_JZuYVP8E", []string{"a", "c"}, nil)},
-		conversions: map[string][]*ninjaString{
+		conversions: map[string][]string{
-			"A": outputs("dedup--44g_C5MPySMYMOb1lLzwTRymLuXe4tNWQO4UFViBgM"),
+			"A": []string{"dedup--44g_C5MPySMYMOb1lLzwTRymLuXe4tNWQO4UFViBgM"},
-			"B": outputs("dedup--44g_C5MPySMYMOb1lLzwTRymLuXe4tNWQO4UFViBgM"),
+			"B": []string{"dedup--44g_C5MPySMYMOb1lLzwTRymLuXe4tNWQO4UFViBgM"},
-			"C": outputs("dedup-9F3lHN7zCZFVHkHogt17VAR5lkigoAdT9E_JZuYVP8E"),
+			"C": []string{"dedup-9F3lHN7zCZFVHkHogt17VAR5lkigoAdT9E_JZuYVP8E"},
-			"D": outputs("dedup-9F3lHN7zCZFVHkHogt17VAR5lkigoAdT9E_JZuYVP8E"),
+			"D": []string{"dedup-9F3lHN7zCZFVHkHogt17VAR5lkigoAdT9E_JZuYVP8E"},
 		},
 	}}
 	for index, tc := range testCases {
@@ -1253,7 +1246,7 @@ func TestDeduplicateOrderOnlyDeps(t *testing.T) {
 			find := func(k string) *buildDef {
 				for _, m := range tc.modules {
 					for _, b := range m.actionDefs.buildDefs {
-						if reflect.DeepEqual(b.Outputs, outputs(k)) {
+						if reflect.DeepEqual(b.OutputStrings, []string{k}) {
 							return b
 						}
 					}
@@ -1265,7 +1258,7 @@ func TestDeduplicateOrderOnlyDeps(t *testing.T) {
 				if actual == nil {
 					t.Errorf("Couldn't find %s", k)
 				}
-				if !reflect.DeepEqual(actual.OrderOnly, conversion) {
+				if !reflect.DeepEqual(actual.OrderOnlyStrings, conversion) {
 					t.Errorf("expected %s.OrderOnly = %v but got %v", k, conversion, actual.OrderOnly)
 				}
 			}

go.mod

@@ -1,3 +1,3 @@
 module github.com/google/blueprint
 
-go 1.18
+go 1.20

ninja_defs.go

@@ -261,18 +261,24 @@ func (r *ruleDef) WriteTo(nw *ninjaWriter, name string,
 
 // A buildDef describes a build target definition.
 type buildDef struct {
-	Comment         string
+	Comment               string
-	Rule            Rule
+	Rule                  Rule
-	RuleDef         *ruleDef
+	RuleDef               *ruleDef
-	Outputs         []*ninjaString
+	Outputs               []*ninjaString
-	ImplicitOutputs []*ninjaString
+	OutputStrings         []string
-	Inputs          []*ninjaString
+	ImplicitOutputs       []*ninjaString
-	Implicits       []*ninjaString
+	ImplicitOutputStrings []string
-	OrderOnly       []*ninjaString
+	Inputs                []*ninjaString
-	Validations     []*ninjaString
+	InputStrings          []string
-	Args            map[Variable]*ninjaString
+	Implicits             []*ninjaString
-	Variables       map[string]*ninjaString
+	ImplicitStrings       []string
-	Optional        bool
+	OrderOnly             []*ninjaString
+	OrderOnlyStrings      []string
+	Validations           []*ninjaString
+	ValidationStrings     []string
+	Args                  map[Variable]*ninjaString
+	Variables             map[string]*ninjaString
+	Optional              bool
 }
 
 func formatTags(tags map[string]string, rule Rule) string {
@@ -319,32 +325,32 @@ func parseBuildParams(scope scope, params *BuildParams,
 	}
 
 	var err error
-	b.Outputs, err = parseNinjaStrings(scope, params.Outputs)
+	b.Outputs, b.OutputStrings, err = parseNinjaOrSimpleStrings(scope, params.Outputs)
 	if err != nil {
 		return nil, fmt.Errorf("error parsing Outputs param: %s", err)
 	}
 
-	b.ImplicitOutputs, err = parseNinjaStrings(scope, params.ImplicitOutputs)
+	b.ImplicitOutputs, b.ImplicitOutputStrings, err = parseNinjaOrSimpleStrings(scope, params.ImplicitOutputs)
 	if err != nil {
 		return nil, fmt.Errorf("error parsing ImplicitOutputs param: %s", err)
 	}
 
-	b.Inputs, err = parseNinjaStrings(scope, params.Inputs)
+	b.Inputs, b.InputStrings, err = parseNinjaOrSimpleStrings(scope, params.Inputs)
 	if err != nil {
 		return nil, fmt.Errorf("error parsing Inputs param: %s", err)
 	}
 
-	b.Implicits, err = parseNinjaStrings(scope, params.Implicits)
+	b.Implicits, b.ImplicitStrings, err = parseNinjaOrSimpleStrings(scope, params.Implicits)
 	if err != nil {
 		return nil, fmt.Errorf("error parsing Implicits param: %s", err)
 	}
 
-	b.OrderOnly, err = parseNinjaStrings(scope, params.OrderOnly)
+	b.OrderOnly, b.OrderOnlyStrings, err = parseNinjaOrSimpleStrings(scope, params.OrderOnly)
 	if err != nil {
 		return nil, fmt.Errorf("error parsing OrderOnly param: %s", err)
 	}
 
-	b.Validations, err = parseNinjaStrings(scope, params.Validations)
+	b.Validations, b.ValidationStrings, err = parseNinjaOrSimpleStrings(scope, params.Validations)
 	if err != nil {
 		return nil, fmt.Errorf("error parsing Validations param: %s", err)
 	}
@@ -411,14 +417,20 @@ func parseBuildParams(scope scope, params *BuildParams,
 
 func (b *buildDef) WriteTo(nw *ninjaWriter, pkgNames map[*packageContext]string) error {
 	var (
-		comment       = b.Comment
+		comment             = b.Comment
-		rule          = b.Rule.fullName(pkgNames)
+		rule                = b.Rule.fullName(pkgNames)
-		outputs       = b.Outputs
+		outputs             = b.Outputs
-		implicitOuts  = b.ImplicitOutputs
+		implicitOuts        = b.ImplicitOutputs
-		explicitDeps  = b.Inputs
+		explicitDeps        = b.Inputs
-		implicitDeps  = b.Implicits
+		implicitDeps        = b.Implicits
-		orderOnlyDeps = b.OrderOnly
+		orderOnlyDeps       = b.OrderOnly
-		validations   = b.Validations
+		validations         = b.Validations
+		outputStrings       = b.OutputStrings
+		implicitOutStrings  = b.ImplicitOutputStrings
+		explicitDepStrings  = b.InputStrings
+		implicitDepStrings  = b.ImplicitStrings
+		orderOnlyDepStrings = b.OrderOnlyStrings
+		validationStrings   = b.ValidationStrings
 	)
 
 	if b.RuleDef != nil {
@@ -426,7 +438,10 @@ func (b *buildDef) WriteTo(nw *ninjaWriter, pkgNames map[*packageContext]string)
 		orderOnlyDeps = append(b.RuleDef.CommandOrderOnly, orderOnlyDeps...)
 	}
 
-	err := nw.Build(comment, rule, outputs, implicitOuts, explicitDeps, implicitDeps, orderOnlyDeps, validations, pkgNames)
+	err := nw.Build(comment, rule, outputs, implicitOuts, explicitDeps, implicitDeps, orderOnlyDeps, validations,
+		outputStrings, implicitOutStrings, explicitDepStrings,
+		implicitDepStrings, orderOnlyDepStrings, validationStrings,
+		pkgNames)
 	if err != nil {
 		return err
 	}
@@ -456,7 +471,7 @@ func (b *buildDef) WriteTo(nw *ninjaWriter, pkgNames map[*packageContext]string)
 	}
 
 	if !b.Optional {
-		err = nw.Default(pkgNames, outputs...)
+		err = nw.Default(pkgNames, outputs, outputStrings)
 		if err != nil {
 			return err
 		}

ninja_strings.go

@@ -87,16 +87,31 @@ func (ps *parseState) pushVariable(start, end int, v Variable) {
 type stateFunc func(*parseState, int, rune) (stateFunc, error)
 
 // parseNinjaString parses an unescaped ninja string (i.e. all $<something>
-// occurrences are expected to be variables or $$) and returns a list of the
+// occurrences are expected to be variables or $$) and returns a *ninjaString
-// variable names that the string references.
+// that contains the original string and a list of the referenced variables.
 func parseNinjaString(scope scope, str string) (*ninjaString, error) {
+	ninjaString, str, err := parseNinjaOrSimpleString(scope, str)
+	if err != nil {
+		return nil, err
+	}
+	if ninjaString != nil {
+		return ninjaString, nil
+	}
+	return simpleNinjaString(str), nil
+}
+
+// parseNinjaOrSimpleString parses an unescaped ninja string (i.e. all $<something>
+// occurrences are expected to be variables or $$) and returns either a *ninjaString
+// if the string contains ninja variable references, or the original string and nil
+// for the *ninjaString if it doesn't.
+func parseNinjaOrSimpleString(scope scope, str string) (*ninjaString, string, error) {
 	// naively pre-allocate slice by counting $ signs
 	n := strings.Count(str, "$")
 	if n == 0 {
 		if len(str) > 0 && str[0] == ' ' {
 			str = "$" + str
 		}
-		return simpleNinjaString(str), nil
+		return nil, str, nil
 	}
 	variableReferences := make([]variableReference, 0, n)
 	result := &ninjaString{
@@ -116,13 +131,13 @@ func parseNinjaString(scope scope, str string) (*ninjaString, error) {
 		r := rune(str[i])
 		state, err = state(parseState, i, r)
 		if err != nil {
-			return nil, fmt.Errorf("error parsing ninja string %q: %s", str, err)
+			return nil, "", fmt.Errorf("error parsing ninja string %q: %s", str, err)
 		}
 	}
 
 	_, err = state(parseState, len(parseState.str), eof)
 	if err != nil {
-		return nil, err
+		return nil, "", err
 	}
 
 	// All the '$' characters counted initially could have been "$$" escapes, leaving no
@@ -131,7 +146,7 @@ func parseNinjaString(scope scope, str string) (*ninjaString, error) {
 		result.variables = nil
 	}
 
-	return result, nil
+	return result, "", nil
 }
 
 func parseFirstRuneState(state *parseState, i int, r rune) (stateFunc, error) {
@@ -246,6 +261,8 @@ func parseBracketsState(state *parseState, i int, r rune) (stateFunc, error) {
 	}
 }
 
+// parseNinjaStrings converts a list of strings to *ninjaStrings by finding the references
+// to ninja variables contained in the strings.
 func parseNinjaStrings(scope scope, strs []string) ([]*ninjaString,
 	error) {
 
@@ -263,6 +280,50 @@ func parseNinjaStrings(scope scope, strs []string) ([]*ninjaString,
 	return result, nil
 }
 
+// parseNinjaOrSimpleStrings splits a list of strings into *ninjaStrings if they have ninja
+// variable references or a list of strings if they don't.  If none of the input strings contain
+// ninja variable references (a very common case) then it returns the unmodified input slice as
+// the output slice.
+func parseNinjaOrSimpleStrings(scope scope, strs []string) ([]*ninjaString, []string, error) {
+	if len(strs) == 0 {
+		return nil, strs, nil
+	}
+
+	// allSimpleStrings is true until the first time a string with ninja variable references is found.
+	allSimpleStrings := true
+	var simpleStrings []string
+	var ninjaStrings []*ninjaString
+
+	for i, str := range strs {
+		ninjaStr, simpleStr, err := parseNinjaOrSimpleString(scope, str)
+		if err != nil {
+			return nil, nil, fmt.Errorf("error parsing element %d: %s", i, err)
+		} else if ninjaStr != nil {
+			ninjaStrings = append(ninjaStrings, ninjaStr)
+			if allSimpleStrings && i > 0 {
+				// If all previous strings had no ninja variable references then they weren't copied into
+				// simpleStrings to avoid allocating it if the input slice is reused as the output.  Allocate
+				// simpleStrings and copy all the previous strings into it.
+				simpleStrings = make([]string, i, len(strs))
+				copy(simpleStrings, strs[:i])
+			}
+			allSimpleStrings = false
+		} else {
+			if !allSimpleStrings {
+				// Only copy into the output slice if at least one string with ninja variable references
+				// was found.  Skipped strings will be copied the first time a string with ninja variable
+				// is found.
+				simpleStrings = append(simpleStrings, simpleStr)
+			}
+		}
+	}
+	if allSimpleStrings {
+		// None of the input strings had ninja variable references, return the input slice as the output.
+		return nil, strs, nil
+	}
+	return ninjaStrings, simpleStrings, nil
+}
+
 func (n *ninjaString) Value(pkgNames map[*packageContext]string) string {
 	if n.variables == nil || len(*n.variables) == 0 {
 		return defaultEscaper.Replace(n.str)

ninja_strings_test.go

@@ -19,6 +19,7 @@ import (
 	"strconv"
 	"strings"
 	"testing"
+	"unsafe"
 )
 
 type testVariableRef struct {
@@ -215,6 +216,105 @@ func TestParseNinjaStringWithImportedVar(t *testing.T) {
 	}
 }
 
+func Test_parseNinjaOrSimpleStrings(t *testing.T) {
+	testCases := []struct {
+		name            string
+		in              []string
+		outStrings      []string
+		outNinjaStrings []string
+		sameSlice       bool
+	}{
+		{
+			name:      "nil",
+			in:        nil,
+			sameSlice: true,
+		},
+		{
+			name:      "empty",
+			in:        []string{},
+			sameSlice: true,
+		},
+		{
+			name:      "string",
+			in:        []string{"abc"},
+			sameSlice: true,
+		},
+		{
+			name:            "ninja string",
+			in:              []string{"$abc"},
+			outStrings:      nil,
+			outNinjaStrings: []string{"${abc}"},
+		},
+		{
+			name:            "ninja string first",
+			in:              []string{"$abc", "def", "ghi"},
+			outStrings:      []string{"def", "ghi"},
+			outNinjaStrings: []string{"${abc}"},
+		},
+		{
+			name:            "ninja string middle",
+			in:              []string{"abc", "$def", "ghi"},
+			outStrings:      []string{"abc", "ghi"},
+			outNinjaStrings: []string{"${def}"},
+		},
+		{
+			name:            "ninja string last",
+			in:              []string{"abc", "def", "$ghi"},
+			outStrings:      []string{"abc", "def"},
+			outNinjaStrings: []string{"${ghi}"},
+		},
+	}
+
+	for _, tt := range testCases {
+		t.Run(tt.name, func(t *testing.T) {
+			inCopy := append([]string(nil), tt.in...)
+
+			scope := newLocalScope(nil, "")
+			scope.AddLocalVariable("abc", "abc")
+			scope.AddLocalVariable("def", "def")
+			scope.AddLocalVariable("ghi", "ghi")
+			gotNinjaStrings, gotStrings, err := parseNinjaOrSimpleStrings(scope, tt.in)
+			if err != nil {
+				t.Errorf("unexpected error %s", err)
+			}
+
+			wantStrings := tt.outStrings
+			if tt.sameSlice {
+				wantStrings = tt.in
+			}
+
+			wantNinjaStrings := tt.outNinjaStrings
+
+			var evaluatedNinjaStrings []string
+			if gotNinjaStrings != nil {
+				evaluatedNinjaStrings = make([]string, 0, len(gotNinjaStrings))
+				for _, ns := range gotNinjaStrings {
+					evaluatedNinjaStrings = append(evaluatedNinjaStrings, ns.Value(nil))
+				}
+			}
+
+			if !reflect.DeepEqual(gotStrings, wantStrings) {
+				t.Errorf("incorrect strings output, want %q got %q", wantStrings, gotStrings)
+			}
+			if !reflect.DeepEqual(evaluatedNinjaStrings, wantNinjaStrings) {
+				t.Errorf("incorrect ninja strings output, want %q got %q", wantNinjaStrings, evaluatedNinjaStrings)
+			}
+			if len(inCopy) != len(tt.in) && (len(tt.in) == 0 || !reflect.DeepEqual(inCopy, tt.in)) {
+				t.Errorf("input modified, want %#v, got %#v", inCopy, tt.in)
+			}
+
+			if (unsafe.SliceData(tt.in) == unsafe.SliceData(gotStrings)) != tt.sameSlice {
+				if tt.sameSlice {
+					t.Errorf("expected input and output slices to have the same backing arrays")
+				} else {
+					t.Errorf("expected input and output slices to have different backing arrays")
+				}
+			}
+
+		})
+	}
+}
+
 func Benchmark_parseNinjaString(b *testing.B) {
 	b.Run("constant", func(b *testing.B) {
 		for _, l := range []int{1, 10, 100, 1000} {

ninja_writer.go

@@ -34,12 +34,12 @@ type StringWriterWriter interface {
 }
 
 type ninjaWriter struct {
-	writer io.StringWriter
+	writer StringWriterWriter
 
 	justDidBlankLine bool // true if the last operation was a BlankLine
 }
 
-func newNinjaWriter(writer io.StringWriter) *ninjaWriter {
+func newNinjaWriter(writer StringWriterWriter) *ninjaWriter {
 	return &ninjaWriter{
 		writer: writer,
 	}
@@ -115,6 +115,8 @@ func (n *ninjaWriter) Rule(name string) error {
 
 func (n *ninjaWriter) Build(comment string, rule string, outputs, implicitOuts,
 	explicitDeps, implicitDeps, orderOnlyDeps, validations []*ninjaString,
+	outputStrings, implicitOutStrings, explicitDepStrings,
+	implicitDepStrings, orderOnlyDepStrings, validationStrings []string,
 	pkgNames map[*packageContext]string) error {
 
 	n.justDidBlankLine = false
@@ -136,14 +138,22 @@ func (n *ninjaWriter) Build(comment string, rule string, outputs, implicitOuts,
 
 	wrapper.WriteString("build")
 
+	for _, output := range outputStrings {
+		wrapper.Space()
+		outputEscaper.WriteString(wrapper, output)
+	}
 	for _, output := range outputs {
 		wrapper.Space()
 		output.ValueWithEscaper(wrapper, pkgNames, outputEscaper)
 	}
 
-	if len(implicitOuts) > 0 {
+	if len(implicitOuts) > 0 || len(implicitOutStrings) > 0 {
 		wrapper.WriteStringWithSpace("|")
 
+		for _, out := range implicitOutStrings {
+			wrapper.Space()
+			outputEscaper.WriteString(wrapper, out)
+		}
 		for _, out := range implicitOuts {
 			wrapper.Space()
 			out.ValueWithEscaper(wrapper, pkgNames, outputEscaper)
@@ -154,32 +164,48 @@ func (n *ninjaWriter) Build(comment string, rule string, outputs, implicitOuts,
 
 	wrapper.WriteStringWithSpace(rule)
 
+	for _, dep := range explicitDepStrings {
+		wrapper.Space()
+		inputEscaper.WriteString(wrapper, dep)
+	}
 	for _, dep := range explicitDeps {
 		wrapper.Space()
 		dep.ValueWithEscaper(wrapper, pkgNames, inputEscaper)
 	}
 
-	if len(implicitDeps) > 0 {
+	if len(implicitDeps) > 0 || len(implicitDepStrings) > 0 {
 		wrapper.WriteStringWithSpace("|")
 
+		for _, dep := range implicitDepStrings {
+			wrapper.Space()
+			inputEscaper.WriteString(wrapper, dep)
+		}
 		for _, dep := range implicitDeps {
 			wrapper.Space()
 			dep.ValueWithEscaper(wrapper, pkgNames, inputEscaper)
 		}
 	}
 
-	if len(orderOnlyDeps) > 0 {
+	if len(orderOnlyDeps) > 0 || len(orderOnlyDepStrings) > 0 {
 		wrapper.WriteStringWithSpace("||")
 
+		for _, dep := range orderOnlyDepStrings {
+			wrapper.Space()
+			inputEscaper.WriteString(wrapper, dep)
+		}
 		for _, dep := range orderOnlyDeps {
 			wrapper.Space()
 			dep.ValueWithEscaper(wrapper, pkgNames, inputEscaper)
 		}
 	}
 
-	if len(validations) > 0 {
+	if len(validations) > 0 || len(validationStrings) > 0 {
 		wrapper.WriteStringWithSpace("|@")
 
+		for _, dep := range validationStrings {
+			wrapper.Space()
+			inputEscaper.WriteString(wrapper, dep)
+		}
 		for _, dep := range validations {
 			wrapper.Space()
 			dep.ValueWithEscaper(wrapper, pkgNames, inputEscaper)
@@ -235,7 +261,7 @@ func (n *ninjaWriter) ScopedAssign(name, value string) error {
 	return nil
 }
 
-func (n *ninjaWriter) Default(pkgNames map[*packageContext]string, targets ...*ninjaString) error {
+func (n *ninjaWriter) Default(pkgNames map[*packageContext]string, targets []*ninjaString, targetStrings []string) error {
 	n.justDidBlankLine = false
 
 	const lineWrapLen = len(" $")
@@ -248,6 +274,10 @@ func (n *ninjaWriter) Default(pkgNames map[*packageContext]string, targets ...*n
 
 	wrapper.WriteString("default")
 
+	for _, target := range targetStrings {
+		wrapper.Space()
+		outputEscaper.WriteString(wrapper, target)
+	}
 	for _, target := range targets {
 		wrapper.Space()
 		target.ValueWithEscaper(wrapper, pkgNames, outputEscaper)
@@ -347,6 +377,7 @@ func (n *ninjaWriterWithWrap) WriteString(s string) (written int, noError error)
 		n.lineLen = indentWidth*2 + n.pendingLen
 		s = strings.TrimLeftFunc(s, unicode.IsSpace)
 		n.pending = append(n.pending, s)
+		n.lineLen += len(s)
 		n.writePending()
 
 		n.space = false
@@ -361,6 +392,11 @@ func (n *ninjaWriterWithWrap) WriteString(s string) (written int, noError error)
 	return
 }
 
+func (n *ninjaWriterWithWrap) Write(p []byte) (written int, noError error) {
+	// Write is rarely used, implement it via WriteString.
+	return n.WriteString(string(p))
+}
+
 // Space inserts a space that is also a possible wrapping point into the string.
 func (n *ninjaWriterWithWrap) Space() {
 	if n.err != nil {

ninja_writer_test.go

@@ -50,12 +50,15 @@ var ninjaWriterTestCases = []struct {
 	},
 	{
 		input: func(w *ninjaWriter) {
-			ck(w.Build("foo comment", "foo", testNinjaStrings("o1", "o2"),
+			ck(w.Build("foo comment", "foo", testNinjaStrings("o3", "o4"),
-				testNinjaStrings("io1", "io2"), testNinjaStrings("e1", "e2"),
+				testNinjaStrings("io3", "io4"), testNinjaStrings("e3", "e4"),
-				testNinjaStrings("i1", "i2"), testNinjaStrings("oo1", "oo2"),
+				testNinjaStrings("i3", "i4"), testNinjaStrings("oo3", "oo4"),
-				testNinjaStrings("v1", "v2"), nil))
+				testNinjaStrings("v3", "v4"), []string{"o1", "o2"},
+				[]string{"io1", "io2"}, []string{"e1", "e2"},
+				[]string{"i1", "i2"}, []string{"oo1", "oo2"},
+				[]string{"v1", "v2"}, nil))
 		},
-		output: "# foo comment\nbuild o1 o2 | io1 io2: foo e1 e2 | i1 i2 || oo1 oo2 |@ v1 v2\n",
+		output: "# foo comment\nbuild o1 o2 o3 o4 | io1 io2 io3 io4: foo e1 e2 e3 e4 | i1 i2 i3 i4 || oo1 oo2 $\n        oo3 oo4 |@ v1 v2 v3 v4\n",
 	},
 	{
 		input: func(w *ninjaWriter) {
@@ -63,15 +66,15 @@ var ninjaWriterTestCases = []struct {
 				testNinjaStrings(strings.Repeat("o", lineWidth)),
 				nil,
 				testNinjaStrings(strings.Repeat("i", lineWidth)),
-				nil, nil, nil, nil))
+				nil, nil, nil, nil, nil, nil, nil, nil, nil, nil))
 		},
-		output: "# foo comment\nbuild $\n        " + strings.Repeat("o", lineWidth) + ": foo $\n        " + strings.Repeat("i", lineWidth) + "\n",
+		output: "# foo comment\nbuild $\n        " + strings.Repeat("o", lineWidth) + ": $\n        foo $\n        " + strings.Repeat("i", lineWidth) + "\n",
 	},
 	{
 		input: func(w *ninjaWriter) {
-			ck(w.Default(nil, testNinjaStrings("foo")...))
+			ck(w.Default(nil, testNinjaStrings("foo"), []string{"bar"}))
 		},
-		output: "default foo\n",
+		output: "default bar foo\n",
 	},
 	{
 		input: func(w *ninjaWriter) {
@@ -108,7 +111,8 @@ var ninjaWriterTestCases = []struct {
 			ck(w.ScopedAssign("pool", "p"))
 			ck(w.BlankLine())
 			ck(w.Build("r comment", "r", testNinjaStrings("foo.o"),
-				nil, testNinjaStrings("foo.in"), nil, nil, nil, nil))
+				nil, testNinjaStrings("foo.in"), nil, nil, nil, nil,
+				nil, nil, nil, nil, nil, nil))
 			ck(w.ScopedAssign("_arg", "arg value"))
 		},
 		output: `pool p