ubisoft / Sharpmake

Sharpmake is an open-source C#-based solution for generating project definition files, such as Visual Studio projects and solutions, GNU makefiles, Xcode projects, etc.
Apache License 2.0
935 stars 171 forks source link

[Csproj] Add support for conditional PackageReferences #258

Open jpankalainen opened 1 year ago

jpankalainen commented 1 year ago

Adds support for conditionals in csproj PackageReference entries.

jspelletier commented 1 year ago

checking this

jspelletier commented 1 year ago

Thanks for this intesting proposal.

I discussed this with a colleague as I don't know well the csproj generator.

While your proposal works, we thought it might be better to extend the existing ItemGroup support for supporting new types of conditions(Platform + Configuration)

There is right now support for automatic ItemGroup section conditions for dotnet framework version as you can see in the DotNetMultiFrameworksHelloWorld sample.

This can generate PackageReference references with framework version conditions when multiple framework versions are specified as target fragments. As you can see, there is no conditions here, the generation of conditions is driven by what happens in Configure methods.

Example(DotNetMultiFrameworksHelloWorld):
    if (target.GetFragment<DotNetFramework>().IsDotNetStandard())
    {
        conf.ReferencesByNuGetPackage.Add("System.Text.Encoding.CodePages", "4.5.0");
        conf.ReferencesByNuGetPackage.Add("Newtonsoft.Json", "13.0.3");
    }

would generate:

  <ItemGroup Condition="'$(TargetFramework)'=='netstandard2.0'">
    <PackageReference Include="Newtonsoft.Json" Version="13.0.3" />
    <PackageReference Include="System.Text.Encoding.CodePages" Version="4.5.0" />
  </ItemGroup>

I synced your branch and I modified the sample to use the nuget used in your tests to add more cases.


    if (target.GetFragment<DotNetFramework>().IsDotNetFramework())
    {
        conf.ReferencesByName.Add("System");
    }

    if (target.GetFragment<DotNetFramework>().IsDotNetStandard())
    {
        conf.ReferencesByNuGetPackage.Add("System.Text.Encoding.CodePages", "4.5.0");
        conf.ReferencesByNuGetPackage.Add("NUnit-debug", "3.4.1", condition: "$(Configuration)='Debug'");
        // Note: This can be confusing for people used to sharpmake configs as this code is executed in a debug config but it won't be
        // use in debug because of the condition
        conf.ReferencesByNuGetPackage.Add("NUnit", "3.4.1", condition: "$(Configuration)='Release'");
    }

    conf.ReferencesByNuGetPackage.Add("PackageAlwaysPresent", "1.0.0");

    generates:
  <ItemGroup Condition="'$(TargetFramework)'=='net461' OR '$(TargetFramework)'=='net472'">
    <Reference Include="System">
      <Private>False</Private>
    </Reference>
  </ItemGroup>
  <ItemGroup>
    <PackageReference Include="PackageAlwaysPresent" Version="1.0.0" />
  </ItemGroup>
  <ItemGroup Condition="'$(TargetFramework)'=='netstandard2.0'">
    <PackageReference Include="NUnit" Version="3.4.1" Condition="$(Configuration)='Release'" />
    <PackageReference Include="NUnit-debug" Version="3.4.1" Condition="$(Configuration)='Debug'" />
    <PackageReference Include="System.Text.Encoding.CodePages" Version="4.5.0" />
  </ItemGroup>

    Not bad but we think we should do something a bit different as we will explain below.

What we think we should do is we shouldn't specify the condition as parameter and modify Sharpmake so that it is able to magically generate the condition for Configurations and Platforms automatically. In fact, this is a bit similar to the PropertyGroup section. This would better align with how we do things in Sharpmake elsewhere.

Example(DotNetMultiFrameworksHelloWorld):

    if (target.GetFragment<DotNetFramework>().IsDotNetFramework())
    {
        conf.ReferencesByName.Add("System");
    }

    if (target.GetFragment<DotNetFramework>().IsDotNetStandard())
    {
        conf.ReferencesByNuGetPackage.Add("System.Text.Encoding.CodePages", "4.5.0");
        if (target.GetOptimization() == Optimization.Debug)
        {
            conf.ReferencesByNuGetPackage.Add("NUnit-debug", "3.4.1");
        }
        else
        {
            conf.ReferencesByNuGetPackage.Add("NUnit", "3.4.1");
        }
    }

    conf.ReferencesByNuGetPackage.Add("PackageAlwaysPresent", "1.0.0");

    This should generate:
    <ItemGroup Condition="'$(TargetFramework)'=='net461' OR '$(TargetFramework)'=='net472'">
      <Reference Include="System">
        <Private>False</Private>
      </Reference>
    </ItemGroup>
    <ItemGroup>
      <PackageReference Include="PackageAlwaysPresent" Version="1.0.0" />
    </ItemGroup>    
    <ItemGroup Condition="'$(TargetFramework)'=='netstandard2.0'">
        <PackageReference Include="System.Text.Encoding.CodePages" Version="4.5.0" />
    </ItemGroup>
    <ItemGroup Condition="'$(TargetFramework)'=='netstandard2.0' AND "$(Configuration)='Release'" ">
        <PackageReference Include="NUnit" Version="3.4.1" />
    </ItemGroup>
    <ItemGroup Condition="'$(TargetFramework)'=='netstandard2.0' AND "$(Configuration)='Debug'" ">
        <PackageReference Include="NUnit-debug" Version="3.4.1" />
    </ItemGroup>

To implement this, we would have to rename TargetFrameworksCondition to something like TargetsConditions and modify it to take into account the Configuration and platform associated with the item that is added to the collection. I think this will handled automatically but the code shouldn't generate useless conditions, similar to the example just above.

For example: