In order to do less cloning, refactor selects so that all the
soong-visibile structs are immutable to soong and can be reused.
Additionally, refactor how the inner linked list of selects is managed,
so that the append/prepend/replace logic is simpler.
Bug: 323382414
Test: m nothing --no-skip-soong-tests
Change-Id: Iba5d27405decc1b0596590c3e0555daeb044bf9e
The biggest issue here is that Configurable objects needed to be
cloned before appended to one another, otherwise some Configurables
that were used in defaults could be edited after being applied to one
module and apply a different value to another module.
Also fix an issue where a select without a defined appendWrapper
always evaluated to nil.
I plan to make a followup refactor cl to make these things clearer,
but start with the bugfix.
Bug: 323382414
Test: m nothing --no-skip-soong-tests (see other cl in topic for tests)
Change-Id: Icf68d0ee1779c76bfb3d68db43b35d7e09bc0dd9
Sometimes modules add arch-variant properties in load hooks, to disable
modules by default on certain platforms for example. When changing the
property to a Configurable property, these load hooks would also need
to be changed in order to have a matching type for
ExtendMatchingProperties.
Since this can be kindof a pain to address everywhere, for now,
special case the extension functions to promote non-configurable
properties to configurable ones. We can remove this later when
everything switches to configurable properties.
Bug: 323382414
Test: go tests
Change-Id: Iac96587dbd60ccdd6aa667dd69a71ad252abe589
Some soong code sets arch-variant properties in order to control a
module's default behavior. I'll make this continue to work, but long
term the arch-variant properties should be replaced with selects,
so expose an API for creating select statements in soong code.
Bug: 323382414
Test: m nothing --no-skip-soong-tests
Change-Id: I6c65d6e112b6f826f1027777b6fdf36915d34b1d
Adding a dependency on a module that has already had a TransitionMutator
run on it may require adjusting the variation name based on the results
of IncomingTranstion. Store the variants that existed before the
TransitionMutator ran, find one that is a subset of the requested
variant, and call TranstionMutator.IncomingTransition to update the
value.
Bug: 319288033
Test: TestPostTransitionDeps
Change-Id: I690357f9792401a3edbc5ae9fdcb666495954fbc
This adds support for selecting on multiple variables at once, so that
you can do AND/OR combindations of them. For example:
select((
arch(),
os(),
), {
("arm64", "linux"): ["libfoo64"],
(default, "linux"): ["libfoo"],
(default, "windows"): ["libfoowindows"],
(default, default): ["libbar"],
})
It also allows for select conditions to be boolean-typed. You can
write literal true and false without quotes to select on them. Currently
we don't have any boolean-typed variables though, so a fake one was
added for testing.
Bug: 323382414
Test: m nothing --no-skip-soong-tests
Change-Id: Ibe586e7b21865b8734027848cc421594cbd1d8cc
Part of the design of property structs is that they were easy to access.
In keeping with that spirit, use a shorter and easier to spell name
for the getter, and add GetDefault() so that you don't need to pass
the result of Get() to one of the
proptools.StringDefault/BoolDefault/etc functions.
Bug: 323382414
Test: m nothing --no-skip-soong-tests
Change-Id: Ib9a69dcf2ab56a758935a461f37fe46bc0e17e27
If a property is a pointer to a bool/string, that property would be
replaced instead of appended to when calling ExtendProperties().
This is confusing behavior, and I don't want to give anyone any reason
to use a pointer to a configurable property, so add this relflection
tag to recreate the functionality.
Bug: 323382414
Test: m nothing --no-skip-soong-tests
Change-Id: I6b9e4dfee1d2685e0c9dc6a646cf45724cc04756
Previously I was using an underscore to denote the default branch
because I was thinking that I would allow variable bindings in the
select branches, and 'default' could be mistaken for the name of a
variable. But I think it's better to just introduce alternate syntax,
like `default @ my_var: "foo" + my_var,` to do the variable bindings,
so that we can have a clearer name for the default case.
Bug: 323382414
Test: m nothing --no-skip-soong-tests
Change-Id: Ied762694e453855c03dd471898ebb52e97a5a671
Currently, with the arch/os mutator, you can override a property
using the default value for just a few arch types, for example:
cc_defaults {
name: "my_defaults",
target: {
windows: {
enabled: true,
}
}
}
cc_binary {
name: "foo",
enabled: false,
defaults: ["my_defaults"],
}
You could make a select statment that acts like the above if it were
all in one module, but currently with select statements you can't make
a defaults module that can be generically applied to any other module
and have the same behavior as the above.
After this cl, the defaults module could look like:
cc_defaults {
name: "my_defaults",
enabled: select(variant("arch"), {
"windows": true,
_: unset,
}),
}
Which would have the same behavior. Unset may also be useful for
setting the property under some configurations, but wanting to leave
the implementation-specific default value in others.
Bug: 323382414
Test: m nothing --no-skip-soong-tests
Change-Id: I3ea3277ea8b9a0ac5e613b4378945388b9df036a
* changes:
Store mutator in mutatorContext instead of name
Explicitly delete old logicModule entries from Context.moduleInfo
Keep logicModule for obsolete variants
Add Provider to transition contexts
Cache outgoing transitions
Use maps and slices packages
TransitionMutators will need access to the original *mutatorInfo,
store it in mutatorContext instead of the name, and update all
accesses to the name to go through the *mutatorInfo.
Bug: 319288033
Test: go test ./...
Change-Id: I4bb1a17f44e55b23dd70708c9a5fde2ae80ce154
Creating variants has an optimization to reuse the logicModule of the
original variant as the logicModule of the first new variant. Using
the same logicModule meant that updating the Context.moduleInfo map
from logicModules to *moduleInfo would overwrite the old entry for
the original variant with the new entry for the first variant.
TransitionMutators will need to keep the original logicModule for
later use, which will require disabling the optimization that reuses
it. When the first variant is added to the map it no longer overwrites
the original variant. Excplicitly track the original logicModule and
remove it from the map if necessary.
Bug: 319288033
Test: go test ./...
Change-Id: I05ffdf6d7ce60508f6d9e9657c21032f2c4f5d9c
When splitting a module into variants the original *moduleInfo was
left in place but with logicModule set to nil as a marker that the
variant was obsolete and any incoming dependencies needed to be resolved
onto one of the newly split variants. A change to allow TransitionMutator
IncomingTransition calls when adding dependencies later will require
keeping the obsolete *moduleInfo and logicModule around so that
IncomingTransition can be called on it. so use an explicit boolean
to mark the module as obsolete instead of clearing logicModule.
Bug: 319288033
Test: go test ./...
Change-Id: I5dc201f43442aa07ac1b858240c675bab3782b55
Allow TransitionMutator OutgoingTransition and IncomingTransition
methods to access Providers from the current and dependency module
respectively.
Bug: 319288033
Test: none
Change-Id: If16ee7980ee0b8f4abaf3c112738fca3f13ab61c
TransitionMutators were calling OutgoingTransition and IncomingTransition
twice, once to determine all the necessary variations for each module,
and then again when creating the variations to resolve dependencies.
Store the final variation needed for each dependency during the first
computation, and use it to resolve the dependency variation in the second
pass.
Bug: 319288033
Test: all soong tests
Change-Id: I2e42c0d69efbfcff915267c484c18554b857e0b6
This is to match how (non-pointer) booleans work in AppendProperties().
Bug: 323382414
Test: m nothing --no-skip-soong-tests
Change-Id: I16791fc5ac684eedf8064a65953b8b68f18c37c1
filterPropertyStructFields should treat configurable properties as
leaf nodes and not recurse into them. Configurable properties can be
made arch_variant, at least while we transition to full use of
Configurable.
Bug: 323382414
Test: soong tests when changing enabled to be configurable
Change-Id: Ib22dd3797f69e912dab0e0c3c1030d6c1f04db7e
...and pass property name to ConfigurableEvaluator.
When trying to convert Enabled to a configurable property, I enountered
a bunch of different context types that all had different error methods.
OtherModulePropertyErrorf is an error method that can be implemented
by all contexts.
Bug: 323382414
Test: m nothing --no-skip-soong-tests
Change-Id: I38b2a545c0ee8d23281cd88bc397f79f39835bc4
ctx.ModuleErrorf for the main blueprint config object doesn't
report an error, it just returns it.
If go had something like C++'s `[[nodiscard]]` or rust's `#[must_use]`
that would be useful here, but the issue has been open since 2017:
https://github.com/golang/go/issues/20803
Test: Presubmits
Change-Id: I72709e6c5466d55f5c0f3fe51a25c29df0826271
Bug: 323382414
Test: go tests, and I also ran both the old/new bpfmts on the whole aosp source tree, and the only difference between the results was one extra removed line in external/uwb
Change-Id: I4942c9247a66f1de5028de39caa5cd34b66093c3
To make it easier to handle the result of
Configurable[[]string].Evaluate()
Bug: 329711542
Test: go tests
Change-Id: I7364170564b1049a33873424c6da4fc26aff305b
Select statements are a new blueprint feature inspired by bazel's select
statements. They are essentially alternative syntax for soong config
variables that require less boilerplate. In addition, they support
making decisions based on a module's variant, which will eliminate
the need for manual property struct manipulation, such as the arch
mutator's arch: and target: properties.
In order to support decisions based on the variant, select statements
cannot be evaluated as soon as they're parsed. Instead, they must be
stored in the property struct unevaluated. This means that individual
properties need to change their type from say, string, to
Configurable[string]. Currently, only configurable strings, bools, and
string slices are supported, but more types can be added later.
The module implementation must call my_property.Evaluate(ctx) in order
to get the final, resolved value of the select statement.
Bug: 323382414
Test: go tests
Change-Id: I62f8721d7f0ac3d1df4a06d7eaa260a5aa7fcba3
It's not super useful because of Go's stupid restriction on not letting
you reflect private fields.
Test: m nothing
Change-Id: I1f73f06f5e64eaf4adce58a667b98b131aede53f
The property squashing functions were performing 19% of all allocations
and allocating 3.43 GB, most of which was used to track the name of the
property, but the name of the property is only used to print errors.
Keep the elements of the name in a preallocated slice instead, and only
compute the name when an error occurs.
Calling reflect.Value.Interface() was also expensive, and only passed
to filter and order functions, none of which use the values. Modify
the signature of the filter and order functions to remove the interfaces
and the property name.
Test: extend_test.go
Change-Id: I517f89daf251bb43f7cfefa6f1e83951c0e271b7
blueprint.variationMap.Equal was responsible for 11.5% of allocations
and 2.2% of allocated memory. reflect.DeepEquals is an expensive way
to compare a map. variationMap.subsetOf is already iterating through
the elements of the map to compare them, replace equal with a check that
the maps are the same length and then reuse subsetOf to check that the
have the same keys and values.
Test: SOONG_PROFILE_MEM=/tmp/mem.pprof m nothing
Change-Id: Ifb7cdf612e5455fd2f412488b7f94416c4e70c54