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

Fix corehost RID generation logic for versionless distros #318

Merged
merged 1 commit into from
Dec 11, 2019
Merged

Fix corehost RID generation logic for versionless distros #318

merged 1 commit into from
Dec 11, 2019

Conversation

Saancreed
Copy link
Contributor

The Linux/Bash build script for corehost incorrectly assumes that VERSION_ID will be set after sourcing /etc/os-release, which is not the case on rolling release distributions like Arch Linux. On such systems, calling init_rid_plat initializes base Runtime Identifier to values similar to arch. which ultimately results in malformed RIDs like arch.-x64 and breaks the build process further down the line.

With this change, if VERSION_ID is unset, then the dot will be omitted and final RID will be more correct arch-x64. On distributions with VERSION_ID set in /etc/os-release, this change doesn't affect the RID computation at all.

Applying this fix allows former core-setup repository to correctly build on Arch Linux. Hopefully if this PR is accepted, it will be backported to .NET Core 3.1 too.

(Issue and the fix discovered here, see that issue for more background info.)

Copy link
Member

@ericstj ericstj left a comment

Choose a reason for hiding this comment

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

Looks OK to me, I see we have similar logic here:

if (string.IsNullOrEmpty(OperatingSystemVersion))
{
return string.Empty;
}
return $".{OperatingSystemVersion}";

and here:
if (fFoundVersion)
{
ridOS.append(_X("."));
ridOS.append(valVersionID);
}

@ericstj ericstj merged commit dd60d56 into dotnet:master Dec 11, 2019
@ghost ghost locked as resolved and limited conversation to collaborators Dec 11, 2020
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants