Repository navigation
Conversation
dotnet-policy-service
Bot
added
the
community-contribution
|
Azure Pipelines: Successfully started running 3 pipeline(s). 13 pipeline(s) were filtered out due to trigger conditions. There may be pipelines that require an authorized user to comment /azp run to run. |
|
Tagging subscribers to this area: @karelz, @dotnet/ncl |
|
Draft Pull Request was automatically closed for 30 days of inactivity. Please let us know if you'd like to reopen it. |
linux, freebsd, openbsd and haiku compiled the identical OpenSSL code path; build them as a single unix flavor.
linux and freebsd compiled the same sources; platform differences are already handled at runtime in msquic.cs. Remove the unused SOCKADDR_HAS_LENGTH define and the FreeBSD Interop.Libraries.cs which only duplicated the MsQuic library name.
linux, freebsd and openbsd shared the inotify implementation; build it as a single unix flavor and keep osx/maccatalyst on FSEvents. Throw PlatformNotSupportedException when inotify is unavailable instead of a generic IOException.
Build the OpenLDAP implementation for unix instead of linux only.
Replace the linux, freebsd, openbsd, illumos, solaris and haiku flavors with a single unix flavor that selects the Linux or BSD implementation via OperatingSystem checks, which are JIT/trim time constants. NetworkChange.IsSupported becomes a partial property so the unix flavor can compute it.
|
Azure Pipelines: Successfully started running 4 pipeline(s). 12 pipeline(s) were filtered out due to trigger conditions. There may be pipelines that require an authorized user to comment /azp run to run. |
|
@jkotas, I've reworked it; published apps have no difference in size. There is a small 17.5 KB increase in shared framework size because in "library mode" we use ref assemblies and |
17.5 kB is not prohibitive on its own. The challenge is that this approach doesn't scale. So far, we've preferred build-time checks to exclude OS-specific types rather than relying on runtime checks. If we were to start favoring runtime checks more broadly, the cumulative size increase will be much more significant. This PR moves us away from having consistent preference. How are we going to decide between build-time and runtime checks for OS-specific types going forward? |
There was a problem hiding this comment.
This can be simplified further by using SetLibraryImportResolver
| public static NetworkInterface[] GetAllNetworkInterfaces() | ||
| { | ||
| return LinuxNetworkInterface.GetLinuxNetworkInterfaces(); | ||
| if (OperatingSystem.IsLinux()) |
There was a problem hiding this comment.
@dotnet/ncl Do you have an opinion about merging some unix variants into same build, and replacing the build time checks with runtime checks?
There was a problem hiding this comment.
What would be the argument for the chagne? My current hunch is to leave it as-is, but I will bring it up on for team discussion
I don't think this framing is accurate because we are using mixed bag of #if TARGET_FOO and if (OperatingSystem.IsFoo()) in libraries (in my estimation, we are using OS.IsFoo more these days). Secondly, we have only five non-corelib projects left using per unix-flavor TPM. After this PR, there will be one. Also, trimming of OperatingSystem.IsXX in library mode is a illinker.targets fix, which levels the assembly size (I have it locally). |
Could you please submit this fix in a separate PR? This refactoring will be much easier to accept once there is no size regression. |
No description provided.