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

--fx-version argument to dotnet exec should apply assembly conflict resolution #3166

Closed
dsplaisted opened this issue May 11, 2018 · 10 comments
Closed
Labels
area-Host discussion enhancement Product code improvement that does NOT require public API changes/additions
Milestone

Comments

@dsplaisted
Copy link
Member

Steps to reproduce

  • Publish an app targeting .NET Core 2.0 which references System.Memory version 4.5.0-rc1.
  • Run the published DLL on .NET Core 2.1 with a command such as dotnet exec --fx-version 2.1.0-rtm-26510-03 path\to\app.dll.

Expected behavior

System.Memory.dll is loaded from the framework folder

Actual behavior

System.Memory.dll is loaded from the publish folder

Justification

If the app were run normally but there was not an available .NET Core 2.0 runtime, it would roll forward to .NET Core 2.1, and the assembly conflict resolution would choose the higher version from the framework folder.

Having the --fx-version argument respect the conflict resolution policy would enable testing an app rolled forward to a newer runtime without having to delete the runtime it targets from a machine. It would also make it a lot easier for us to have automated test coverage of the roll-forward behavior.

Repro project

<Project Sdk="Microsoft.NET.Sdk">
  <PropertyGroup>
    <OutputType>Exe</OutputType>
    <TargetFramework>netcoreapp2.0</TargetFramework>
  </PropertyGroup>
  <ItemGroup>
    <PackageReference Include="System.Memory" Version="4.5.0-rc1" />
  </ItemGroup>
</Project>
using System;

static class Program
{
    public static void Main()
    {
        Console.WriteLine(typeof(object).Assembly.Location);
        Console.WriteLine(typeof(System.Collections.Immutable.ImmutableList).Assembly.Location);
        Console.WriteLine(typeof(System.MemoryExtensions).Assembly.Location);
    }
}

@steveharter @weshaggard

@steveharter
Copy link
Member

steveharter commented Jun 5, 2018

This is by design. We would need to add another flag (e.g. --fx-version-minimum) to prevent breaks.

@steveharter
Copy link
Member

An alternative of creating a new flag is to use the latest Semantic Versioning version ranges and wildcards

Although this could be a breaking change for some cases, doing this in a major release (3.0) may be acceptable. A version reference such as "1.0.0" would imply >= 1.0.0 (inclusive), so to get only that single version would require "[1.0.0]"

If we go down this path, however, it would be inconsistent with other areas that we provide version numbers (runtimeconfig.json, deps.json, etc).

@steveharter
Copy link
Member

steveharter commented Jul 23, 2018

Another alternative is to support a combination of --roll-forward-on-no-candidate-fx and --fx-version

The default value of --roll-forward-on-no-candidate-fx is 1 which means roll-forward to the closest Minor and Patch if the version specified does not exist locally. However, today when --fx-version is specified, it essentially forces --roll-forward-on-no-candidate-fx=0 (off) but it also implies applyPatches=false.

However applyPatches only exists as a runtimeconfig.json setting, not a command-line argument. Thus to be complete we would want an --apply-patches command line option. ApplyPatches=true means once the candidate framework is found (based on rollForwardOnNoCandidateFx), then apply the latest patch.

Thus the full feature for this alternative:

  • Add an --apply-patches bool command-line argument
  • If --fx-version is specified, then assume --apply-patches=false and --roll-forward-on-no-candidate-fx=0 like we do today. However these can now be overridden by the corresponding explicit values of --apply-patches and --roll-forward-on-no-candidate-fx.

Example:
dotnet exec --apply-patches true --roll-forward-on-no-candidate-fx 1 --fx-version 2.1.0-rtm-26510-03 path\to\app.dll

@dsplaisted
Copy link
Member Author

This is by design. We would need to add another flag (e.g. --fx-version-minimum) to prevent breaks.

@steveharter I'm not really suggesting a "minimum version" flag for the runtime, or any changes to the roll-forward behavior, etc. The issue here is that specifying a runtime version stops an app being able to ship an updated version of a DLL that's part of the runtime. If I specify --fx-version 2.1.0, I would expect to get the same behavior as if I hadn't specified the argument but 2.1.0 was the only version installed on the machine. This is not the case.

I don't know how much the --fx-version parameter is actually used, but this doesn't seem like it has a high risk of breaking people.

@steveharter
Copy link
Member

The framework version value in the runtimeconfig.json is a "minimum version" by default. It can be greater depending on patch and minor roll forward.

The original reason for --fx-version was designed and documented to just load that version and only that version, so I'm not sure who depends on it.

@nguerrera
Copy link
Contributor

There still seems to be confusion here. Daniel isn't proposing any change to "use this version and only this version", but saying that the conflict resolution must pick the appropriate assembly for an app running on that version. The conflict resolution shouldn't depend on how an app ended up running on a given framework, the newest assembly between app and current framework should win regardless. We had a similar conversation about this for patch roll forward. Where did that land?

@nguerrera nguerrera changed the title --fx-version argument to dotnet exec should apply roll-forward policy --fx-version argument to dotnet exec should apply assembly conflict resolution Feb 2, 2019
@vitek-karas
Copy link
Member

I'll have to investigate, but AFAIK the code to resolve per-file versions runs after all of the frameworks and versions are resolved. So I would expect it to work the same regardless how we determined which framework to use. That is to say, I would expect this to already be fixed.

@jeffschwMSFT
Copy link
Member

@vitek-karas can you check if this has been addressed?

@vitek-karas
Copy link
Member

This should work just fine. I tried to repro this using the repro project above:

  • running with with defauls (no --fx-version) - this runs on framework 2.0.9 and does pick up System.Memory from NuGet cache (expected as that is the one referenced by the app and the framework doesn't have this assembly in version 2.0.9)
  • running it (using --fx-version) on 2.1.6 and also 3.0.0-preview6... will in both cases use the specified framework and pick up the System.Memory from the framework since it's a higher version 4.1.0.0 compared to the one in the NuGet package 4.0.1.0.

AFAIK this is the correct behavior.

From an implementation point of view:

  • framework resolution is done by hostfxr which is global on the machine.. and it's the only component which recognizes the --fx-version parameter.
  • the effect of --fx-version is very similar to specifying that particular version in the .runtimeconfig.json of the app and then disabling roll forward on it (that's how it's internally implemented).
  • the assembly resolution is done in hostpolicy, which is per-runtime (ships with the runtime really). This library has no knowledge of --fx-version, so should not be affected by it.

It's possible that the 2.1.0 version originally used to repro this had a bug in hostpolicy which since has been fixed, but I didn't try to track that down.

@nguerrera
Copy link
Contributor

Thanks.

@msftgits msftgits transferred this issue from dotnet/core-setup Jan 30, 2020
@msftgits msftgits added this to the 3.0 milestone Jan 30, 2020
@ghost ghost locked as resolved and limited conversation to collaborators Dec 17, 2020
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
area-Host discussion enhancement Product code improvement that does NOT require public API changes/additions
Projects
None yet
Development

No branches or pull requests

6 participants