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

Mark "target framework" as a variant dimension #3020

Merged
merged 2 commits into from
Dec 12, 2017
Merged

Conversation

davkean
Copy link
Member

@davkean davkean commented Dec 5, 2017

One of many changes as we move to using the new IActiveConfigurationGroupService which returns all the configurations that we consider "active" for a multi-targeting scenarios (replacing IActiveConfiguredProjectsProvider).

  • Mark "target framework" as a variant dimension
    This marks "target framework" dimension as a variant dimension; ie a configuration dimension that when present is effectively ignored when calculating "active configurations" returned by IActiveConfigurationGroupService:

For example, given the following multi-targeting project:

  -> All known project configurations:

      Configuration Platform    TargetFramework
      -------------------------------------------
              Debug |   AnyCPU  |           net45
              Debug |   AnyCPU  |           net46
            Release |   AnyCPU  |           net45
            Release |   AnyCPU  |           net46

  -> Active solution configuration:

              Debug |   AnyCPU  |           net45

  -> Target framework dimension is ignored, and active configurations returned by IActiveConfigurationGroupService:

              Debug |   AnyCPU  |           net45
              Debug |   AnyCPU  |           net46

Whereas, given the following non-multi-targeting project:

  -> All known project configurations:

      Configuration   Platform
      ------------------------
              Debug |   AnyCPU
            Release |   AnyCPU

  -> Active solution configuration:

              Debug |   AnyCPU

  -> Active configurations return active configurations returned by IActiveConfigurationGroupService:

              Debug |   AnyCPU

We need to consume a new API from CPS, and hence need to upgrade dependency. It brings in a new MSBuild dependency so we need to upgrade it too to avoid conflicts.
This marks "target framework" dimension as a variant dimension; ie a configuration dimension that when present is is effectively ignored when calculating "active configurations" returned by IActiveConfigurationGroupService:

For example, given the following multi-targeting project:

      -> All known project configurations:

          Configuration Platform    TargetFramework
          -------------------------------------------
                 Debug |   AnyCPU  |           net45
                  Debug |   AnyCPU  |           net46
                Release |   AnyCPU  |           net45
                Release |   AnyCPU  |           net46

      -> Active solution configuration:

                  Debug |   AnyCPU  |           net45

      -> Target framework dimension is ignored, and active configurations returned by IActiveConfigurationGroupService:

                  Debug |   AnyCPU  |           net45
                  Debug |   AnyCPU  |           net46

 Whereas, given the following non-multi-targeting project:

      -> All known project configurations:

          Configuration   Platform
          ------------------------
                  Debug |   AnyCPU
                Release |   AnyCPU

      -> Active solution configuration:

                  Debug |   AnyCPU

      -> Active configurations return active configurations returned by IActiveConfigurationGroupService:

                  Debug |   AnyCPU
@davkean
Copy link
Member Author

davkean commented Dec 5, 2017

Still testing these bits, CPS is already consuming this - so adding this attribute will immediately take effect for the Error List. Now all errors/warnings from all design-time builds for a multi-targeting project will be added to it, instead of just the first "active" one.

@davkean davkean requested a review from a team December 5, 2017 06:32
@davkean davkean changed the title Mark "target framework" as a variant dimension [WIP] Mark "target framework" as a variant dimension Dec 5, 2017
@Pilchie
Copy link
Member

Pilchie commented Dec 6, 2017

LGTM - is the plan to merge this now and then follow up, or add more commits to this PR?

@davkean
Copy link
Member Author

davkean commented Dec 6, 2017

Don't plan on adding more commits to this PR - but am still testing this.

@davkean
Copy link
Member Author

davkean commented Dec 6, 2017

Okay was wrestling with this all day - finally got bits that work.

Before this change, given this project:

<Project Sdk="Microsoft.NET.Sdk">

  <PropertyGroup>
    <OutputType>Exe</OutputType>
    <TargetFrameworks>netcoreapp2.0;net45</TargetFrameworks>
  </PropertyGroup>

  <Target AfterTargets="Compile" Name="FooBar">

    <Warning Text="$(TargetFramework)" />
  </Target>
</Project>

The following would be output into the Error List after a design-time build:

Warning		netcoreapp2.0	ConsoleApp128	C:\Users\davkean\Source\Repos\ConsoleApp128\ConsoleApp128\ConsoleApp128.csproj	10	

After this change, the folllowing is now output to the Error List:

Warning		netcoreapp2.0	ConsoleApp128	C:\Users\davkean\Source\Repos\ConsoleApp128\ConsoleApp128\ConsoleApp128.csproj	10	
Warning		net45	ConsoleApp128	C:\Users\davkean\Source\Repos\ConsoleApp128\ConsoleApp128\ConsoleApp128.csproj	10	

@davkean davkean changed the title [WIP] Mark "target framework" as a variant dimension Mark "target framework" as a variant dimension Dec 6, 2017
@davkean
Copy link
Member Author

davkean commented Dec 6, 2017

This is ready for signoff.

@davkean
Copy link
Member Author

davkean commented Dec 6, 2017

@lifengl When are you planning on inserting this so that we can coordinate?

@jmarolf
Copy link
Contributor

jmarolf commented Dec 6, 2017

can we add a test for this please?

@davkean
Copy link
Member Author

davkean commented Dec 6, 2017

We don't have working VSI tests - how would you propose I write one?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants