Skip to content

Conversation

@trungnt2910
Copy link
Contributor

This contains the code required to build the first managed runtime libraries for Haiku, namely System.Private.CoreLib.

Part of #55803.

Copilot AI review requested due to automatic review settings November 21, 2025 14:26
@github-actions github-actions bot added the needs-area-label An area label is needed to ensure this gets routed to the appropriate area owners label Nov 21, 2025
@dotnet-policy-service dotnet-policy-service bot added the community-contribution Indicates that the PR has been added by a community member label Nov 21, 2025
Copilot finished reviewing on behalf of trungnt2910 November 21, 2025 14:31
This contains the code required to build the first managed runtime
libraries for Haiku, namely `System.Private.CoreLib`.

Co-authored-by: Jessica Hamilton <[email protected]>
@trungnt2910 trungnt2910 force-pushed the dev/trungnt2910/haiku-lib-corelib branch from 540d8bb to f20b31c Compare November 21, 2025 14:34
Copy link
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull request overview

This PR adds initial managed libraries support for Haiku OS by implementing the necessary interop and environment code for System.Private.CoreLib. The changes enable basic Haiku platform detection and working set memory queries.

  • Adds Haiku OS platform detection with TARGET_HAIKU constant
  • Implements WorkingSet property for Haiku using native area_info API
  • Provides comprehensive Haiku OS interop definitions (teams, threads, areas, system info)

Reviewed changes

Copilot reviewed 5 out of 5 changed files in this pull request and generated no comments.

Show a summary per file
File Description
src/libraries/System.Private.CoreLib/src/System/OperatingSystem.cs Adds TARGET_HAIKU constant to platform name detection
src/libraries/System.Private.CoreLib/src/System/Environment.Haiku.cs Implements WorkingSet property for Haiku by iterating process memory areas
src/libraries/System.Private.CoreLib/src/System.Private.CoreLib.Shared.projitems Configures conditional compilation for Haiku and includes required interop files
src/libraries/Common/src/Interop/Haiku/Interop.OS.cs Defines comprehensive Haiku OS interop structures, enums, and P/Invoke methods for process/thread/memory management
src/libraries/Common/src/Interop/Haiku/Interop.Libraries.cs Defines libroot library constant for Haiku interop
Comments suppressed due to low confidence (3)

src/libraries/Common/src/Interop/Haiku/Interop.OS.cs:209

  • The parameter documentation is incorrect. The who parameter is of type BTeamUsage (an enum specifying self or children), not "The thread ID". It should describe that this parameter specifies whether to get usage information for the team itself or its children.
    src/libraries/Common/src/Interop/Haiku/Interop.OS.cs:252
  • The parameter documentation should use <see cref="system_info"/> instead of just "system_info" for consistency with other parameter documentation in this file (e.g., lines 159, 170, 181, 210, 230, 242).
    src/libraries/Common/src/Interop/Haiku/Interop.OS.cs:86
  • The enum name thread_state uses snake_case which is inconsistent with C# naming conventions and the pattern used by other enums in this file (BTeamUsage, BPriority). It should be renamed to ThreadState to follow PascalCase naming convention.

@am11 am11 added area-System.Runtime os-haiku and removed needs-area-label An area label is needed to ensure this gets routed to the appropriate area owners labels Nov 21, 2025
@trungnt2910
Copy link
Contributor Author

C/c @am11

Haiku's System.Diagnostics.Process depends on this so I will open that one later.

@dotnet-policy-service
Copy link
Contributor

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

@am11
Copy link
Member

am11 commented Nov 21, 2025

Haiku's System.Diagnostics.Process depends on this so I will open that one later.

Each library has its own review team, and they’ll use your tests as a measure of correctness while providing feedback mainly on efficiency and code style. The individual PRs don’t need to build (there’s no Haiku CI here).

You could open two or three PRs in parallel if you want to speed up the upstream process (since you already have these patches in your fork tested for a while).

@trungnt2910
Copy link
Contributor Author

Haiku's System.Diagnostics.Process depends on this so I will open that one later.

What I meant was the two share a common file, Interop.OS.cs, so it will be hard to have both open concurrently.

internal const int B_OS_NAME_LENGTH = 32;

[LibraryImportAttribute(Interop.Libraries.libroot, SetLastError = false)]
private static unsafe partial int _get_next_area_info(int team, ref nint cookie, out area_info areaInfo, nuint size);
Copy link
Member

Choose a reason for hiding this comment

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

The comment on this in haiku header files says

https://github.com/haiku/haiku/blob/d39ce117e02d3d8aac966b1a46bd955a2b59c069/headers/os/kernel/OS.h#L111

Is it ok to be depending on "system private" surface here?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

@waddlesplash How likely are the _get_next_[*]_info functions going to break?

Copy link
Member

Choose a reason for hiding this comment

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

Additionally, is ABI stability (beyond source compatibility) supported on Haiku?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

The alternative is to convert those macros into symbols in System.Native.* then LibraryImport those 🤔

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Additionally, is ABI stability (beyond source compatibility) supported on Haiku?

I am quite sure ABI compatibility within a major release is supported.

Copy link
Member

Choose a reason for hiding this comment

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

git grep SystemNative_Sysctl will give you all the needed wiring.

Copy link
Contributor Author

@trungnt2910 trungnt2910 Nov 21, 2025

Choose a reason for hiding this comment

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

What I meant was that if we create shims, we might want to put them in new files (like pal_haiku_os.c and pal_haiku_image.c) for Haiku.

But with waddlesplash confirming that those LibraryImported functions are stable, should we just keep the current implementation instead?

Copy link
Member

Choose a reason for hiding this comment

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

Going by that SystemNative_Sysctl (and many others) pattern, you would create a file called pal_area_info.c or something similar (filename is agnostic of OS) and then #ifdef HAVE_xx where #else case is (void)foo; // unsed for other platforms. This pattern keeps the internal entrypoint accounting same across platforms.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

The issue here would be _get_next_area_info is not the only API I need to pull in.

#121883 pulls in a bunch of other functions in a similar manner. Therefore, I think an OS-guarded file similar to src/native/libs/System.Native/pal_iossupportversion.m would be better?

Furthermore, sysctl is quite among the UNIX-like OSes, despite some differences among the flavors, while _get_next_area_info is Haiku-specific, so #ifdef TARGET_HAIKU would make more sense than #ifdef HAVE_xx.

Finally, with those functions being confirmed as public and ABI stable from the Haiku side, can't we just keep the current implementation and LibraryImport directly?

Copy link
Member

Choose a reason for hiding this comment

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

Furthermore, sysctl is quite among the UNIX-like OSes, despite some differences among the flavors, while _get_next_area_info is Haiku-specific, so #ifdef TARGET_HAIKU would make more sense than #ifdef HAVE_xx.

We are only using sysctl on FreeBSD.

During the port work, try fitting into the most prevalent existing pattern especially when it has given pause to the reviewers and pointed out in docs #121880 (comment).

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

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

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants