Update static graph spec#4242
Conversation
rainersigwald
left a comment
rainersigwald
left a comment
There was a problem hiding this comment.
mostly LGTM but I'm not sure I understand the code changes: are they integral to the doc change, or just more convenient to bundle?
| <!-- how it works: outer builds and inner builds --> | ||
| Crosstargeting is implemented by having a project reference itself multiple times, once for each combination of crosstargeting global properties. This leads to multiple evaluations of the same project, with different global properties. These evaluations can be classified in two groups | ||
| 1. Multiple inner builds. Each inner build is evaluated with one set of crosstargeting global properties (e.g. the `TargetFramework=net472` inner build, or the `TargetFramework=netcoreapp2.2` inner build). | ||
| 2. One outer build. This evaluation does not have any crosstargeting global properties set. It can be viewed as a proxy for the inner builds. Other projects query the outer build in order to learn the set of valid crosstargeting global properties (the set of valid inner builds). When the outer build is also the root of the project to project graph, the outer build multicasts the entry target (i.e. `Build`, `Clean`, etc) to all inner builds. |
There was a problem hiding this comment.
Is it a problem that this "multicast" isn't very accurate today, and basically only works for Build and Clean?
There was a problem hiding this comment.
Not really, as Build, Clean are known graph wide operation for the SDK, and its multicasting code knows about both of them. Other SDKs that would want to implement their own multitargeting scheme, would have their own separate graph wide operations and their separate multicasting machinery. Unless the intent is that Microsoft.Common.Crosstargeting.targets should be used by all potentially crosstargeting SDKs. So I'd say to ignore this issue until the next crosstargeting SDK comes along and we have to actually decide on common infrastructure. Or until we hit real world usages :)
| - Project classification: | ||
| - *Outer build*, when `$($(InnerBuildProperty))` is empty AND `$($(InnerBuildPropertyValues))` is not empty. | ||
| - *Dependent inner build*, when both `$($(InnerBuildProperty))` and `$($(InnerBuildPropertyValues))` are non empty. These are inner builds that were generated from an outer build. | ||
| - *Standalone inner build*, when `$($(InnerBuildProperty))` is not empty and `$($(InnerBuildPropertyValues))` is empty. These are inner builds that were not generated from an outer build. |
There was a problem hiding this comment.
I don't understand the distinction from the next case. Don't we always fill in TargetFramework, with a default if necessary? How would this arise?
There was a problem hiding this comment.
TargetFramework is not filled in for legacy csprojes:
So then with these non crosstargeting projects, SDKs can allow a mix of crosstargeting and non crosstargeting projects. If the need arises, we can add extra settings on how these are interpreted, like whether the SDK allows non crosstargeting projects or not.
There was a problem hiding this comment.
I guess the larger question is: is there really a strong distinction between a single-targeted SDK-style project and a legacy csproj?
There was a problem hiding this comment.
For the current SDK not really, but the code needs to reason about each type of project, so every project needs a type :) It seemed somehow wrong to classify these non-participating projects as standalone inner builds, so I added this extra one. Let me know if you feel strongly about removing it.
Realized that the spec made an assertion for which no unit test existed, so decided to add the unit test. While adding the test, I need to expose extra stuff, and while exposing extra stuff I realized I can do a nice refactoring to decouple some stuff out of BuildManager. |
Co-Authored-By: cdmihai <micodoba@microsoft.com>
Co-Authored-By: cdmihai <micodoba@microsoft.com>
Co-Authored-By: cdmihai <micodoba@microsoft.com>
Co-Authored-By: cdmihai <micodoba@microsoft.com>
2 participants
With the details from #4218 and caching.