core
core copied to clipboard
Add new release labels
Apply new labels to release-index.json
and release.json
.
This PR will be merged on July 13th.
Context: https://github.com/dotnet/designs/pull/265
We cannot change the existing support-phase
property, due to compatibility. It will go through the following progression:
preview
-> go-live
-> [lts
| sts
] -> maintenance
-> eol
The new release-type
property will be set as either lts
or sts
and will not change, even after EOL.
When release-type
== support-phase
, that means "active" of "full" support. All other states are self-descriptive. That's a bit wonky and not how we would have designed if we were starting from scratch, but we're not. This approach seems pretty workable.
The spec suggests a string array of labels. On further discussion, that doesn't seem necessary. This simpler approach seems better and more efficient for readers. The only label we're missing is latest
. It doesn't seem warranted to add another property to account for that.
This change is a breaking change since it switches to sts
instead of current
. sts
is "short-term support" while lts
is "long-term support". This breaking change won't be observable until .NET 7 is GA and support-phase
== sts
instead of current
.
I updated all the releases in release-index.json
to make it internally consistent, but then only updated the in-support releases for release.json
. That's definitely up for discussion. Seemed pragmatic.
@mairaw
@mairaw can we test this for dot.net to ensure this doesn’t break it?
We know that this change will break the website (mostly due to the same issue as https://github.com/dotnet/core/pull/7497). @mairaw would like to test this change, as you suggest, to see if there are other issues.
If we have other known contacts who use these files, now would be a good time to contact them.
The new property doesn't cause any issues on our ingestion process. And I just created a PR to not rely on eol-date being null to determine support phase, which can be merged today. With that change merged, we could even add the .NET 7 EOL dates back in.
With that change merged, we could even add the .NET 7 EOL dates back in.
This PR does that. We can wait to merge this PR or unrevert the other one. Might as well wait and then we can tell people about the changes all at once.
Yep, either way is fine.
I'll leave that up to you. Whichever approach you prefer.
Are you planning on adding the release-type to older releases.json files, e.g. 1.0 or 2.2? If not, what should be the default value when reading these files for the property? We have use cases for processing these files and I want to make sure the releases API handles this in a consistent fashion. I could also consider an undefined enum.
I updated all the release objects in release-index.json
. I didn't update the old release.json
files in absence of a reason to do so. If you have a scenario where that would be helpful, I'm willing to do it. It won't take me very long. Should I?
I updated all the release objects in
release-index.json
. I didn't update the oldrelease.json
files in absence of a reason to do so. If you have a scenario where that would be helpful, I'm willing to do it. It won't take me very long. Should I?
That would be great, unless you have a suggestion for assigning a default value when the release-type
is absent in the JSON - I'd be open to that as well.
I'll start working on the PR for the website now. @richlander do you want to handle the merge conflicts here?
Happy to deal with the merge conflicts. However, I think I promised that I wouldn't merge these files until 7/13 (or some similar date). I'd rather wait until July patch Tuesday. Fair?
Of course, no problem!
I just dealt with the merge conflicts since I also had to add the missing property to all other versions. And when would we add the changes to the support-phase property? I just need to account for the interim solution with the current values.
Thank @mairaw!
Do we have tests or any validation for install scripts? There are args that expect Channel that will now be changing (https://docs.microsoft.com/en-us/dotnet/core/tools/dotnet-install-script#options)
now changing
I assume you mean "not changing"
Current - Most current release.
Looks like that can still be satisfied w/o issue. I'm not sure if what it says is what it does. Good question.
Maybe @YuliiaKovalova and @dotnet/install-scripts-maintainers can tell us.
Yes @richlander I'm thinking to make sure things like -Channel sts
would be expected to work...I think it will work based on my 2m reading of the code, but might need to just expand test validation/coverage of adding the new labels.
Ah. I see now. I'm not sure there is a big scenario for that. I think the following scenarios make sense:
- Latest release
- Latest LTS
- this x.y release
For example, what would sts
return now? Seems like an anti-pattern. The logical labels should only exist if they are guaranteed to always return an in-support release. Fair?
I think today sts
would error because it would try to construct an aka link that doesn't exist. https://github.com/dotnet/install-scripts/blob/main/src/dotnet-install.ps1#L907 Maybe a non-issue, but would be good to make sure things work as expected. There is 'Current' in places in the code and tests...probably need expansion.
I believe that @MattGal and @mmitche were involved in setting up those feeds. Is "Current" the same as latest?
If so, we may want to undocument that and (compatibly) rename it to "Latest".
Tagging @bekir-ozturk also directly.
Hi everyone,
If I understand the potential changes correctly, we don't need to apply any modifications to Channel
, it promises to continue working without problems, like:
but we will need to modify existing test cases + documentation for sure.
If I understand the potential changes correctly, we don't need to apply any modifications to
Channel
, it promises to continue working without problemsbut we will need to modify existing test cases + documentation for sure.
Thank you @YuliiaKovalova for confirming!
I just validated that Current
work correct.
rich@MacBook-Air-2 ~ % ./dotnet-install.sh -Channel Current -Verbose
dotnet-install: Note that the intended use of this script is for Continuous Integration (CI) scenarios, where:
dotnet-install: - The SDK needs to be installed without user interaction and without admin rights.
dotnet-install: - The SDK installation doesn't need to persist across multiple CI runs.
dotnet-install: To set up a development environment or to run apps, use installers rather than this script. Visit https://dotnet.microsoft.com/download to get the installer.
dotnet-install: Calling: machine_has curl
dotnet-install: Calling: calculate_vars
dotnet-install: Calling: get_normalized_architecture_from_architecture <auto>
dotnet-install: Calling: get_machine_architecture
dotnet-install: Normalized architecture: 'arm64'.
dotnet-install: Calling: get_normalized_os
dotnet-install: Calling: get_current_os_name
dotnet-install: Normalized OS: 'osx'.
dotnet-install: Calling: get_normalized_quality
dotnet-install: Normalized quality: ''.
dotnet-install: Calling: get_normalized_channel Current
dotnet-install: Normalized channel: 'current'.
dotnet-install: Calling: get_normalized_product
dotnet-install: Normalized product: 'dotnet-sdk'.
dotnet-install: Calling: resolve_installation_path <auto>
dotnet-install: Calling: get_user_install_path
dotnet-install: resolve_installation_path: user_install_path=/Users/rich/.dotnet
dotnet-install: InstallRoot: '/Users/rich/.dotnet'.
dotnet-install: Calling: get_download_link_from_aka_ms
dotnet-install: Retrieving primary payload URL from aka.ms for channel: 'current', quality: '', product: 'dotnet-sdk', os: 'osx', architecture: 'arm64'.
dotnet-install: Constructed aka.ms link: 'https://aka.ms/dotnet/current/dotnet-sdk-osx-arm64.tar.gz'.
dotnet-install: Calling: get_http_header https://aka.ms/dotnet/current/dotnet-sdk-osx-arm64.tar.gz true
dotnet-install: Calling: machine_has curl
dotnet-install: Calling: get_http_header_curl https://aka.ms/dotnet/current/dotnet-sdk-osx-arm64.tar.gz true
dotnet-install: Received response: HTTP/1.1 301 Moved Permanently
Server: Kestrel
Location: https://aka.ms/dotnet/6.0/dotnet-sdk-osx-arm64.tar.gz
Request-Context: appId=cid-v1:9b037ab9-fa5a-4c09-81bd-41ffa859f01e
X-Response-Cache-Status: True
Content-Length: 0
Expires: Wed, 29 Jun 2022 21:23:46 GMT
Cache-Control: max-age=0, no-cache, no-store
Pragma: no-cache
Date: Wed, 29 Jun 2022 21:23:46 GMT
Connection: keep-alive
Strict-Transport-Security: max-age=31536000 ; includeSubDomains
HTTP/1.1 301 Moved Permanently
Server: Kestrel
Location: https://dotnetcli.azureedge.net/dotnet/Sdk/6.0.301/dotnet-sdk-6.0.301-osx-arm64.tar.gz
Looks like we're good to go.
As I understand it, the label Current
works as expected with install scripts (installs .NET 6.0) while it undesirably refers to 5.0 in VS.
@richlander already mentioned that this will be a breaking change, but I wanted to double check that we are on the same page about what exactly will be broken. So, here are some questions that popped into my head:
- Does removal of
Current
label also mean the removal ofCurrent
channel? @mmitche @adiaaida -
Current
allows the users of install scripts to say "give me whatever is the latest GA version, whether it is LTS or not" which (warning: speculation ahead!) may be a common scenario for people wanting to stay up to date (end of speculation). If the channelCurrent
will indeed be removed, can we say that "this is one of the breaking changes we are introducing"? What alternative can we offer for these users? - As we are updating install scripts documentation, do we want to add
sts
to the list of supported channels? Agreeing with @richlander above, it may be an anti-pattern since from the context of the scripts it will mean "give me the latest GA build, but make sure it is one of those that will expire soon". I'm unsure whether the benefits of having ansts
channel in scripts will worth the maintenance costs as well as the potential user confusion.
- Does removal of
Current
label also mean the removal ofCurrent
channel? @mmitche @adiaaida
Current
in terms of the concept that users can dotnet-install --version current (ie, the aka.ms links) seem like they would be unaffected by this change. Creating the current
links is a manually run pipeline that we do once a release (more or less on GA release day).
I'm unsure whether the benefits of having an
sts
channel in scripts will worth the maintenance costs as well as the potential user confusion.
If we add sts
to dotnet-install we will need to update a bunch of stuff in other places. Please ping me if we decide to do this.
Let's just stay as-is. I will write up some docs and probably a blog post on our updated approach. I will review it with you folks to ensure everything is correct.
We could change "Current" to "Latest" but that seems like a lot of churn for little benefit. I am NOT proposing that.
@jamshedd I resolved all the comments because changing those is no longer the plan of record. Please review again.
These changes should be included with all other releases.json changes for GA. We won't be able to handle different changes coming at different PRs for the website (or we'll need to skip some deployments).