Sdk: Allow the --roll-forward switch to be used with local tools

[ViktorHofer]

The runtime-option --roll-forward is settable when using the exec SDK command but currently not when invoking a repo tool with dotnet tool run. If a repo tool doesn't define a roll-forward policy then there's no way to make a tool run on a higher shared framework version.

I'm not up-to-date on the decisions being made around repo tools but naively I would expect that option to be usable with repo tools.

cc @KathleenDollard @vitek-karas @wli3

[wli3]

1. @vitek-karas do we want to support this option? I remember there is no such flag for apphost. That means global tools do not support --roll-forward. We don't need to support that for local tools. And setting environment variable does work.

2. and it is challenging to come up with a clear UX to support that. dotnet tool run --roll-forward Major dotnetsay 'hi' should be short and close to host's syntax. But that is ambiguous for runtime arguments --roll-forward Major and the tool name dotnetsay. Unless we say the tool name is the first one without --. I don't think it is reliable. We could use dotnet tool run dotnetsay --rest-is-for-runtime --roll-forward Major -- 'hi'. it is not pretty but not ambiguous. And at the same time I don't think we should support it in the shortcut version "dotnet dotnetsay" @KathleenDollard @nguerrera

[wli3]

add release blocking to make sure to discussed that before the release.

[vitek-karas]

Number 1 from above is not correct. Apphost does support --roll-forward. The way we parse the command line is simple - we take the options we recognize and pass the rest to the app.
I don't know how dotnet tool run is implemented, but it should be possible to run the tool's .exe directly with the --roll-forward parameter.

What I don't understand is why the environment variable DOTNET_ROLL_FORWARD is not enough for these scenarios? Can somebody describe a scenario where using the environment variable won't work?

[wli3]

If so we should have the parity with global tools. @KathleenDollard @ViktorHofer we need to discussion the syntax

[KathleenDollard]

@vitek-karas The environment variable affects all roll-forward doesn't it? We anticipate that people may want to be more aggressive with roll forward on tools than on anything they are running.

[vitek-karas]

The environment variable takes effect if the app doesn't explicitly specify roll forward policy, but yes, it affects anything executed with it. I was relating this to the scenarios which @ViktorHofer probably has in mind which are all around the "in-repo" tools. The "repo" typically has exactly one version in it (also typically something very recent). So when we run things in the repo, we pretty much always want to run them on the in-repo version.

I would definitely not recommend to set this machine wide.

[ViktorHofer]

Exactly. The environment variable of course would work but we don't want to set it for the entire environment ie test script just because of repo tools and their lack of a roll forward strategy.

I was discussing this change with Will offline and I agree with Vitek that runtime-options should just work, ideally with the same Syntax and ordering as with dotnet exec.

[wli3]

@ViktorHofer @nguerrera @dsplaisted @dotnet/dotnet-cli

Talked to @KathleenDollard offline. We think moving it to tool manifest might be a good idea. Repo owner should have more power than the producer to determine which runtime it should run on. At the same time, it can reduce the pain of the initial update of 3.0 incompact, when all 2.x tools cannot run on 3.0 runtime at the beginning. The repo own can set these params to allow existing tools to be run.

{
  "version": 1,
  "isRoot": true,
  "tools": {
    "dotnetsay": {
      "version": "2.1.4",
      "commands": [
        "dotnetsay"
      ],
      "runtimeArguments" : [
        "--roll-forward", "Major"
      ]
    }
  }
}

[richlander]

If we're going to enable roll-forward to the manifest, we should first look at modeling it in the same way as runtimeconfig.json. Was that already rejected? Are there other runtime arguments you envision?

One option is to use runtimeconfig.json syntax in this manifest and then enable updating global tools runtimeconfig.json in the same way.

[richlander]

There is also a question of whether users want to set this value for every single tool or would like an option for the whole manifest.

[wli3]

Good point, it should be close to runtimeconfig.json syntax, instead of commandline. However, that will require sdk to write a file every time, or some “translations ” in SDK layer — that would be a circular dependency from sdk to runtime. To avoid it all together, maybe back to command line instead of manifest file would be better.

[richlander]

Can you elaborate? Why would a file need to be written? Yes, I was thinking that the SDK would know how to translate that JSON syntax into the correct commandline arg. That seems reasonable to me and a good model for users, instead of needing to learn two different models for the same thing. No?

[KathleenDollard]

@richlander It may be a friction point to have the arguments that would appear on the command line in the manifest file, but is it learning a second model? Assuming the values here would be the same ones you'd pass if we add a --tool-runtime-rollforward and we name it something that clearly indicates they are arguments.

[richlander]

It's just a question on whether the proposed solution is the best one. As a starting point, offering two ways of doing roll-forward for apps does not seem like the best approach. I'd like to see a good reason to depart from the model we already have in runtimeconfig.json before we propose another model. I'm looking for a user-centered reason not "implementation complexity".

[nguerrera]

Apphost does support --roll-forward. The way we parse the command line is simple - we take the options we recognize and pass the rest to the app.

I just learned this, and I'm concerned by it, to be honest. I would not expect that my exe gains special command line args if I port from .NET Framework to .NET Core. Or really anything other than what my Main(string[] args) handles even if I didn't port from .NET Framework. We will need to be careful about any args we add that might conflict with the app, which would be a breaking change.

[wli3]

Apphost does support --roll-forward. The way we parse the command line is simple - we take the options we recognize and pass the rest to the app.

@vitek-karas
From what I tried, these params need be in front of the dll path. Is that right?

[richlander]

@nguerrera That's a very valid concern. Some scenarios use -- for these kind of things. That's not an intuitive pattern for many people, but maybe that's fine for scenarios like this and avoids breaking changes.

For clarity, this concern is logically separate from my concern.

[nguerrera]

For clarity, this concern is logically separate from my concern.

Yes, I wish there was threading support on issue discussion.

[nguerrera]

Some scenarios use -- for these kind of things. That's not an intuitive pattern for many people, but maybe that's fine for scenarios like this and avoids breaking changes

The thing is that if I'm calling myapp.exe, we can't use -- either for this concern. myapp's Main can interpret that too. There's no good position in invocation of myapp.exe to mean args for the runtime, not my app. We would need to pick something that is obscure enough to reserve for runtime args.

[vitek-karas]

Well - we already shipped with the runtime options on apphost.exe command line, and we actually support quite a few there. The interesting ones are --additionalProbingPath, --additionalDeps and now the --rollForward.
We could change that I guess since apphost.exe is really only a thing since 2.2 and even then it was rather obscure (outside of global tools).

@wli3 For dotnet.exe the runtime options must come before the app.dll. Anything after the app.dll is passed to the app.

For apphost.exe what I wrote above is slightly misleading. The parsing of the command line is actually relatively simple:

• We have a set of arguments we recognize.
• We go from left to right, once we find something we don't recognize we stop
• Everything after this point is passed to the app as is

So in reality the runtime arguments must always come first and must not be "interleaved" with anything else.

[richlander]

There are a few separate questions here ...

Roll forward for tools

1. How should we expose roll-forward to tools? Via the commandline and/or the existing tools manifest?
2. Should we expose a CLI gesture for enabling roll-forward for tools or require manifest editing?
3. Should we expose roll-forward for just local tools (since they have a manifest) or both local and globally installed tools?

Runtime command-line arguments

1. Should we expose all runtime config options (what's exposed in the JSON file) as command-line args (or is that already the case)?
2. Should we expose a more formal scheme for separating runtime from app command-line arguments. This is somewhat similar to grep -- (see section 2.1.7 @ grep man page).

In response to @vitek-karas ... that sounds good. The only challenge is if we have a CLI argument that could overlap with an app and it is the first one in the (intended for) app argument list.

[KathleenDollard]

@nguerrera Can we get the link to the --roll-forward effect on apps in general?

@vitek-karas @mairaw Do we have this documented for customers?

I don't know if it would be worth changing, but I think the chance of colliding with an application switch would be signficantly reduced if we called ig --fx-roll-forward.

[KathleenDollard]

@richlander I didn't see what you intended in the grep link

[nguerrera]

@nguerrera Can we get the link to the --roll-forward effect on apps in general?

Do you mean a link to a separate issue on the potential for app/runtime command line arg conflict? I said I'd file one yesterday evening, but haven't gotten to it yet this morning. Or do you want something else from me here?

[nguerrera]

I filed https://github.com/dotnet/core-setup/issues/7234

[wli3]

a meta conclusion. we find this change would require more discussion to avoid paint us into a corner in the future and it depends on dotnet/core-setup#7234

we will not rush it to make it into 3.0.100, so i'll remove the blocking release tag

[KathleenDollard]

@nguerrera Yep, just realized I was continuing to add to that discussion here.

[richlander]

@KathleenDollard -- grep has a scheme for separating one form of command-line input from another with --. That's what the man page doc says that I linked to. It is similar to the -- as supported by dotnet run. If we want to make the separation formal, we need a similar form of delimiter.

[richlander]

I think the chance of colliding with an application switch would be signficantly reduced if we called it --fx-roll-forward.

I agree in theory, however, that devolves into "make the runtime switches more obscure and less usable." That seems like a bad tradeoff and evidence of a missing capability.

Also, the runtime switches are intended to map directly to runtime config names, which doesn't require this kind of namespacing. It doesn't make sense to change our config naming scheme because of a weak commandline model.

[nguerrera]

Let's move the discussion on the collision aspect to dotnet/core-setup#7234

[richlander]

I'm going to re-hydrate half of what I wrote earlier now that we have moved the CLI arguments topic out. Here:

1. How should we expose roll-forward to tools? Via the commandline and/or the existing tools manifest?
2. Should we expose a CLI gesture for enabling roll-forward for tools or require manifest editing?
3. Should we expose roll-forward for just local tools (since they have a manifest) or both local and globally installed tools?

Here is my take, in priority order:

• Both globally and locally installed tools need to enable roll-forward via a CLI gesture (on launch).
• It's OK if only locally installed tools have the option to persist this choice in a manifest file. This would be an advantage of local tools. There needs to be a way to update the manifest file to declare roll-forward via a CLI gesture.

Thoughts?

[richlander]

I was thinking about this a bit more while I was riding my bike. I think we still have a bit of a problem.

This scenario is clear:

dotnet --roll-forward foo.dll

It may be less clear with local tools:

dotnet --roll-forward locally-installed-tool

The issue is that --roll-forward is a directive to dotnet about which runtime to load, however local tools is a CLI concept and the CLI needs to load on a runtime before it can even think about reading the manifest file. The --roll-forward directive in this case isn’t for the dotnet host but for the CLI to use when it asks the dotnet host (or another part of the CLI) to load the local tool. Is this a non-issue or is it indeed messy?

[wli3]

There needs to be a way to update the manifest file to declare roll-forward via a CLI gesture.

I don't think that is required. Ideally we don't want every users to think about runtime -- what the runtime the tools wants, what runtime the user has installed and what runtime is safe to use. But once the user has to consider it, it is pretty much an expert scenario. And without a CLI gesture is fine. I think most people facing this problem will just install a new runtime.

Is this a non-issue or is it indeed messy?

This is the ambiguous issue I mentioned above. So we might have to say something like dotnet tool run dotnetsay --rest-is-for-runtime --roll-forward Major -- 'hi' . And we think pass via command line would be hard to use

[richlander]

But once the user has to consider it, it is pretty much an expert scenario

I don't think so.

We need to give guidance to tools authors. It can be any of:

• Multi-target your tools so that compatible versions are available [My least favorite option].
• Set <RollForward>Major</RollForward> in the tool project file [My second least favorite option].
• Tell your users to roll-forward your tool via CLI or config (for local tools).
• Tell your users to install the target runtime, either proactively or when they see an error.

There are at least two scenarios where installing the target runtime won't be desirable:

• build scripts (they will want to limit the build to using one runtime and sdk if at all possible).
• The target runtime is EOL.

Those don't sound like expert scenarios to me.

So we might have to say something like dotnet tool run dotnetsay --rest-is-for-runtime --roll-forward Major -- 'hi'

That isn't acceptable CLI syntax, IMO. Everything after dotnetsay has to be for the app. Also, we cannot steal the -- from the app. Something closer to:

dotnet tool run -roll-forward Major dotnetsay 'hi'

That said, I don't want to require dotnet tool run for this case. I also want to see side-by-side syntax for globally- and locally-installed tools.

[wli3]

Tell your users to roll-forward your tool via CLI or config (for local tools).
Tell your users to install the target runtime, either proactively or when they see an error.

This sounds very complicated. "We tell producer to tell consumer..." Also, the consumer still need to understand the compatibility matrix. I don't think it is a long term solution and we should not spend much effort to "patch" this route.

dotnet tool run -roll-forward Major dotnetsay 'hi' it looks good. But in technical side we don't have a good way to tell dotnetsay is not a runtime argument. Currently our parsing is stateless. When if we do so to scan available tools first. We would need much more effort to rebuild the error message system to deal with parsing command line input twice. (first parse to know we are in dotnet tool run second is after finding the tool name). For example how to give a understandable error message of the following command? dotnet tool run -roll-forward dotnetsay Major 'hi'

[richlander]

This sounds very complicated. "We tell producer to tell consumer..." Also, the consumer still need to understand the compatibility matrix. I don't think it is a long term solution and we should not spend much effort to "patch" this route.

Unfortunately, we need to. If .NET Core tools become a large ecosystem, then users will hit this problem every day.

Another perfect example is that .NET Core 2.x isn't supported on ARM64 so there is no option to install .NET Core 2.x on those platforms; roll-forward is the only option.

For example how to give a understandable error message of the following command? dotnet tool run -roll-forward dotnetsay Major 'hi'

Good question. What error condition did you have in mind?