Reorganize libs.native

[am11]

Fixes #61864.

[dotnet-issue-labeler]

I couldn't figure out the best area label to add to this PR. If you have write-permissions please help me learn by adding exactly one area label.

[am11]

@jkotas, @carlossanlop, with this structure, there is no Unix, Windows and AnyOS subdirectories anymore and System.IO.Compression/ subdir has everything required by that lib. The merged cmake configs will make things relatively easier if/when zlib build from source on Unix is enabled.

@safern, @jkoritzinsky, I've moved native-binplace.proj to src/libraries/native-binplace.proj instead; since it relies on common libraries' props/targets.

[jkotas]

Looks great! Thank you for doing this.

I was hoping that the shims can be directly in the native directory, but I understand why you have put them native/libs and I do not have a better suggestion. So LGTM.

[carlossanlop]

@am11 @jkotas - For my own education, can you please confirm if I'm understanding the changes in this PR correctly? Please correct me if I got something wrong:

All the native code is built using the same cmake script(s), but depending on the OS, we choose which code files to build for that OS:

• Th pal_*.c|.h files contain syscalls that only apply to Unix and WASM..
• The zlib stuff is cross-platform, and could be built in any OS, but we currently only consume it in Windows, but maybe in the future it could be consumed by others.
• The new structure will be clearer and will avoid future confusion like when I recently asked why zlib lived inside the Windows folder.

At first glance the changes look good to me, but I'm far from being the expert in cmake stuff, so I'll defer the decision to merge to @jkotas.

[am11]

All the native code is built using the same cmake script(s), but depending on the OS, we choose which code files to build for that OS:

That's right. The cmake configuration are now unified, to make it easier to enable which sources to compile without changing the directory structure. This simplifies things; no more indirections and conditions outside the cmake script.

Th pal_*.c|.h files contain syscalls that only apply to Unix and WASM..

Currently these utility headers are only used by Unix. When a source file which compiles on Windows require them, we can sprinkle around some ifdefs to make it compile. :)

The zlib stuff is cross-platform, and could be built in any OS, but we currently only consume it in Windows, but maybe in the future it could be consumed by others.

Yup, enabling zlib compile-from-src will be a matter of hoisting these lists https://github.com/dotnet/runtime/blob/92854b914abe1ca2796ffc6c0abf3a076b222313/src/native/libs/System.IO.Compression.Native/CMakeLists.txt#L115
out of if (CLR_CMAKE_TARGET_UNIX OR CLR_CMAKE_TARGET_BROWSER) / else () condition and adding ZLIB_SOURCES to Unix.
On that note, we still have three copies of zlib in the repo:

• src/mono/mono/zlib
• src/native/libs/System.IO.Compression.Native/zlib-intel
• src/native/libs/System.IO.Compression.Native/zlib

it might be possible to converge to one (less) flavor. (cc @lambdageek, @akoeplinger)

[safern]

Other than some adjustments on the evaluate path subsets, looks good to me.

[carlossanlop]

On that note, we still have three copies of zlib in the repo:

• src/mono/mono/zlib
• src/native/libs/System.IO.Compression.Native/zlib-intel
• src/native/libs/System.IO.Compression.Native/zlib

it might be possible to converge to one (less) flavor.

Oh I didn't know there was also a separate zlib in mono. I'll share this info in my PR: #61883

[safern]

@am11 I was curious at what we saw previously that we needed to add src/native/* to the mono and coreclr subsets on the change you pointed out and it seems like it was a confusion, what was missing was to include the condition on the runtime_tests changed path variable so that the specific job that was missing was executed? I just ran this test with what you have at the moment for coreclr subset (removing src/native/* from the include list:

>> git diff
@@ -1,6 +1,6 @@
 #ifndef __DIAGNOSTICS_DUMP_PROTOCOL_H__
 #define __DIAGNOSTICS_DUMP_PROTOCOL_H__
-
+// change
 #include "ds-rt-config.h"
 
 #ifdef ENABLE_PERFTRACING

>> eng/pipelines/evaluate-changed-paths.sh --azurevariable containsChange --difftarget dotnet/main --subset coreclr --includepaths 'src/libraries/System.Private.CoreLib/*+src/native/libs/Common/*+src/native/libs/System.Globalization.Native/*+src/native/libs/System.IO.Compression.Native/*' --excludepaths 'eng/Version.Details.xml+*.md+LICENSE.TXT+PATENTS.TXT+THIRD-PARTY-NOTICES.TXT+docs/*+src/installer/*+src/mono/*+src/libraries/*+src/native/libs/*+src/tests/*+eng/pipelines/installer/*+eng/pipelines/mono/*+eng/pipelines/libraries/*'


******* Probing coreclr exclude paths *******
eng/Version.Details.xml
*.md
LICENSE.TXT
PATENTS.TXT
THIRD-PARTY-NOTICES.TXT
docs/*
src/installer/*
src/mono/*
src/libraries/*
src/native/libs/*
src/tests/*
eng/pipelines/installer/*
eng/pipelines/mono/*
eng/pipelines/libraries/*

+ git diff -M -C -b --ignore-cr-at-eol --ignore-space-at-eol --exit-code --quiet dotnet/main -- ':!eng/Version.Details.xml' ':!*.md' ':!LICENSE.TXT' ':!PATENTS.TXT' ':!THIRD-PARTY-NOTICES.TXT' ':!docs/*' ':!src/installer/*' ':!src/mono/*' ':!src/libraries/*' ':!src/native/libs/*' ':!src/tests/*' ':!eng/pipelines/installer/*' ':!eng/pipelines/mono/*' ':!eng/pipelines/libraries/*'

----- Matching files for coreclr -----
+ git diff -M -C -b --ignore-cr-at-eol --ignore-space-at-eol --name-only dotnet/main -- ':!eng/Version.Details.xml' ':!*.md' ':!LICENSE.TXT' ':!PATENTS.TXT' ':!THIRD-PARTY-NOTICES.TXT' ':!docs/*' ':!src/installer/*' ':!src/mono/*' ':!src/libraries/*' ':!src/native/libs/*' ':!src/tests/*' ':!eng/pipelines/installer/*' ':!eng/pipelines/mono/*' ':!eng/pipelines/libraries/*'
src/native/eventpipe/ds-dump-protocol.h

Setting pipeline variable containsChange=true
##vso[task.setvariable variable=containsChange;isSecret=false;isOutput=true]true

[am11]

Thanks for checking it, @safern.

Currently all jobs that are conditioned on runtimetests also have either coreclr or mono (checked with git grep -B3 -A1 SetPathVars_runtimetests -- :/eng/pipelines), so we are safe. If it seems implicit, we can make it explicit by adding src/native/* or src/native/eventpipe/* to runtimetests' include list.

[safern]

I think it is safe to leave it as is, I don't want to end on a world where every include path needs to be listed, unless we have a subset like the jit subset where only want to include certain paths and exclude the rest, so we implicitly include them and not exclude any paths.

[am11]

FSW test failure on OSX is unrelated to changes.

[carlossanlop]

If everything looks good, can this get merged? My PR #61883 is currently blocked by this change.