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

Improved support for NuGet packages with multiple targets #380

Closed
christianlang opened this issue Nov 21, 2014 · 8 comments
Closed

Improved support for NuGet packages with multiple targets #380

christianlang opened this issue Nov 21, 2014 · 8 comments

Comments

@christianlang
Copy link
Contributor

Following up on #281 I have been thinking about a more robust and future-proof way to handle NuGet packages that provide multiple version in the lib folder. Especially packages supporting portable profiles. Right now, the way Paket handles this is too restrictive. More specifically, there are two problems:

  1. If a package supports "net45+win8+wp8+wpa81" it's not only usable from projects that target portable profile 259 (which is the profile that exactly supports the platforms mentioned in this folder name). It's also okay to use it from profile 47 (which is .Net 4.5 + Silverlight 5 + Windows 8) or any other portable profile that targets just a subset of these platforms. So we have to know which profiles are compatible, even if they are not the same.

  2. Portable profiles can be named in a whole lot of different ways. "net45+win8+wp8+wpa81" could also be written as "portable-wpa8+netcore45+net45+windowsphone8", or someone could decide to specifically add mono to the list and call it "net45+win8+wp8+wpa81+monoandroid+monotouch". The algorithm has to be robust enough to handle this. Sometimes a clear mapping to a portable profile isn't even possible and we have to make a guess.

I have implemented an algorithm that solves these problems but I haven’t been able to integrate it into Paket yet, because the approach is so different. You’ll find my current state so far in this repo:
https://github.com/christianlang/PaketExperiments
Please follow the unit tests and the test command-line app (including the comments in Program.fs). This should give you a good intro to the code.

You will see that I implemented a heuristic that tries to find the best matching path for a given target profile. So, as an example, say a NuGet package contains the following paths:

lib\
    net40+win8\
    net451+win81+wpa81\
    net45\

Now I take every known target profile (all known versions of .NET and Silverlight, all portable profiles, etc.) and check if any of the paths supports them. In many cases this will return more than one path.

For a .NET 4.5 project, actually all three paths will be fine, for example. But "net45" will the best choice, because it contains a specific build just for this framework.

For the portable profile 44 (.NET 4.5.1, Windows 8.1) the first two paths will be ok. But the second path ("net451+win81+wpa81") will be a better match because it closer matches the versions.

The algorithm I propose tries to respect this by applying penalties when the version doesn't match exactly and by preferring paths which support less platforms.

As a result, we can't regard paths individually but we always have to look at all available paths to find the best one for a given target profile.

Unfortunately this doesn't play well at all with the way Paket currently handles paths and project references. In several places the FrameworkIdentifier.Extract method is used to get exactly one target framework for a given path.

I also think we should change the way the libs are referenced from the project files. Right now the process is rather complicated, I think, and the InstallModel tries to minimize the number of Choose/When statements needed to support all targets that come with a NuGet package.

I suggest that, instead, we have one “When” option per target folder of the package. That way we will always create the least amount of Choose/When options possible, which is one for each file¬.

Here’s an example of a package structure and the resulting references in project files.

lib\
    net40+win8\Foo.dll
    net451+win81+wpa81\Foo.dll
    net45\Foo.dll
    <Choose>
      <!--net40+win8 -->
      <When Condition="
            $(TargetFrameworkIdentifier) == '.NETFramework' And $(TargetFrameworkVersion) == 'v4.0'
            Or $(TargetFrameworkIdentifier) == '.NETCore' And $(TargetFrameworkVersion) == 'v8.0'
            Or $(TargetFrameworkProfile) == 'Profile5'
            Or $(TargetFrameworkProfile) == 'Profile6'
            Or $(TargetFrameworkProfile) == 'Profile7'
            ">
            <ItemGroup>
              <Reference Include="Foo">
                <HintPath>..\packages\Foo\lib\net45\Foo.dll</HintPath>
                <Private>True</Private>
                <Paket>True</Paket>
              </Reference>
            </ItemGroup>
      </When>
      <!--net451+win81+wpa81-->
      <When Condition="
            $(TargetFrameworkIdentifier) == '.NETFramework' And $(TargetFrameworkVersion) == 'v4.5.1'
            Or $(TargetFrameworkIdentifier) == '.NETFramework' And $(TargetFrameworkVersion) == 'v4.5.2'
            Or $(TargetFrameworkIdentifier) == '.NETCore' And $(TargetFrameworkVersion) == 'v8.1'
            Or $(TargetFrameworkIdentifier) == 'WindowsPhoneApp' And $(TargetFrameworkVersion) == 'v8.1'
            Or $(TargetFrameworkProfile) == 'Profile32'
            Or $(TargetFrameworkProfile) == 'Profile44'
            Or $(TargetFrameworkProfile) == 'Profile151'
            ">
            <ItemGroup>
              <Reference Include="Foo">
                <HintPath>..\packages\Foo\lib\net40\Foo.dll</HintPath>
                <Private>True</Private>
                <Paket>True</Paket>
              </Reference>
            </ItemGroup>
      </When>
      <!--net45-->
      <When Condition="
            $(TargetFrameworkIdentifier) == '.NETFramework' And $(TargetFrameworkVersion) == 'v4.0'
            Or $(TargetFrameworkIdentifier) == '.NETCore' And $(TargetFrameworkVersion) == 'v8.0'
            Or $(TargetFrameworkProfile) == 'Profile5'
            Or $(TargetFrameworkProfile) == 'Profile6'
            Or $(TargetFrameworkProfile) == 'Profile7'
            ">
            <ItemGroup>
              <Reference Include="Foo">
                <HintPath>..\packages\Foo\lib\net40\Foo.dll</HintPath>
                <Private>True</Private>
                <Paket>True</Paket>
              </Reference>
            </ItemGroup>
      </When>
    </Choose>

Of course this doesn’t fit the current approach of Paket at all. So, before I continue going down that path, I’d like to get some feedback. Do you think this is the way to go? Are you willing to help me change Paket significantly to make that work? Do you see problems with my algorithm?

@cdrnet
Copy link
Member

cdrnet commented Nov 24, 2014

When tweaking the penalties, please keep in mind that for a full framework target imo we should always prefer full framework builds even if they are older (at very least within the major version) and there is an exact match by a PCL build.

For example:

lib\
    net451+win81+wpa81\Foo.dll
    net40\Foo.dll

For target net451 (and also e.g. for net453) we should still take the net40 one, not the PCL one. The current alpha build 0.17.0-alpha003 would take the PCL one in this case.

@christianlang
Copy link
Contributor Author

Hm, can you give a reason why we should prefer the .NET 4.0 version? I actually think we should do the opposite - that's why I designed it that way. The new portable version might use new features the old .NET 4.0 build doesn't support (like async/await). In my experience the PCL in this case would be more "modern" than the .NET 4.0 build.

@christianlang
Copy link
Contributor Author

Do you know what NuGet would do in this case? Which one would it reference?

@cdrnet
Copy link
Member

cdrnet commented Nov 24, 2014

Related: #236

NuGet always installs proper builds (e.g. net40, SL50) instead of PCL ones, even if older - except if the target itself is a PCL. There's even a pattern around this where some NuGet packages provide proper builds for all targets supported by some PCL profiles and then add fake PCL assemblies (so other PCL libs can reference them; but they are never actually used for anything beyond IntelliSense).

@christianlang
Copy link
Contributor Author

Ok, makes sense. Then we have to revisit this. Thanks.

@forki
Copy link
Member

forki commented Nov 24, 2014

In any case: we should add tests
On Nov 24, 2014 4:42 PM, "Christian Lang" notifications@github.com wrote:

Ok, makes sense. Then we have to revisit this. Thanks.


Reply to this email directly or view it on GitHub
#380 (comment).

@cdrnet
Copy link
Member

cdrnet commented Nov 24, 2014

Some background on why I think proper builds should be preferred: they can always leverage the full platform, while PCL builds are often a restricted compromise.

For example, in Math.NET Numerics the proper .Net build is much more efficient and can use the proper framework types (e.g. Complex from System.Numerics) while at least some of the PCL builds have to bring their own incompatible types (System.Numerics is not available in most profiles) and even drop support for some features entirely (e.g. no Crypto-RNG, no native MKL provider, no arbitrary precision numbers).

If a lib wants to leverage features of newer versions it should provide a proper build for that version.

@christianlang
Copy link
Contributor Author

@forki I'm going to look into that

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

No branches or pull requests

3 participants