Skip to content
This repository has been archived by the owner on Jan 23, 2023. It is now read-only.

add basic FreeBSD support #3228

Merged
merged 4 commits into from
Oct 3, 2017
Merged

add basic FreeBSD support #3228

merged 4 commits into from
Oct 3, 2017

Conversation

wfurt
Copy link
Member

@wfurt wfurt commented Sep 19, 2017

This is part for https://github.com/dotnet/corefx/issues/1626

This change adds recognition for FreeBSD and implements function to get OS version.
There should be no impact for other platforms.

./dotnet --info
Runtime Environment:
OS Name: FreeBSD
OS Version: 11.0-RELEASE-p1
OS Platform: FreeBSD
RID: linux-x64
Base Path: /mnt/dotnetcli4/dotnetcli.OK/sdk/2.0.0-preview1-005977/

(RID fixup is not part of this change)

@dnfclas
Copy link

dnfclas commented Sep 19, 2017

@wfurt,
Thanks for having already signed the Contribution License Agreement. Your agreement was validated by .NET Foundation. We will now review your pull request.
Thanks,
.NET Foundation Pull Request Bot

@wfurt wfurt added the os-freebsd FreeBSD OS label Sep 19, 2017
@eerhardt
Copy link
Member

@wfurt - the RID calculation code exists in 2 places - once in C# and once in C++. Since you are changing the C# RID calculation code, you should also change the C++ RID calculation:

https://github.com/dotnet/core-setup/blob/master/src/corehost/common/pal.unix.cpp#L207-L400

@wfurt
Copy link
Member Author

wfurt commented Sep 19, 2017

Thanks for the pointer @eerhardt I'll get that as separate PR.
I'm still not quite sure how one or the other is used.
This change is needed so cli can properly determine file extensions
as fas as RID I did not find direct use yet.

@eerhardt
Copy link
Member

I'll get that as separate PR.

Please don't separate this. These two pieces of logic (managed and native RID calculation) need to remain in sync, it doesn't make sense for one to be updated, without the other.

I'm still not quite sure how one or the other is used.

The managed RID calculation is pretty self-explanatory - anywhere managed code needs to get the current machine's RID, it calls into RuntimeEnvironment.GetRuntimeIdentifier().

The native RID calculation is used by the dotnet host in order to pick the correct native assets at runtime.

It would be ideal if these two places could be shared, but it currently isn't easily possible. The managed code is used outside of .NET Core, so it can't depend on an API that is only available in .NET Core. The native piece is invoked before the CLR is even loaded, so it cannot invoke managed code. This is why we have the calculation in two places.

@@ -101,6 +103,8 @@ private static string GetRIDOS()
return OperatingSystem.ToLowerInvariant();
case Platform.Darwin:
return "osx";
case Platform.FreeBSD:
return "FreeBSD";

This comment was marked as spam.

String verson = RuntimeInformation.OSDescription;
try
{
return RuntimeInformation.OSDescription.Split()[1];

This comment was marked as spam.

This comment was marked as spam.

@wfurt wfurt requested a review from janvorli September 20, 2017 18:55
@wfurt
Copy link
Member Author

wfurt commented Sep 20, 2017

set on rid freebsd.11-x64

else
__rid_plat="linux"
fi
fi
echo __rid_plat=$__rid_plat

This comment was marked as spam.

This comment was marked as spam.

@@ -60,6 +60,8 @@ private static string GetRIDVersion()
return $".{OperatingSystemVersion}";
case Platform.Darwin:
return $".{OperatingSystemVersion}";
case Platform.FreeBSD:
return $".{OperatingSystemVersion.Split('.')[0]}";

This comment was marked as spam.

This comment was marked as spam.

This comment was marked as spam.

This comment was marked as spam.

@@ -214,6 +231,11 @@ private static Platform DetermineOSPlatform()
{
return Platform.Darwin;
}
if (RuntimeInformation.IsOSPlatform(OSPlatform.Create("FREEBSD")))

This comment was marked as spam.

This comment was marked as spam.

This comment was marked as spam.

@@ -60,6 +60,8 @@ private static string GetRIDVersion()
return $".{OperatingSystemVersion}";
case Platform.Darwin:
return $".{OperatingSystemVersion}";
case Platform.FreeBSD:
return $".{OperatingSystemVersion.Split('.')[0]}";

This comment was marked as spam.

if (ret == 0)
{
char *pos = strchr(str,'.');
if (pos) {

This comment was marked as spam.

try
{
// second token up to first dot
return RuntimeInformation.OSDescription.Split()[1].Split('.')[0];

This comment was marked as spam.

This comment was marked as spam.

This comment was marked as spam.

Copy link
Member

@eerhardt eerhardt left a comment

Choose a reason for hiding this comment

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

Looks good. Thanks @wfurt !

@janvorli janvorli merged commit 9dff3ce into dotnet:master Oct 3, 2017
@bartonjs
Copy link
Member

Based on how we do Ubuntu (every numbered release) this should probably include the minor. So on a latest-and-greatest build we'd want it to be freebsd.11.2-x64 (to allow is to insert new assets aligned to a minor release, if needed)

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
os-freebsd FreeBSD OS
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants