From c5b3c0ca9e04981badb9bc10eea970c11df47a9e Mon Sep 17 00:00:00 2001 From: Colin Cross Date: Tue, 7 May 2024 13:49:51 -0700 Subject: [PATCH] Don't sort the results of TransitionMutator.Split The ordering of variants can be significant when adding inter-variant dependencies, as a variant can only depend on earlier variants. Allow maintaining the existing variant ordering when converting mutators to TransitionMutator by keeping the ordering of TransitionMutator.Split. Variations that were requested by incoming dependencies that are not present in Split are still sorted, as they have no inherent ordering. Bug: 319288033 Test: TestPostTransitionDeps Flag: NONE Change-Id: I648ef95f08a05f9a64ce97e6a39bae10ce88771a --- transition.go | 5 ++++- transition_test.go | 6 +++--- 2 files changed, 7 insertions(+), 4 deletions(-) diff --git a/transition.go b/transition.go index b5a46c2..2867c35 100644 --- a/transition.go +++ b/transition.go @@ -186,8 +186,11 @@ func (t *transitionMutatorImpl) topDownMutator(mctx TopDownMutatorContext) { // and modules that directly depend on it. Since this is a top-down mutator, // all modules that directly depend on this module have already been processed // so no locking is necessary. - module.transitionVariations = addToStringListIfNotPresent(module.transitionVariations, mutatorSplits...) + // Sort the module transitions, but keep the mutatorSplits in the order returned + // by Split, as the order can be significant when inter-variant dependencies are + // used. sort.Strings(module.transitionVariations) + module.transitionVariations = addToStringListIfNotPresent(mutatorSplits, module.transitionVariations...) outgoingTransitionCache := make([][]string, len(module.transitionVariations)) for srcVariationIndex, srcVariation := range module.transitionVariations { diff --git a/transition_test.go b/transition_test.go index 3e07b53..a380b86 100644 --- a/transition_test.go +++ b/transition_test.go @@ -60,7 +60,7 @@ const testTransitionBp = ` transition_module { name: "A", deps: ["B", "C"], - split: ["a", "b"], + split: ["b", "a"], } transition_module { @@ -132,7 +132,7 @@ func TestTransition(t *testing.T) { assertNoErrors(t, errs) // Module A uses Split to create a and b variants - checkTransitionVariants(t, ctx, "A", []string{"a", "b"}) + checkTransitionVariants(t, ctx, "A", []string{"b", "a"}) // Module B inherits a and b variants from A checkTransitionVariants(t, ctx, "B", []string{"", "a", "b"}) // Module C inherits a and b variants from A, but gets an outgoing c variant from B @@ -184,7 +184,7 @@ func TestPostTransitionDeps(t *testing.T) { assertNoErrors(t, errs) // Module A uses Split to create a and b variants - checkTransitionVariants(t, ctx, "A", []string{"a", "b"}) + checkTransitionVariants(t, ctx, "A", []string{"b", "a"}) // Module B inherits a and b variants from A checkTransitionVariants(t, ctx, "B", []string{"", "a", "b"}) // Module C inherits a and b variants from A, but gets an outgoing c variant from B