Skip to content

Commit

Permalink
Implement more restrictive merge of roll forward
Browse files Browse the repository at this point in the history 
If LatestMinor and Major references are merged, previously the effective reference would be LatestMinor. Bu that goes against the Major semantics of "pick closest".
This change is modifying the outcome of that merge to Minor. This produces the more restrictive version selection (lower versions are prefered).

This behavior is still being discussed in dotnet/core-setup#5870 and may change.

Added tests for merging roll forward settings.
  • Loading branch information
@vitek-karas
vitek-karas committed Apr 19, 2019
1 parent 74f72bc commit 26ba328
Show file tree
Hide file tree
Showing 2 changed files with 105 additions and 1 deletion.
21 changes: 20 additions & 1 deletion src/corehost/cli/fx_reference.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -62,9 +62,28 @@ bool fx_reference_t::is_compatible_with_higher_version(const fx_ver_t& higher_ve

void fx_reference_t::merge_roll_forward_settings_from(const fx_reference_t& from)
{
// In general lower value roll_forward should win, with one exception
// If Major and LatestMinor is merged, the result should be Minor for now.
// - this is the conservative approach where the most strict behavior is taken
// Major means "closest available version and allow roll over major"
// LatestMinor means "latest available version and allow roll over minor"
// So the most restrictive merge would be "closest available version and allow roll over minor"
// which is exactly what Minor does.
// All other combinations are already following the same logic.

if (from.get_roll_forward() < get_roll_forward())
{
set_roll_forward(from.get_roll_forward());
roll_forward_option effective_roll_forward = from.get_roll_forward();
if (from.get_roll_forward() == roll_forward_option::LatestMinor && get_roll_forward() == roll_forward_option::Major)
{
effective_roll_forward = roll_forward_option::Minor;
}

set_roll_forward(effective_roll_forward);
}
else if (from.get_roll_forward() == roll_forward_option::Major && get_roll_forward() == roll_forward_option::LatestMinor)
{
set_roll_forward(roll_forward_option::Minor);
}

if (get_apply_patches() == true && from.get_apply_patches() == false)
Expand Down
85 changes: 85 additions & 0 deletions src/test/HostActivationTests/FrameworkResolution/RollForwardMultipleFrameworks.cs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -651,6 +651,91 @@ public void PreferReleaseToRelease(string appVersionReference, string frameworkV
.ShouldHaveResolvedFramework(MicrosoftNETCoreApp, resolvedFramework);
}

// Verify that inner framework reference (<fxRefVersion>, <fxRollForward>)
// is correctly reconciled with app's framework reference (<appRefVersion>, <appRollForward>).
// It then also tests it the other way round (as the result should not depend on which setting comes from FX and which from app)
// In this case the direct reference from app is first, so the framework reference from app
// is actually resolved against the disk - and the resolved framework is than compared to
// the inner framework reference (potentially causing re-resolution).
// This is mostly a collection of interesting cases as testing the full matrix is prohibitively large
[Theory] // appRefVersion appRollForward fxRefVersion fxRollForward resolvedFramework
// Disable + anything -> Disable
[InlineData("5.1.0", Constants.RollForwardSetting.Disable, "5.1.0", Constants.RollForwardSetting.Disable, null)]
[InlineData("5.1.0", Constants.RollForwardSetting.Disable, "5.1.0", Constants.RollForwardSetting.LatestPatch, null)]
[InlineData("5.1.0", Constants.RollForwardSetting.Disable, "5.1.0", Constants.RollForwardSetting.Minor, null)]
[InlineData("5.1.0", Constants.RollForwardSetting.Disable, "5.1.0", Constants.RollForwardSetting.LatestMinor, null)]
[InlineData("5.1.0", Constants.RollForwardSetting.Disable, "5.1.0", Constants.RollForwardSetting.Major, null)]
[InlineData("5.1.0", Constants.RollForwardSetting.Disable, "5.1.0", Constants.RollForwardSetting.LatestMajor, null)]
// Default - should apply normal Minor semantics
[InlineData("5.0.0", null, "5.0.0", null, "5.1.3")]
// Default + LatestPatch -> LatestPatch
[InlineData("5.0.0", null, "5.0.0", Constants.RollForwardSetting.LatestPatch, null)]
// Default + LatestMinor -> Minor
[InlineData("5.0.0", null, "5.0.0", Constants.RollForwardSetting.LatestMinor, "5.1.3")]
// Default + Major -> Minor
[InlineData("5.0.0", null, "5.0.0", Constants.RollForwardSetting.Major, "5.1.3")]
// Default + LatestMajor -> Minor
[InlineData("5.0.0", null, "5.0.0", Constants.RollForwardSetting.LatestMajor, "5.1.3")]
// LatestMinor + Major -> for now picks the most restrictive and thus Minor behavior
[InlineData("5.0.0", Constants.RollForwardSetting.LatestMinor, "5.0.0", Constants.RollForwardSetting.Major, "5.1.3")]
// LatestMinor + LatestMajor -> LatestMinor
[InlineData("5.0.0", Constants.RollForwardSetting.LatestMinor, "5.0.0", Constants.RollForwardSetting.LatestMajor, "5.6.0")]
// LatestMajor + Major -> Major
[InlineData("4.0.0", Constants.RollForwardSetting.LatestMajor, "4.0.0", Constants.RollForwardSetting.Major, "5.1.3")]
// LatestMajor + Minor -> Minor
[InlineData("4.0.0", Constants.RollForwardSetting.LatestMajor, "4.0.0", Constants.RollForwardSetting.Minor, null)]
// LatestMinor + LatestPatch -> LatestPatch
[InlineData("5.1.0", Constants.RollForwardSetting.LatestMinor, "5.1.0", Constants.RollForwardSetting.LatestPatch, "5.1.3")]
[InlineData("5.0.0", Constants.RollForwardSetting.LatestMinor, "5.0.0", Constants.RollForwardSetting.LatestPatch, null)]
// LatestMajor + LatestPatch -> LatestPatch
[InlineData("5.1.0", Constants.RollForwardSetting.LatestMajor, "5.1.0", Constants.RollForwardSetting.LatestPatch, "5.1.3")]
[InlineData("5.0.0", Constants.RollForwardSetting.LatestMajor, "5.0.0", Constants.RollForwardSetting.LatestPatch, null)]
public void ReconcileFrameworkReferences_MergeRollForward(
string appVersionReference,
string appRollForward,
string fxVersionReference,
string fxRollForward,
string resolvedFramework)
{
CommandResult result = RunTest(
runtimeConfig => runtimeConfig
.WithFramework(new RuntimeConfig.Framework(MicrosoftNETCoreApp, appVersionReference)
.WithRollForward(appRollForward))
.WithFramework(MiddleWare, "2.1.0"),
dotnetCustomizer => dotnetCustomizer.Framework(MiddleWare).RuntimeConfig(runtimeConfig =>
runtimeConfig.GetFramework(MicrosoftNETCoreApp)
.WithRollForward(fxRollForward)
.Version = fxVersionReference));

if (resolvedFramework == null)
{
result.ShouldFailToFindCompatibleFrameworkVersion();
}
else
{
result.ShouldHaveResolvedFramework(MicrosoftNETCoreApp, resolvedFramework);
}

result = RunTest(
runtimeConfig => runtimeConfig
.WithFramework(new RuntimeConfig.Framework(MicrosoftNETCoreApp, fxVersionReference)
.WithRollForward(fxRollForward))
.WithFramework(MiddleWare, "2.1.0"),
dotnetCustomizer => dotnetCustomizer.Framework(MiddleWare).RuntimeConfig(runtimeConfig =>
runtimeConfig.GetFramework(MicrosoftNETCoreApp)
.WithRollForward(appRollForward)
.Version = appVersionReference));

if (resolvedFramework == null)
{
result.ShouldFailToFindCompatibleFrameworkVersion();
}
else
{
result.ShouldHaveResolvedFramework(MicrosoftNETCoreApp, resolvedFramework);
}
}

private CommandResult RunTest(
Func<RuntimeConfig, RuntimeConfig> runtimeConfig,
Action<DotNetCliExtensions.DotNetCliCustomizer> customizeDotNet = null,
Expand Down

0 comments on commit 26ba328

Please sign in to comment.