Skip to content
New issue

Have a question about this project? # for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “#”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? # to your account

V3 setup-dotnet ADR #291

Merged
merged 6 commits into from
Sep 14, 2022
Merged

V3 setup-dotnet ADR #291

merged 6 commits into from
Sep 14, 2022

Conversation

vsafonkin
Copy link

@vsafonkin vsafonkin commented Apr 29, 2022

This PR outlines the basic proposals for the v3 version of setup-dotnet.

Internal issue: https://github.com/actions/virtual-environments-internal/issues/1807

#171 (comment):

To summarize the current behavior for .NET team review

We have a bunch of .NET Core versions pre-installed on images. And the problem is that action works very inconsistent on different platforms.

  • Ubuntu:
    Pre-installed location (where .NET is pre-installed): “/usr/share/dotnet”
    Action installs to: “/home/runner/.dotnet”
  • Windows:
    Pre-installed location: “C:\Program Files\dotnet”
    Action installs to “C:\Users\runneradmin\AppData\Local\Microsoft\dotnet”
  • MacOS:
    Pre-installed location: “/Users/runner/.dotnet”
    Action installs to “/Users/runner/.dotnet”
    How it works currently on macOS
    Pre-installed location and action location are the same. It means that if we have versions 2.1.x, 3.1.x and 5.0.x pre-installed on image and user will install 3.1 via action, installation will be skipped because version already exists. if customer runs dotnet --version it will report version 5.x if customer doesn't have global.json (because both 2.1, 3.1, 5.0 will be located in the same folder and dotnet will choose the latest one based on https://docs.microsoft.com/en-us/dotnet/core/versions/selection)

How it works currently on Windows / Ubuntu

Pre-installed location and action location are different. It means if we have versions 2.1.x, 3.1.x and 5.0.x pre-installed on image and user will install 3.1 via action, version 3.1 will be installed to the separate location and dotnet --version will report version 3.1 because it is the single version installed in that location.

Problems

Behavior is inconsistent between platforms and it confuses users
macOS users complain that dotnet still uses 5.x despite the fact that they invoked action with 3.1
Windows / Ubuntu users complain that pre-installed .NET are not consumed and it is always installed that takes build time
In our understand, the correct behavior is on macOS right now.
The new versions should be installed in the same location like pre-installed. It is expected behavior that dotnet --version will point the latest one and we should just point customers to documentation.

@vsafonkin vsafonkin requested a review from a team April 29, 2022 09:24
@vsafonkin vsafonkin mentioned this pull request Apr 29, 2022
@vsafonkin vsafonkin changed the title v3 adr V3 setup-dotnet ADR Apr 29, 2022
@BenjaminMichaelis
Copy link

Any updates on the timeline for v3? These fixes of getting the preinstalled versions would be very useful and speed up build times by potentially almost a minute even for fairly simple installs on ubuntu or windows, and considering these speed ups over the course of the hundreds of thousands of builds that depend on this action significantly, there could be many build minutes saved from these issues being resolved.

docs/adrs/v3-setup-dotnet.md Outdated Show resolved Hide resolved
docs/adrs/v3-setup-dotnet.md Outdated Show resolved Hide resolved
docs/adrs/v3-setup-dotnet.md Outdated Show resolved Hide resolved
docs/adrs/v3-setup-dotnet.md Outdated Show resolved Hide resolved
docs/adrs/v3-setup-dotnet.md Outdated Show resolved Hide resolved
docs/adrs/v3-setup-dotnet.md Outdated Show resolved Hide resolved
docs/adrs/v3-setup-dotnet.md Outdated Show resolved Hide resolved
docs/adrs/v3-setup-dotnet.md Outdated Show resolved Hide resolved
docs/adrs/v3-setup-dotnet.md Outdated Show resolved Hide resolved
docs/adrs/v3-setup-dotnet.md Outdated Show resolved Hide resolved
In proposal, the specified version will be installed on machine but the latest version will be used by `dotnet` command by default (because specified version will be installed alongside with pre-installed .NET versions).
Based on [official .NET documentation](https://docs.microsoft.com/en-us/dotnet/core/versions/selection), it is expected behavior and how it works on user's local machine with multiple installed .NET versions but some customers could be broken because they already rely on current behavior.

- All possible inputs will continue to work as previously except `5.x`. It is totally okay to drop it because we shouldn't allow users to rely on such input. The new minor release of .NET will break their builds.
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Is this a consequence of the above bullet point, or is it a separate issue?

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@brcrista, it's a separate issue. Dotnet installer scripts can handle as parameter either full exact version or major.minor version. We're going to use only installer scripts functionality to resolve version without any additional http requests to dotnet releases list, so we are forced to remove an input version format like 5.x.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

So you can't say "give me the latest version of dotnet" anymore? That seems like a serious limitation. Do we know how many workflows would be broken by this change?

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

About 4% of workflow runs with setup-dotnet action have this format (5.x, 6.x and etc) in the last month according Kusto.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Besides breaking those workflows, there's also a design issue. All the other setup- actions accept the full syntax for SemVer version ranges. So excluding 5.x, which is part of that syntax, is a bit confusing.

Even though the code is more complicated, the user experience is paramount. It would be better to continue to look up the latest minor version as we have been. I think wanting to install the latest version of .NET is a perfectly legitimate use case and one we should continue to support.

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@brcrista, I see. Since we can't implement this logic without some damage for customers I removed the part of ADR about it.

* Update ADR proposal

ADR proposal was updated in order to reflect cahnges that will be done
more thoroughly.

* Fix formatting
@marko-zivic-93 marko-zivic-93 merged commit fd4e15f into actions:main Sep 14, 2022
@vsafonkin vsafonkin deleted the v-vsafonkin/v3-adr branch September 14, 2022 18:29
# for free to join this conversation on GitHub. Already have an account? # to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

7 participants