Skip to content

OSDescription.Linux: return a user-friendly name based on /etc/os-release.#83976

Merged
stephentoub merged 6 commits into
dotnet:mainfrom
tmds:os_description_pretty
Apr 18, 2023
Merged

OSDescription.Linux: return a user-friendly name based on /etc/os-release.#83976
stephentoub merged 6 commits into
dotnet:mainfrom
tmds:os_description_pretty

Conversation

@tmds

@tmds tmds commented Mar 27, 2023 •
edited
Loading

Copy link
Copy Markdown
Member

@tmds tmds force-pushed the os_description_pretty branch from c21fce1 to 8451453 Compare March 27, 2023 14:36
@ghost ghost added needs-area-label An area label is needed to ensure this gets routed to the appropriate area owners community-contribution Indicates that the PR has been added by a community member labels Mar 27, 2023
teo-tsirpanis
teo-tsirpanis reviewed Mar 27, 2023
View reviewed changes
Comment thread src/libraries/Common/src/Interop/Linux/os-release/Interop.OSReleaseFile.cs Outdated
@build-analysis build-analysis Bot mentioned this pull request Mar 27, 2023
Closed
@MichalPetryka

MichalPetryka commented Mar 27, 2023

Copy link
Copy Markdown
Contributor

Should a fallback based on lsb_release be added here?

@tmds

tmds commented Mar 27, 2023

Copy link
Copy Markdown
Member Author

Should a fallback based on lsb_release be added here?

It can be added in a follow-up PR.
Do you have a specific distro in mind?

@MichalPetryka

MichalPetryka commented Mar 27, 2023

Copy link
Copy Markdown
Contributor

Do you have a specific distro in mind?

I was thinking about Alpine here.

@tmds

tmds commented Mar 27, 2023

Copy link
Copy Markdown
Member Author

I was thinking about Alpine here.

Though it doesn't use systemd, Alpine does include an /etc/os-release file.

@am11

am11 commented Mar 28, 2023

Copy link
Copy Markdown
Member

Fixes #37923.

I don't think this is addressing #37923.

danmoseley
danmoseley reviewed Mar 28, 2023
View reviewed changes
Comment thread src/libraries/Common/tests/Tests/Interop/osReleaseTests.cs Outdated
@tmds

tmds commented Mar 28, 2023

Copy link
Copy Markdown
Member Author

I don't think this is addressing #37923.

ok, I've removed it.

@teo-tsirpanis teo-tsirpanis added area-System.Runtime.InteropServices and removed needs-area-label An area label is needed to ensure this gets routed to the appropriate area owners labels Mar 28, 2023
@ghost

ghost commented Mar 28, 2023

Copy link
Copy Markdown

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

Issue Details

Fixes #83287.

@am11 @tannergooding @danmoseley @stephentoub ptal.

cc @richlander

Author: tmds
Assignees: -
Labels:

area-System.Runtime.InteropServices, community-contribution
Milestone: -

@build-analysis build-analysis Bot mentioned this pull request Mar 28, 2023
Closed
@richlander

richlander commented Mar 28, 2023

Copy link
Copy Markdown
Member

Thanks for taking this on.

omajid
omajid reviewed Mar 29, 2023
View reviewed changes
Comment thread src/libraries/Common/src/Interop/Linux/os-release/Interop.OSReleaseFile.cs
@tmds tmds closed this Apr 13, 2023
@tmds tmds reopened this Apr 13, 2023
@tmds

tmds commented Apr 13, 2023

Copy link
Copy Markdown
Member Author

CI looks good for this. This is up for review.

stephentoub
stephentoub reviewed Apr 13, 2023
View reviewed changes
Comment thread src/libraries/Common/src/Interop/Linux/os-release/Interop.OSReleaseFile.cs Outdated
Comment thread src/libraries/Common/src/Interop/Linux/os-release/Interop.OSReleaseFile.cs Outdated
Comment thread src/libraries/Common/src/Interop/Linux/os-release/Interop.OSReleaseFile.cs Outdated
Comment thread src/libraries/Common/src/Interop/Linux/os-release/Interop.OSReleaseFile.cs Outdated
Comment thread src/libraries/System.Runtime.InteropServices.RuntimeInformation/tests/DescriptionNameTests.cs
tmds and others added 2 commits April 13, 2023 16:11
@tmds

tmds commented Apr 18, 2023

Copy link
Copy Markdown
Member Author

Is this good to merge?

stephentoub
stephentoub reviewed Apr 18, 2023
View reviewed changes
Comment thread src/libraries/Common/src/Interop/Linux/os-release/Interop.OSReleaseFile.cs
}
}

return null;

@stephentoub stephentoub Apr 18, 2023 •
edited
Loading

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Nit: I'd find this easier to read if this were moved up to above the static local function.

stephentoub
stephentoub approved these changes Apr 18, 2023
View reviewed changes

@stephentoub stephentoub left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Thanks

@stephentoub stephentoub merged commit 9672d82 into dotnet:main Apr 18, 2023
@tmds tmds mentioned this pull request Apr 19, 2023
Merged
@ghost ghost locked as resolved and limited conversation to collaborators May 18, 2023
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Reviewers

@danmoseley danmoseley danmoseley left review comments
@stephentoub stephentoub stephentoub approved these changes
+2 more reviewers
@omajid omajid omajid left review comments
@teo-tsirpanis teo-tsirpanis teo-tsirpanis left review comments
Reviewers whose approvals may not affect merge requirements

Assignees

No one assigned

Labels

area-System.Runtime.InteropServices community-contribution Indicates that the PR has been added by a community member

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

RuntimeInformation.OSDescription is really KernelDescription

8 participants

@tmds @MichalPetryka @am11 @richlander @omajid @stephentoub @danmoseley @teo-tsirpanis