Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Respect projects that say they only have one TFM #1667

Merged

Conversation

rainersigwald
Copy link
Member

@rainersigwald rainersigwald commented Feb 2, 2017

Partially fixes dotnet/sdk#739.

The first iteration of cross-targeting support code unconditionally
queried each ProjectReference for the best TFM to build against, and
then explicitly specified that TFM when building the reference. This
caused a race condition when building a set of projects that had a
single-TFM project and another project that had a reference to it. The
entry point (probably .sln) build would build the referenced project
with no TF specified, and then the referencing project would build it
with an explicit TF specified. These two builds appeared different to
the MSBuild engine (because they had different sets of global
properties) and so were both executed, resulting in a race condition.

The fix is in two parts:

  • Allow a project to report back through GetTargetFrameworkProperties
    that it only has one TF to build. (Report that a project only has one TFM sdk#797)
  • When a project reports that it has only one TF to build, issue its
    build request without specifying any TF.

This commit is the latter.

Partially fixes dotnet/sdk#739.

The first iteration of cross-targeting support code unconditionally
queried each ProjectReference for the best TFM to build against, and
then explicitly specified that TFM when building the reference. This
caused a race condition when building a set of projects that had a
single-TFM project and another project that had a reference to it. The
entry point (probably .sln) build would build the referenced project
with no TF specified, and then the referencing project would build it
with an explicit TF specified. These two builds appeared different to
the MSBuild engine (because they had different sets of global
properties) and so were both executed, resulting in a race condition.

The fix is in two parts:
* Allow a project to report back through GetTargetFrameworkProperties
that it only has one TF to build.
* When a project reports that it has only one TF to build, issue its
build request without specifying any TF.

This commit is the latter.
@@ -1530,6 +1530,10 @@ Copyright (C) Microsoft Corporation. All rights reserved.
<ItemGroup>
<_MSBuildProjectReferenceExistent Condition="'%(_MSBuildProjectReferenceExistent.Identity)' == '%(Identity)'">
<SetTargetFramework>$(_ProjectReferenceTargetFrameworkProperties)</SetTargetFramework>

<UndefineProperties Condition="$(_ProjectReferenceTargetFrameworkProperties.Contains(`ProjectHasSingleTargetFramework=true`))">%(_MSBuildProjectReferenceExistent.UndefineProperties);TargetFramework;ProjectHasSingleTargetFramework</UndefineProperties>
Copy link
Contributor

@cdmihai cdmihai Feb 3, 2017

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why not make ProjectHasSingleTargetFramework a separate output property of the MSBuild task rather than embedding it inside another property?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

That kind of thing (returning a tuple) would have been ideal, but I don't see a way to do it. The MSBuild task only has one output parameter, and it can be assigned either to an Item or a Property.

Returning an item has the advantage of being structured data, but it runs somewhat afoul of the current nature of assigning these properties. I put that together at dotnet/sdk#797 (comment) -- do you like that better? It's better in some ways (this "undefined, or unconditionally undefined" is definitely ugly), but worse in others (clearing the item so we can look at it one at a time, extracting the metadata for insertion into the referring project's metadata). I think on balance I like this better but I could be persuaded . . .

@AndyGerlicher AndyGerlicher merged commit 7636893 into dotnet:vs15-rtw Feb 3, 2017
rainersigwald added a commit to rainersigwald/sdk that referenced this pull request Feb 3, 2017
Partially fixes dotnet#739.

The first iteration of cross-targeting support code unconditionally
queried each ProjectReference for the best TFM to build against, and
then explicitly specified that TFM when building the reference. This
caused a race condition when building a set of projects that had a
single-TFM project and another project that had a reference to it. The
entry point (probably .sln) build would build the referenced project
with no TF specified, and then the referencing project would build it
with an explicit TF specified. These two builds appeared different to
the MSBuild engine (because they had different sets of global
properties) and so were both executed, resulting in a race condition.

The fix is in two parts:
* Allow a project to report back through GetTargetFrameworkProperties
that it only has one TF to build.
* When a project reports that it has only one TF to build, issue its
build request without specifying any TF (dotnet/msbuild#1667)

This commit is the former.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants