aboutsummaryrefslogtreecommitdiffhomepage
path: root/src/main/java/com/google/devtools/build/lib/skyframe/ConfiguredTargetFunction.java
Commit message (Collapse)AuthorAge
* Do not report errors when aspects try to attach to files.Gravatar dslomov2017-05-08
| | | | | | | | | | | Instead, silently ignore them in the same way we do for rules to which aspects are not applicable. In the future aspects will gain the ability to apply to, and propagate through, files. RELNOTES: None. PiperOrigin-RevId: 155369925
* Implement dynamically configured LIPO builds.Gravatar gregce2017-05-04
| | | | | | | | | | | | | | | Quick overview: - provide a dynamic interface for getting the artifact owner configuration - provide a (dynamic) RuleTransitionFactory LIPO_ON_DEMAND to replace the (static) RuleClass.Configurator LIPO_ON_DEMAND. Eventually we'll remove the rule class configurator interface entirely. This doesn't actually turn dynamic LIPO on. So the direct effect of this change should be a no-op. The flip will come in a followup change. For now, dynamic LIPO can be triggered with --experimental_dynamic_configs=notrim. PiperOrigin-RevId: 155096056
* Automated g4 rollback of commit d5ee3b5397135eebd4b5d5b6bd4a4444093c4df8.Gravatar kchodorow2017-04-26
| | | | | | | | | | | | | | | | | | | | *** Reason for rollback *** The go rules have tons of transitive dependencies that are not declared in their "local" WORKSPACE files, so this broke lots of projects on ci.bazel.build I'll fix up the go rules, update all of _their_ reverse-dep projects, and resubmit. *** Original change description *** Repositories can only be accessed in projects that define them in their WORKSPACE file This is prep for #1943 - hierarchical workspace loading. RELNOTES[INC]: Remote repositories must define any remote repositories they themselves use (e.g., if @x//:foo depends on @y//:bar, @y must be defined in @x's WORKSPACE file). PiperOrigin-RevId: 154321845
* Repositories can only be accessed in projects that define them in their ↵Gravatar kchodorow2017-04-26
| | | | | | | | | | | | WORKSPACE file This is prep for #1943 - hierarchical workspace loading. RELNOTES[INC]: Remote repositories must define any remote repositories they themselves use (e.g., if @x//:foo depends on @y//:bar, @y must be defined in @x's WORKSPACE file). PiperOrigin-RevId: 154295762
* If --batch, --keep_going, --discard_analysis_cache, and the new ↵Gravatar janakr2017-03-31
| | | | | | | | | | | | | | | --noexperimental_enable_critical_path_profiling flags are all specified, then Bazel will delete Actions from ActionLookupValues as they are executed in order to save memory. Because an already-run action may output an artifact that is only requested later in the build, we need to maintain a way for the artifact to look up the action. But in most cases we don't need to keep the action itself, just its output metadata. Some actions unfortunately are needed post-execution, and so we special-case them. Also includes dependency change with description: Move action out of key. This keeps action references from polluting the graph -- actions are just stored in one SkyValue, instead of being present in SkyKeys. This does mean additional memory used: we have a separate ActionLookupData object per Action executed. That may reach ~24M for million-action builds. PiperOrigin-RevId: 151756383
* Do not execute aspect implementation if target advertizes but fails to ↵Gravatar Dmitry Lomov2017-03-01
| | | | | | | | | | | provide a provider. Previously we always executed the function, but didn't add the aspect to the deps. -- PiperOrigin-RevId: 148887089 MOS_MIGRATED_REVID=148887089
* Report inconsistent aspect order error to the user.Gravatar Dmitry Lomov2017-02-24
| | | | | | -- PiperOrigin-RevId: 148342788 MOS_MIGRATED_REVID=148342788
* Restrict aspects visible to other aspects according to their advertised ↵Gravatar Dmitry Lomov2017-02-15
| | | | | | | | providers. -- PiperOrigin-RevId: 147526961 MOS_MIGRATED_REVID=147526961
* Encapsulate the required provider logic in RequiredProviders class.Gravatar Dmitry Lomov2017-01-10
| | | | | | | | | For now, only for aspects, but eventually expand to Attribute's mandatory providers as well. -- PiperOrigin-RevId: 144063369 MOS_MIGRATED_REVID=144063369
* Names of extra-action protos now take into account all aspect names.Gravatar Dmitry Lomov2016-12-16
| | | | | | | | | | | | | If an Aspect registered an action that an extra-action is shadowing, its name is used when creating the extra-action's ID and name. Since recently, an aspect can see other aspects applied to the same target. This CL record the names of other aspects applied to the target as well, disambiguating the action owners. -- PiperOrigin-RevId: 142264153 MOS_MIGRATED_REVID=142264153
* Fix an analysis phase performance regression with dynamic configurations.Gravatar Greg Estren2016-12-14
| | | | | | | | | | | | | | | | | | | | | | | | | | | | The short story is that env.valuesMissing() returns true without regard for which Skyframe evaluation missed values. In other words, given: env.getValues(missingValues); // Not all values ready. env.getValues(presentValues); // All value ready. if (env.valuesMissing()) { return null; } this returns null even if "env.getValues(presentValues)" has all its results. This doesn't have correctness consequences, but it does have (major) performance consequences, particularly in ConfiguredTargetFunction. The quick reason is that the successful call can still do useful followup work, but that gets short-circuited if the function nulls out early. See the code changes for full details. This eliminates a 30% hit on analysis time with dynamic configurations. With this change, that goes down to 0. The takeaway: ConfiguredTargetFunction is both unintuitively complex and performance-critical. C'est la vie. -- PiperOrigin-RevId: 142044069 MOS_MIGRATED_REVID=142044069
* Provide deterministic order for split configured deps (roll forward part 2).Gravatar Greg Estren2016-12-07
| | | | | | | | | | | | | | | | | | | | | Also: - Make ConfiguredTargetFunction.getDynamicConfigurations more readable. - Add a bit more testing coverage for configured dep resolution. This is a roll forward of commit 7505d94c19727e3100ac5e16a960bff2cb324f23. The original changed failed for two reasons: 1) Windows-only: "ppc" wasn't recognized as a valid cpu: https://github.com/bazelbuild/bazel/issues/2191 2) Bazel requires android_binary's "manifest" attribute to be "AndroidManifest.xml": https://www.google.com/url?sa=D&q=http%3A%2F%2Fci.bazel.io%2Fjob%2Fbazel-tests%2FBAZEL_VERSION%3DHEAD%2CPLATFORM_NAME%3Dubuntu_15.10-x86_64%2FlastCompletedBuild%2FtestReport%2F This version uses "armeabi-v7a" instead of "ppc" and "AndroidManifest.xml" in the splitDeps test. -- PiperOrigin-RevId: 141313454 MOS_MIGRATED_REVID=141313454
* Rollback of commit 12d766df10fbc5eba16ec1e6c20c8cd85f9c616f.Gravatar Damien Martin-Guillerez2016-12-06
| | | | | | | | | | | | | | | | | | | | | | | | | | | *** Reason for rollback *** Still fails bazel-tests See http://ci.bazel.io/job/bazel-tests/BAZEL_VERSION=HEAD,PLATFORM_NAME=ubuntu_15.10-x86_64/lastCompletedBuild/testReport/ for instance *** Original change description *** Provide deterministic order for split configured deps (roll forward) Also: - Make ConfiguredTargetFunction.getDynamicConfigurations more readable. - Add a bit more testing coverage for configured dep resolution. This is a roll forward of commit 7505d94c19727e3100ac5e16a960bff2cb324f23. The original changed failed on Windows because "ppc" wasn't recognized as a valid cpu: https://github.com/bazelbuild/bazel/issues/2191 This version uses "armeabi-v7a" instead. -- PiperOrigin-RevId: 141212457 MOS_MIGRATED_REVID=141212457
* Provide deterministic order for split configured deps (roll forward)Gravatar Greg Estren2016-12-06
| | | | | | | | | | | | | | | | | Also: - Make ConfiguredTargetFunction.getDynamicConfigurations more readable. - Add a bit more testing coverage for configured dep resolution. This is a roll forward of commit 7505d94c19727e3100ac5e16a960bff2cb324f23. The original changed failed on Windows because "ppc" wasn't recognized as a valid cpu: https://github.com/bazelbuild/bazel/issues/2191 This version uses "armeabi-v7a" instead. -- PiperOrigin-RevId: 141185293 MOS_MIGRATED_REVID=141185293
* Rollback of commit 7505d94c19727e3100ac5e16a960bff2cb324f23.Gravatar Damien Martin-Guillerez2016-12-06
| | | | | | | | | | | | | | | | | | | | *** Reason for rollback *** Newly added test fail on Windows platform Fixes https://github.com/bazelbuild/bazel/issues/2191 *** Original change description *** Provide deterministic order for split configured deps. Also: - Make ConfiguredTargetFunction.getDynamicConfigurations more readable. - Add a bit more testing coverage for configured dep resolution. -- PiperOrigin-RevId: 141167110 MOS_MIGRATED_REVID=141167110
* Provide deterministic order for split configured deps.Gravatar Greg Estren2016-12-06
| | | | | | | | | | Also: - Make ConfiguredTargetFunction.getDynamicConfigurations more readable. - Add a bit more testing coverage for configured dep resolution. -- PiperOrigin-RevId: 141095973 MOS_MIGRATED_REVID=141095973
* Allow aspects to specify multiple sets of required providers to match ↵Gravatar Rumou Duan2016-12-01
| | | | | | | against rules. Aspects can attach to a rule if at least one set of required providers are present on the rule. -- MOS_MIGRATED_REVID=140605023
* Run the analysis phase with as many threads as the user wants. In order to ↵Gravatar Janak Ramakrishnan2016-11-18
| | | | | | | | | avoid memory blow-up intra-configured-target analysis, use a semaphore to ensure that CPU-bound work only occurs on #CPU-many threads. RELNOTES: Use --loading_phase_threads to control the number of threads used during the loading/analysis phase. -- MOS_MIGRATED_REVID=139477645
* Fixes incomplete support for dynamic split transitions in Bazel'sGravatar Greg Estren2016-11-17
| | | | | | | | | | | | | | | | | | | | | | test infrastructure. The small picture story is that SkyframeExecutor.getDynamicConfigOptions (which gets dynamic BuildOptions for tests) wasn't updated with dynamic split support when that was added to ConfiguredTargetFunction.getDynamicTransitionOptions (which does the same thing for production builds). This change plugs that gap. See 373e3e28274cca5b87f48abe33884edb84016dd3 for the original change. The bigger picture story is that Bazel's configured target creation logic is forked: test code goes down a similar but sadly not-quite-the-same path that results in tons of duplicated logic, spaghetti code mess, and risk of bugs like this one. We'd like to ultimately undo that fork. But unfortunately it's an involved effort that won't happen overnight. In the meantime, this change takes one small step by merging the two methods that caused this bug. -- MOS_MIGRATED_REVID=139342710
* Aspects-on-aspects implementation.Gravatar Dmitry Lomov2016-11-15
| | | | | -- MOS_MIGRATED_REVID=139189444
* Do not crash when aspects provide duplicate things.Gravatar Dmitry Lomov2016-11-11
| | | | | -- MOS_MIGRATED_REVID=138860974
* Lazily evaluate hash codes for ConfiguredTargetFunction#AttributeAndLabel.Gravatar Greg Estren2016-09-30
| | | | | | | | | | Profiling shows this shaves off 8% of getDynamicConfigurations' CPU time. And brings down AttributeAndLabel instantiation from 30% of that time to 24%. Over a simple cc_binary, this reduces the number of Objects.hash calls by 96%. -- MOS_MIGRATED_REVID=134687748
* ConfiguredTargetFunction#getDynamicConfigurations: halve the number of ↵Gravatar Greg Estren2016-09-30
| | | | | | | | | | | | | | | | AttributeAndLabel instantiations. This shaves 25% off the method's execution time in --experimental_dynamic_configs=notrim mode. This isn't a crucial optimization, since getDynamicConfigurations is already fast. Profiling: $ bazel shutdown; bazel build --nobuild //testapp:cc --experimental_dynamic_configs=notrim shows getDynamicConfigurations takes 1.3% of the analysis phase's CPU time before this change and 1.0% after. Before this change, AttributeAndLabel instantiation took 44% of the method's CPU time. After: 30%. Profiling done with JProfiler. -- MOS_MIGRATED_REVID=134677107
* Optimize how null configurations get created and add test infrastructure for ↵Gravatar Greg Estren2016-09-29
| | | | | | | | | | | | Bazel's dep configuration creation logic. This essentially implements the following TODOs: https://github.com/bazelbuild/bazel/blob/bc6045dcc8fa33d4241d231138020ac4bdecc14f/src/main/java/com/google/devtools/build/lib/skyframe/ConfiguredTargetFunction.java#L599 https://github.com/bazelbuild/bazel/blob/bc6045dcc8fa33d4241d231138020ac4bdecc14f/src/test/java/com/google/devtools/build/lib/skyframe/ConfigurationsForTargetsTest.java#L42 -- MOS_MIGRATED_REVID=134607049
* Eliminates performance overhead of --experimental_dynamic_configs=notrim.Gravatar Greg Estren2016-09-28
| | | | | | | | | | | | | | Before this change, a non-trivial real-world build was measured to have about 20% analysis time overhead. After, that's theoretically down to 2.4%. But that's only the analysis phase, so the impact on full builds is smaller. And the impact on analysis-cached builds is zero. And practical tests show no obvious difference (JProfiler is probably overstating the impact since it excludes known heavyweight methods). The improvements, in short: - Optimize a sanity check that expects each <Attribute, Label> pair to only have one transition. This alone was over half the original performance penalty. - Simplify logic for null configuration deps (of which there are many, e.g.: all source files) - Skip a check for required fragments not available in the configuration. This is irrelevant for notrim mode. There are still some places we could optimize. Dependency.withNullConfiguration in particular takes measurable time (I think from being constructed all the time and in its hashCode calls). But this doesn't seem pressing given the new numbers. -- MOS_MIGRATED_REVID=134533452
* Don't call TransitiveTargetFunction for ↵Gravatar Greg Estren2016-09-26
| | | | | | | | | | | --experimental_dynamic_configs=notrim mode (since the whole point of calling it is to figure out which fragments to trim). This shaves a 25% analysis time penalty over static configs down to 19%. -- MOS_MIGRATED_REVID=134130771
* Split dynamic configurations mode into:Gravatar Greg Estren2016-09-22
| | | | | | | | | | | | | | | --experimental_dynamic_configs=off - don't use dynamic configs --experimental_dynamic_configs=on - use dynamic configs with trimmed fragments --experimental_dynamic_configs=notrim - use dynamic configs with all fragments This lets us decouple two independent dimensions of dynamic configurations: 1) being able to trigger new configurations and transitions anywhere and 2) only including the fragments needed by a target's transitive closure. 2) is likely to take much more time and effort to properly finesse (three notable challenges: late-bound attributes, aspects, and dynamic shedding of output path names). But 1) by itself already yields significant benefits. So in the name of starting to shift the config work from backend theory to stuff real builds actually use, this change lets us focus on productionizing 1) without blocking on getting all of 2) working first. tl;dr: iterable deployment and all that. -- MOS_MIGRATED_REVID=133874661
* Allow Skyframe graph lookups and value retrievals to throw InterruptedException.Gravatar Janak Ramakrishnan2016-08-16
| | | | | | | The only place we now don't handle InterruptedException is in the action graph created after analysis, since I'm not sure that will be around for that much longer. -- MOS_MIGRATED_REVID=130327770
* Changes DependencyResolver <Attribute, Dep> map from a ListMultimap to new classGravatar Greg Estren2016-08-12
| | | | | | | | | | | | | | | | | | | | | | OrderedSetMultimap. This maintains insertion order while eliminating duplicates. Certain rules, in particular, otherwise break this invariant: https://github.com/bazelbuild/bazel/blo[]e3e28274cca5b87f48abe33884edb84016dd3/src/main/java/com/google/devtools/build/lib/skyframe/ConfiguredTargetFunction.java#L403 There's no reason (to my knowledge) to need multiple instances of the same <Attribute, Dependency> pair. More context from Google code review: (Michael Staib): > There are many things which pass around a dependentNodeMap or help construct one or modify one. We want an interface which has the right guarantees. > ListMultimap is not the right interface because it has no guarantee of unique elements, which we want - we don't want the problem that this CL ran into, and there's no reason (that we know of, to be confirmed) that anyone would want multiple identical Dependencies. > SetMultimap is not the right interface because it has no guarantee of deterministic iteration order or efficient iteration, which we want - dependency order sometimes matters (e.g., Java classpath or C++ link order). > We agreed that the best way to get what we want is to define our own interface with its own simultaneous uniqueness and iterability guarantees. Unspoken in the discussion was why we wouldn't just use LinkedHashMultimap as the thing we pass around. IMO the reason for that is that we don't care that it be a LinkedHashMultimap specifically; if tomorrow Guava comes out with a faster cooler map that has deterministic and efficient iteration and guarantees element uniqueness, we want it. > In this case we're going to make the "interface" be a (final?) class: OrderedSetMultimap, an extension of ForwardingSetMultimap which delegates to LinkedHashMultimap, an implementation which does support both of those guarantees. > I had mentioned in the conversation that none of the Multimap implementations make guarantees about key iteration order, but this is not true - LinkedHashMultimap preserves key insertion order. We should perhaps declare this as part of the OrderedSetMultimap contract as well. -- MOS_MIGRATED_REVID=130037643
* Implements dynamic split transitions on latebound attributes.Gravatar Greg Estren2016-08-10
| | | | | | | | | With this change, dynamic configs are at full feature parity for split transitions (minus some small differences in composed transitions from BuildConfigurationCollection.configurationHook). -- MOS_MIGRATED_REVID=129802414
* Implements dynamic split transitions (minus latebound attribute splits).Gravatar Greg Estren2016-08-08
| | | | | | | | | | With the prereq work behind this, this is surprisingly straightforward. The main change is to eliminate BuildConfiguration.SplittableTransitionApplier, make both DynamicTransitionApplier and StaticTransitionApplier split-aware, and add awareness of this to ConfiguredTargetFunction.trimConfigurations. Latebound splits will follow next. -- MOS_MIGRATED_REVID=129480309
* Late-bound split attribute configs weren't being properly propagated to deps ↵Gravatar Greg Estren2016-08-04
| | | | | | | | | | | | | | | | with dynamic configs. The old code incorrectly applied a no-op instead of hard-setting a specific config. Testing: This is a prereq piece for the main change adding dynamic split transitions. Once we have that change, standard Bazel tests over implemented late-bound split attributes (e.g. AppleBinaryRule: ":cc_toolchain") will provide proper coverage. There's no easy way to test this directly since the affected code won't really work until the dynamic split change is in. -- MOS_MIGRATED_REVID=129278253
* Provides a clearer message when target analysis fails because its dynamicGravatar Greg Estren2016-08-03
| | | | | | | | | | | | | | | config is missing required fragments. Before this change, Bazel crashes with the mysterious error: "Fragment foo can't load missing options BarOptions" with no details on which target or dep needed Foo. So figuring out the source of the error is painful. With this change, we instead get: //foo:foo: dependency //bar:bar from attribute "deps" is missing required config fragments: JavaConfiguration -- MOS_MIGRATED_REVID=129143764
* Clean up DependencyResolver's interface for the dynamic config migration and ↵Gravatar Greg Estren2016-07-28
| | | | | | | | | | | | | | | | | | | | for general readability. Major changes: - Remove the intermediate Attribute -> LabelAndConfiguration multimap (computed in resolveAttributes). Instead, feed discovered values directly into the final Attribute -> Dependency map via a new RuleResolver interface. - Remove all references to LabelAndConfiguration. The configuration is always the owning rule's configuration except for two special cases: late-bound attributes with splits and late-bound attributes with LateBoundDefault.useHostConfiguration. The original interface made this very unclear and required a lot of awkward and sometimes incorrect logic. The new interface only involves configurations for the cases that actually need them. - Remove an ugly hack caused by BuildConfiguration.evaluateTransition mixing poorly with LateBoundDefault.useHostConfiguration (https://github.com/bazelbuild/bazel/blo[]e172693c27f3efc95ed163e43a9f0a7a6fb4017/src/main/java/com/google/devtools/build/lib/analysis/DependencyResolver.java#L488). - Remove a hack that applies split transitions twice because of BuildConfiguration.evaluateTransition mixing poorly with late-bound split attributes (https://github.com/bazelbuild/bazel/blo[]e172693c27f3efc95ed163e43a9f0a7a6fb4017/src/main/java/com/google/devtools/build/lib/analysis/DependencyResolver.java#L319). This happens to be innocent now but won't be when nested splits are possible. - Solidifies the API contract for Attribute.LateBoundDefault.useHostConfiguration. - Applies clearer naming and more consistent ordering to method parameters. - Better documentation. This is all also prep work for dynamic split transitions. tl;dr: late-bound attributes are legitimately special. Treat them that way to make the rest of DependencyResolver cleaner and hack-free. -- MOS_MIGRATED_REVID=128582618
* Refactor cycle detection logic to handle dynamic configurations.Gravatar Greg Estren2016-06-09
| | | | | | | | | | | | | | | | | | | | | | | Currently, analysis-time cycle detection expects all cycles to come from ConfiguredTargetFunction. With dynamic configurations, ConfiguredTargetFunction calls out to TransitiveTargetFunction to figure out which configuration fragments its deps need. If there's a cycle between the current target and a dep, the dep's TransitiveTargetFunction fails, which the current cycle detection code can't handle. But even if it could handle it, since the failure occurs in the dep we'd get error messages like: "in cc_library rule //the:dep: cycle in dependency graph" instead of the expected: "in cc_library rule //the:top_level_rule: cycle in dependency graph" This used to not be a problem because loading-phase cycle detection caught the cycle before all this triggered. But interleaved loading and analysis removes that gate. Tested: BuildViewTest cycle detection tests with dynamic configurations turned on -- MOS_MIGRATED_REVID=124391277
* Don't keep the full java.util.HashMap produced when checking for conflictingGravatar Googler2016-06-07
| | | | | | | actions; most of these have exactly one entry, and we never mutate them again. -- MOS_MIGRATED_REVID=124243817
* Add an overview comment about how the analysis phase works and some pointers ↵Gravatar Lukacs Berki2016-05-19
| | | | | | | to it. -- MOS_MIGRATED_REVID=122718503
* Add an "alias" rule.Gravatar Lukacs Berki2016-05-10
| | | | | | | This will be used to replace RedirectChaser so that we don't need to load packages during configuration creation anymore. -- MOS_MIGRATED_REVID=121935989
* Split ActionMetadata into ActionAnalysisMetadata and ActionExecutionMetadata.Gravatar Rumou Duan2016-04-26
| | | | | | | Except in action execution logic (ActionExecutionFunction, SkyframeActionExecutor, etc.), switch Action interface references to either ActionAnalysisMetadata if possible or ActionExecutionMetadata. -- MOS_MIGRATED_REVID=120723431
* Make aspects work through bind(). Gravatar Lukacs Berki2016-04-22
| | | | | | | | | | | bind() is assumed to be able to provide any provider. This is suboptimal, but beats the alternative of traversing the dependency graph to an arbitrary depth. The reason for the removal of the iteration ability in TransitiveInfoCollection is that now aspects can be attached to BindConfiguredTarget, too, which is not a RuleConfiguredTarget. Whereas I could have implemented the iterator, it was used only in BindConfiguredTarget anyway, so there didn't seem to be much reason to. Some work towards #952. -- MOS_MIGRATED_REVID=120549877
* Extract common logic for detecting action and artifact prefix conflicts.Gravatar Rumou Duan2016-04-19
| | | | | -- MOS_MIGRATED_REVID=120145833
* Remove AspectClass.getDefinitionGravatar Dmitry Lomov2016-04-07
| | | | | | | | | | Aspect becomes a triple (AspectClass, AspectDefinition, AspectParameters) and loses its equals() method. After this CL, SkylarkAspectClass.getDefintion still exists and is deprecated. -- MOS_MIGRATED_REVID=119159653
* Use two configurations for AspectKeys.Gravatar Michael Staib2016-03-01
| | | | | | | | | | | | In order for Aspects to support dynamic configuration, they need to have two configurations: one to instantiate the Aspect with, containing all the fragment dependencies of the Aspect itself, and one to create the ConfiguredTargetValue.key with, containing only the dependencies of the Rule. This expands AspectKey to have a second configuration, although it currently does not populate that key with anything different. -- MOS_MIGRATED_REVID=115997454
* Implement proper error handling for interleaved loading and analysis.Gravatar Ulf Adams2016-02-02
| | | | | | | | Add test coverage by re-running BuildViewTest with the new Skyframe loading phase runner. -- MOS_MIGRATED_REVID=113517509
* Refactor DependencyResolver to collect and return loading errors.Gravatar Ulf Adams2016-01-28
| | | | | | | | | This should never be triggered in production, where we always run a loading phase first and only analyze targets that load successfully. I.e., this is just plumbing which will be hooked up in a subsequent change. -- MOS_MIGRATED_REVID=113258593
* Store data about aspect configurations on Dependencies.Gravatar Michael Staib2016-01-27
| | | | | | | | | | | | | | | | | | | | Dependencies are the data structure which needs to propagate the configuration for each aspect as created by trimConfigurations down to the point where it's actually used. We need this to store different configurations for different aspects in a world where aspects have their own configurations, which may have more fragments than the target they're attached to. That world is on its way. Also in this CL: * Refactor Dependency to be an abstract parent class with separate implementations for Attribute.Transitions and BuildConfigurations, as well as null configurations, to avoid having to check nullness in various places. Users of the API will not see this, but get factory methods instead of constructors. As a consequence of this, refactor Dependency to be its own top-level class instead of a nested class in DependencyResolver. -- MOS_MIGRATED_REVID=113109615
* Make AspectFunction error handling match TopLevelAspectFunction.Gravatar Ulf Adams2016-01-25
| | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | Also match ConfiguredTargetFunction for target loading. It isn't currently possible to trigger either of these code paths - the loading phase ensures that we never attempt to analyze targets that fail to load - the Skylark import or conversion cannot fail, because Skylark checks during .bzl execution that all referenced symbols are Skylark aspects Therefore, the only way to trigger this would be if there was a native rule requesting a non-existent or broken Skylark aspect for its dependencies, but that is currently not possible - native rules can only request native aspects. However, for interleaved loading and analysis, we need to limit the set of exception classes that can be thrown from AspectFunction - we do that here by changing the constructor of AspectFunctionException to only accept either NoSuchThingException or AspectCreationException. That in turn requires that we re-throw the Skylark import and conversion exceptions as AspectCreationException, which is exactly what TopLevelAspectFunction is already doing, and necessary for correct error handling if we do ever support Skylark aspects in native rules. Alternatively, I could change the code path to crash Bazel, but that seems strictly worse. Even if we can't test this code, it's conceptually the right way to handle these errors. I'll move part of the error handling into loadSkylarkAspect in a subsequent change. -- MOS_MIGRATED_REVID=112938284
* Move analysis root cause tracking to the ConfiguredTargetFunction.Gravatar Ulf Adams2016-01-25
| | | | | | | | | | | | | | | | | | | | | | | | | The main remaining problem with interleaved loading and analysis is error handling. When interleaving, we don't run a real loading phase anymore, and loading errors can occur during the analysis phase, and need to be handled there. The plan is to have ConfiguredTargetFunction throw a ConfiguredValueCreationException with a list of all loading root causes, which requires that we also catch ConfiguredValueCreationException here, which in turn breaks analysis root cause handling, as that is currently relying on Skyframe root cause tracking. Moving analysis root cause handling into CTFunction makes it possible to subsequently also implement loading root cause handling here. This is also necessary if we want to have complete root cause handling in the general case: a target may have any number (and combination) of loading and analysis root causes at the same time. For now, we only pass a single analysis root cause, which mirrors the current Skyframe-based implementation. -- MOS_MIGRATED_REVID=112930871
* Use the existing exception instance, not a new one.Gravatar Ulf Adams2016-01-25
| | | | | | | | | | | | | Note that this can never happen in production - the loading phase ensures that we only attempt to analyze targets that load successfully. That's also why there are no tests for this. This code path will become live in a later change, with corresponding test coverage; the existing tests cover this in principle, we just need to make them run with the reduced loading phase. -- MOS_MIGRATED_REVID=112926211
* General cleanup for the configured target / aspect creation code.Gravatar Ulf Adams2016-01-19
| | | | | | | | | | | | | | - update some comments - add some comments to make it easier to follow - delete some dead code, in particular the SkyframeDependencyResolver can never be null; remove an non-applicable @Nullable annotation I'm trying to figure out how the error handling code works, in order to add support for interleaved loading+analysis, which requires handling loading errors in this code path. -- MOS_MIGRATED_REVID=112456674
generated by cgit v1.2.3 (git 2.39.1) at 2024-08-07 04:33:37 +0000