Open jhennessey opened 3 years ago
This code addresses this issue by passing a new property DynamicPlatformResolutionPreferredSpecificPlatform
which gets set to the first Platform
value that isn't AnyCPU
. That property gets propagated to each reference and, when necessary, gets used to create a platform look-up table that gets passed to the GetCompatiblePlatform
task: https://github.com/jhennessey/msbuild/commit/e103be15ae83e28a5ef79a2fe0e6a22bced16621
@BenVillalobos @AArnott - Would be curious on your thoughts on that approach.
Thanks for writing this up along with your thoughts and a sample of your proposal.
Your new property doesn't sound like it is guaranteed to be set as a global property for every single P2P reference. Since it doesn't change the OutputPath, that concerns me because it could lead to overbuild of a project, and thus timing breaks when the project builds twice concurrently, once with this global property and once without.
In your example, ProjectB will build twice if a ProjectD [AnyCPU] exists in the graph with no x86/x64 project referencing it. ProjectB [AnyCPU] will try both to "copy local" the x86 and the x64 binaries of ProjectC between ProjectB's two builds. The result of the build is now non-deterministic as to which binary will "win", and likely to break the build when ProjectB builds concurrently. As an AnyCPU project, its output shouldn't vary by CPU architecture at all. And IMO it shouldn't have ProjectC's binaries in its output directory both because it professes to be AnyCPU and because of this undefined end result.
One way we could potentially make this work is to have ProjectB build twice, with two separate output directories. But you've already stated your concern that that slows down the build. What if we only compiled once though? If ProjectB had an AnyCPU "inner build", but then its outer build deployed the output to two separate locations, and did copy-local of the appropriate x86 or x64 dependencies as appropriate, that would at least pacify some of my concerns. But compilation is sometimes not the dominating time in a large repo build so I don't know how much this buys you.
Why does ProjectB have ProjectC as a dependency in your scenario anyway? Does ProjectB compilation actually reference the output from ProjectC in the compilation step, or is it merely to express a runtime dependency?
Since it doesn't change the OutputPath, that concerns me because it could lead to overbuild of a project
It would change outputpath, wouldn't it? In B to C (in the original post), C would be told to build with Platform=whateverA built as
, so when C builds it would get its outputpath set accordingly.
In your example, ProjectB will build twice if a ProjectD [AnyCPU] exists in the graph with no x86/x64 project referencing it
Could you draw out the graph here? I might be misunderstanding
Speaking strictly functionally:
I like the change. I'm not super familiar with AdditionalProperties
, but if it works like it looks like it does then it's worth including IMO. Andrew is asking the right questions (read: asking "why?"), but I'm generally in favor of allowing scenarios that I don't quite understand but regardless are scenarios customers have. Probably because I've seen so many that already exist and that we need to support π
It would change outputpath, wouldn't it? In B to C (in the original post), C would be told to build with Platform=whateverA built as, so when C builds it would get its outputpath set accordingly.
I'm talking about the output path of B. It would build twice (once with DynamicPlatformResolutionPreferredSpecificPlatform
set and once without), but with the same output path. This would slow down the build (at best) or cause timing breaks (at worst).
Could you draw out the graph here? I might be misunderstanding
Sure. The problem requires that two paths to B exist--one that has gone through a platform-specific project and one that has not.
In the graphs below, DPRPSS = DynamicPlatformResolutionPreferredSpecificPlatform
as @jhennessey described above.
Thinking about this more, this case would not be a problem, because the graph starts with an architecture:
ββββββββββββββ βββββββββββββ ββββββββββββββ
βββββββ¬ββββΊβ A (x86/x64)ββββββββββββββΊβ B (AnyCPU)βββββΊβ C (x86/x64)β
ββββββββ΄ββββββ€ ββββββββββββββ DPRPSS=x64 βββββββββββββ ββββββββββββββ
β dirs.proj β P=AnyCPU β² P=x64
βββββββββ¬βββββ β
P=x64 β ββββββββββββββ β
ββββββββββΊβ D (AnyCPU) ββββββββββββββββββββ
ββββββββββββββ DPRPSS=x64
P=AnyCPU
But this case would build B twice:
ββββββββββββββ βββββββββββββ ββββββββββββββ
βββββββ¬ββββΊβ A (x86) ββββββββββββββΊβ B (AnyCPU)βββββββββββΊβ C (x86/x64)β
ββββββββ΄ββββββ€ ββββββββββββββ DPRPSS=x86 βββββββββββββ P=x86 ββββββββββββββ
β dirs.proj β P=AnyCPU β²
βββββββββ¬βββββ β
β ββββββββββββββ β
ββββββββββΊβ D (AnyCPU) ββββββββββββββββββββ
ββββββββββββββ P=AnyCPU
In this example, A only offers one architecture (x86 in this case) so specifying it explicitly is not required as the one is the default. Notice how there is now a path to B that does not set DPRPSS and that does. B now builds twice.
Ah, thanks for the graph. My confusion was AdditionalProperties
sounding too good to be true (giving projects properties for free, not globally).
So it sounds like the realistic scenario here is some projects should build as every platform it contains because there will be multiple projects that depend on it. Are we leaning toward a future where those projects contain some "build me entirely" property that allows it to have its own "outer" and "inner" builds for each platform?
I would very much like to see projects building all their supported platforms the same way they build their inner TFMs -- when they are built as top-level projects (without a global property specifying the platform to build.) But like multi-TFM projects, as a target of a P2P, only the TFM (or platform) that the referencing project requires should be built.
When I think about the scenario of B (AnyCPU) referencing C (x86/x64), ultimately B still (typically) needs to "copy local" the outputs of one of C's platform builds. So IMO B should still always specify the platform it wants explicitly and then only that platform should be built, and then copied local. That is assuming that B should reference C at all.
The best case I can think of for why a B AnyCPU should reference C is that B is just a library and doesn't touch any of C's arch-specific APIs. Theoretically if C was only arch-specific as an implementation detail, an AnyCPU library could reference it (and not copy local at all), leaving it to app A (x86) to copy both C's x86 output and B's AnyCPU output into A's output directory, and it would all work. This works best when C offers an AnyCPU reference assembly in addition to its arch-specific implementation assemblies. .NET Framework and .NET Core BCL do this all the time. We build AnyCPU projects most of the time, referencing BCL assemblies that at runtime may very well be arch-specific, but the ref assemblies we use to compile our projects do not include any APIs that vary with architecture. If there was good build/compiler tooling to support it, I'd say C should leverage that. In that case, B would get the anycpu ref assembly and life is good. And if another arch-specific project were to reference C, they would get the arch-specific build output on the same principle that led into this platform negotiation feature's original design: a P2P should consume the most precisely matching platform possible, because platform-specific is always better, or it wouldn't exist and we'd just have AnyCPU.
The SetPlatform Negotiation feature does not work "out-of-the-box" if there is an
AnyCPU
project that references a specific platform (x86
/x64
) project. For example, the following combination of projects will fail (or possibly fallback to a default platform):ProjectA [x86/x64] -> ProjectB [AnyCPU] -> ProjectC [x86/x64]
In this case, it would be ideal if ProjectB could infer that it should build ProjectC as either
x86
orx64
.Note: In real-world scenarios, ProjectC is generally forced to be both x86 / x64 in order to accommodate a third-party native wrapper.
Additional Context: Created issue based on this comment from @BenVillalobos in #6889.
Discussion Point: As mentioned by @AArnott here, you will generally get a processor architecture mismatch warning when building the specified project chain (but it does work). To address that warning (without working around it via setting the ResolveAssemblyWarnOrErrorOnTargetArchitectureMismatch property) requires ALL projects to have both
x86
andx64
platform configurations. This in turn means that you have to build everything twice to support your dualx86
andx64
buildβ¦which isn't ideal for really large codebases.