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

Add missing OL releases to RID graph #66783

Closed
wants to merge 1 commit into from

Conversation

Mr-Tao
Copy link

@Mr-Tao Mr-Tao commented Mar 17, 2022

The current RID graph is incomplete concerning recent Oracle Linux releases. This change, which provides missing RIDs for recent OL versions, will greatly alleviate the burden of maintenance of dotnet on OL by removing of need to patch the source on every update.

Associated PRs for dotnet versions supported on OL in respective branches: release/6.0 #66684, release/5.0 #66683 and release/3.1 dotnet/corefx#43138.

@ghost ghost added the community-contribution Indicates that the PR has been added by a community member label Mar 17, 2022
@dnfadmin
Copy link

dnfadmin commented Mar 17, 2022

CLA assistant check
All CLA requirements met.

@omajid
Copy link
Member

omajid commented Apr 6, 2022

I think a better long term approach is to teach the .NET Runtime to treat Oracle Linux "7.x" as just "7", "8.x" as "8", and so on. That's what it does for RHEL.

There's also a proposal to freeze the runtime ids, so this approach of adding more every 6 months is definitely not going to work well in the long term.

@ghost
Copy link

ghost commented Apr 9, 2022

Tagging subscribers to this area: @dotnet/area-infrastructure-libraries
See info in area-owners.md if you want to be subscribed.

Issue Details

The current RID graph is incomplete concerning recent Oracle Linux releases. This change, which provides missing RIDs for recent OL versions, will greatly alleviate the burden of maintenance of dotnet on OL by removing of need to patch the source on every update.

Associated PRs for dotnet versions supported on OL in respective branches: release/6.0 #66684, release/5.0 #66683 and release/3.1 dotnet/corefx#43138.

Author: Mr-Tao
Assignees: -
Labels:

area-Infrastructure-libraries, community-contribution

Milestone: -

@ViktorHofer
Copy link
Member

I think a better long term approach is to teach the .NET Runtime to treat Oracle Linux "7.x" as just "7", "8.x" as "8", and so on. That's what it does for RHEL.

@jkotas @ericstj opinions on that?

@jkotas
Copy link
Member

jkotas commented Apr 11, 2022

a better long term approach

dotnet/designs#260 has proposal and discussion about the long-term approach for RIDs.

@ViktorHofer
Copy link
Member

I think a better long term approach is to teach the .NET Runtime to treat Oracle Linux "7.x" as just "7", "8.x" as "8", and so on. That's what it does for RHEL.

@omajid what would be the right short term resolution in your opinion?

@omajid
Copy link
Member

omajid commented Apr 11, 2022

@omajid what would be the right short term resolution in your opinion?

For main (this PR), do we really want short term solutions? 😄

I am okay with these changes hitting 3.1 though. I left some comments/questions at dotnet/corefx#43138

@ericstj
Copy link
Member

ericstj commented May 9, 2022

cc @tmds

For main (this PR), do we really want short term solutions?

Anything that goes to past releases needs to be present in future releases to avoid regressions when folks move forward. I think something is better than nothing. It's not like we're burning resources that would otherwise be spent realizing the new design. Until that new design is implemented we need to unblock folks with the current design.

I think a better long term approach is to teach the .NET Runtime to treat Oracle Linux "7.x" as just "7", "8.x" as "8", and so on. That's what it does for RHEL.

That makes sense to me, especially since OL derives from rhel. @agocke @vitek-karas thoughts on host changes here?

@omajid
Copy link
Member

omajid commented May 10, 2022

Anything that goes to past releases needs to be present in future releases to avoid regressions when folks move forward.

That's a good point: we can't add something to the RID graph in 6.x only, it needs to be in main too.

I think something is better than nothing. It's not like we're burning resources that would otherwise be spent realizing the new design. Until that new design is implemented we need to unblock folks with the current design.

My only concern with adding the RHEL RIDs (assuming they are required for OL to work) is that it's getting us back to needing a new RHEL RID every RHEL minor release (6 months) which we worked hard to move away from. Not having RHEL minor-version RIDs was also a guarantee that a new RHEL minor version wouldn't suddenly be incompatible with .NET because we forgot to update the RID graph. Adding the minor RIDs back makes it possible that we miss out on regressions - because some new piece of code accidentally used the full RHEL version - until the last minute.

@@ -148,15 +148,20 @@
<RuntimeGroup Include="ol">
<Parent>rhel</Parent>
<Architectures>x64</Architectures>
<Versions>7;7.0;7.1;7.2;7.3;7.4;7.5;7.6</Versions>
<Versions>7;7.0;7.1;7.2;7.3;7.4;7.5;7.6;7.7;7.8;7.9;7.10</Versions>
Copy link
Member

Choose a reason for hiding this comment

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

https://access.redhat.com/documentation/en-us/red_hat_enterprise_linux/7/html/7.9_release_notes/overview

Red Hat Enterprise Linux 7.9 is the last minor release of RHEL 7.

We shouldn't need to add 7.10 here.

@@ -216,17 +221,17 @@
<RuntimeGroup Include="rhel">
<Parent>linux</Parent>
<Architectures>x64</Architectures>
<Versions>7;7.0;7.1;7.2;7.3;7.4;7.5;7.6</Versions>
<Versions>7;7.0;7.1;7.2;7.3;7.4;7.5;7.6;7.7;7.8;7.9;7.10</Versions>
Copy link
Member

Choose a reason for hiding this comment

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

Same about 7.10 here.

Copy link
Member

Choose a reason for hiding this comment

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

We we really need to add every minor version here? This is useful only if they are not mutually compatible IMHO. As far as I can see we only track major versions or Centos and Debian.

Copy link
Member

Choose a reason for hiding this comment

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

The right/long term thing to do is not to add every minor version here. We haven't had to add minor versions for RHEL previously, and this just increases the maintenance/servicing burden.

But since we are now adding OL, we need the parent RHEL versions (is this a strict requirement? Can anyone confirm)

Assuming matching RHEL versions are required, we have two main options:

  • Fix everything in the .NET ecosystem (runtime, sdk, build tools) so that it knows that the minor version for Oracle Linux doesn't matter. That's the case for RHEL. Maybe it could be done with greping for rhel and adding ol to those conditions?
  • Add minor versions, expand the RID graph, and deal with keeping it up to date.

Copy link
Member

Choose a reason for hiding this comment

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

since the ol did not have any minor versions, we can keep doing that as well, right? e.g. just add ol 9?

Copy link
Member

Choose a reason for hiding this comment

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

Why would we have to fix everything? We could simply say that the RIDs for ol don't include minor version in them (just like we don't include build numbers of Windows in the Windows RIDs). Modify the host such that it will strip the minor version for ol when computing current RID. The rest of the system should not care in that case.

Copy link
Member

Choose a reason for hiding this comment

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

Why would we have to fix everything?

AFAIK, they are using source-build. Fixing the host will probably be the largest part of the effort, but we will need to update a number of other places like build scripts too.

Copy link
Member

@omajid omajid Jun 30, 2022

Choose a reason for hiding this comment

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

I looked through all of .NET 7 source-build sources and after a quick pass, it looks like these files might also need fixups (it's possible I missed something, though):

runtime/eng/native/init-distro-rid.sh
runtime/src/installer/tests/scripts/linux-test/RuntimeInstallation.sh
runtime/src/installer/tests/scripts/linux-test/SdkInstallation.sh
runtime/src/libraries/Common/tests/TestUtilities/System/PlatformDetection.Unix.cs
runtime/src/libraries/Microsoft.NETCore.Platforms/src/runtimeGroups.props
runtime/src/libraries/Microsoft.NETCore.Platforms/src/runtime.json
runtime/src/libraries/Microsoft.NETCore.Platforms/src/runtime.compatibility.json
runtime/src/native/corehost/hostmisc/pal.unix.cpp
sdk/src/Cli/Microsoft.DotNet.Cli.Utils/RuntimeEnvironment.cs
sdk/src/Tests/Microsoft.NET.TestFramework/EnvironmentInfo.cs
fsharp/scripts/init-tools.sh
fsharp/scripts/dotnet-install.sh
deployment-tools/eng/native/init-distro-rid.sh
deployment-tools/src/clickonce/native/projects/setup.cmake
deployment-tools/src/installer/Directory.Build.props
diagnostics/eng/init-distro-rid.sh
arcade/src/Microsoft.DotNet.XUnitConsoleRunner/src/common/AssemblyResolution/Microsoft.DotNet.PlatformAbstractions/Native/PlatformApis.cs

There are some test files here too.

Copy link
Member

Choose a reason for hiding this comment

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

Also, there's a question about what to do in 3.1 and 6.0. Do we fix up host and other files there too? Otherwise, like discussed earlier in this issue, we would have to add minor versions in 6.0 and then add them here too for the sake of compatiblity.

@ericstj
Copy link
Member

ericstj commented Jun 6, 2022

@Mr-Tao can you address @omajid's feedback?

@ericstj ericstj added the needs-author-action An issue or pull request that requires more info or actions from the author. label Jun 6, 2022
@ghost ghost added the no-recent-activity label Jun 20, 2022
@ghost
Copy link

ghost commented Jun 20, 2022

This pull request has been automatically marked no-recent-activity because it has not had any activity for 14 days. It will be closed if no further activity occurs within 14 more days. Any new comment (by anyone, not necessarily the author) will remove no-recent-activity.

@ghost ghost removed the no-recent-activity label Jun 29, 2022
@ViktorHofer
Copy link
Member

gentle ping, @Mr-Tao

@ghost ghost added the no-recent-activity label Jul 27, 2022
@ghost
Copy link

ghost commented Jul 27, 2022

This pull request has been automatically marked no-recent-activity because it has not had any activity for 14 days. It will be closed if no further activity occurs within 14 more days. Any new comment (by anyone, not necessarily the author) will remove no-recent-activity.

@agocke agocke marked this pull request as draft July 27, 2022 21:14
@ghost ghost removed the no-recent-activity label Jul 27, 2022
@ghost ghost added the no-recent-activity label Aug 11, 2022
@ghost
Copy link

ghost commented Aug 11, 2022

This pull request has been automatically marked no-recent-activity because it has not had any activity for 14 days. It will be closed if no further activity occurs within 14 more days. Any new comment (by anyone, not necessarily the author) will remove no-recent-activity.

@danmoseley
Copy link
Member

Closing. Please reopen @Mr-Tao if and when you would like to continue.

@ghost
Copy link

ghost commented Sep 3, 2022

This pull request has been automatically marked no-recent-activity because it has not had any activity for 14 days. It will be closed if no further activity occurs within 14 more days. Any new comment (by anyone, not necessarily the author) will remove no-recent-activity.

@ghost ghost removed the no-recent-activity label Sep 6, 2022
@carlossanlop
Copy link
Member

Closing. Please reopen @Mr-Tao if and when you would like to continue.

@ghost ghost locked as resolved and limited conversation to collaborators Nov 26, 2022
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
area-Infrastructure-libraries community-contribution Indicates that the PR has been added by a community member needs-author-action An issue or pull request that requires more info or actions from the author.
Projects
None yet
Development

Successfully merging this pull request may close these issues.