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 support for SunOS-like platforms #12198

Merged
merged 1 commit into from
Jul 13, 2020
Merged

Conversation

am11
Copy link
Member

@am11 am11 commented Jun 25, 2020

These are the changes required to cross-compile and use SDK on illumos distors and Oracle Solaris OS.

@am11
Copy link
Member Author

am11 commented Jun 25, 2020

cc @dsplaisted, @eerhardt, like FreeBSD, we can cross-compile SDK for illumos distros with these minimal set of changes. This matches the implementation done in host.

@@ -13,7 +13,9 @@ internal enum Platform
Windows = 1,
Linux = 2,
Darwin = 3,
FreeBSD = 4
FreeBSD = 4,
illumos = 5,
Copy link
Member

Choose a reason for hiding this comment

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

Is this supposed to start with a lower case character?

Copy link
Member Author

Choose a reason for hiding this comment

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

The official casing is lowercase (to disambiguate letters i and lowercase l, like iOS). In the output we print the value as is.

@am11
Copy link
Member Author

am11 commented Jul 10, 2020

If there are no other comments here, can this be merged? Thanks.

@eerhardt eerhardt merged commit 7f9f650 into dotnet:master Jul 13, 2020
@am11 am11 deleted the feature/sunos branch July 13, 2020 18:52
switch (versionDescription)
{
case string version when version.StartsWith("omnios"):
result.Id = "OmniOS";
Copy link
Member

Choose a reason for hiding this comment

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

@am11 @eerhardt Way late, but won't this be a null reference? This was caught by a code scan tool we ran.

Copy link
Member

Choose a reason for hiding this comment

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

Looks like it to me... @am11 thoughts?

Copy link
Member Author

Choose a reason for hiding this comment

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

Ah it will. the testing was blocked by System.Diagnostics.Process port on this platform. I have opened #14395.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants