[stdlib] Use the single-header Glibc modulemap.

[3405691582]

This migrates OpenBSD to use the single-header Glibc modulemap proposed
and implemented in #32404, and necessitates introducing some missing
headers for building Foundation added in #38341.

(This was tested by building Swift and the corelibs on OpenBSD including necessary unmerged prs swiftlang/swift-corelibs-libdispatch#559 and swiftlang/swift-corelibs-foundation#3004.)

[3405691582]

cc @compnerd

[compnerd]

Do you use gyb at all at this point? Why not use CMake to generate the module map? Is there a reason that glibc is still referenced? I think that it makes sense to rename the include header for O/BSD

[3405691582]

I hope I understood your questions, please clarify as necessary.

Do you use gyb at all at this point? Why not use CMake to generate the module map?

Fair enough, we don't need gyb on the libc-openbsd.modulemap.gyb. To avoid using gyb altogether on this file however does incur a large delta on the CMake config for stdlib/public/Platform, so I'm not sure that's a good idea.

Is there a reason that glibc is still referenced?

We still call the module Glibc on other platforms, so we might as well refer it with the same language as per other platforms.

I think that it makes sense to rename the include header for O/BSD

Does it? Wasn't the idea in #32404 was to use a common header, or am I reading that intent wrongly? We could fork SwiftGlibc.h.gyb, but it's already conditionalized via __has_include, so I am concluding that it's not really necessary.

We could alternatively remove libc-openbsd.modulemap.gyb and instead add more conditionals to glibc.modulemap.gyb but I am guessing adding yet more conditionals to that modulemap impacts readability. I am happy to do that if you disagree, however.

[compnerd]

I still strongly disagree with having Glibc be the universal C library import. If we called that MicrosoftVisualC instead of Glibc (and provided an easy way to migrate via a fixit) would that be acceptable? Furthermore, there are much bigger issues with the assumption that Glibc is the import - Linux does not universally use glibc, some use musl and others use uclibc. None of these are compatible with one another and have significant differences that are important. This means that the only way to tell apart the library is addition of a DT_NOTE which is something we do not currently do (and no one else does).

Having the separate paths here I think is a net positive - we can clearly see what different targets are doing and what paths they are going down.

As to the gyb vs cmake - I am slightly partial to CMake because
a) it speeds up the build
b) avoids complexity in the form of gyb handling

AIUI, the reason that the header form was used was to avoid injecting the path to the headers into the module map allowing reuse outside of the original build. This is a good thing, and something we should support, but I am not sure if this is much better. Injecting into system paths is not so terrible, and that allows using of relative paths.

[3405691582]

I still strongly disagree with having Glibc be the universal C library import.

Yeah, it's not ideal. We have @millenomi's CStdlib pitch but I don't know in what state of acceptance that's in, and implementing that or some other variant of this is going to necessitate code changes across the entirety of this and other projects.

As to the gyb vs cmake - I am slightly partial to CMake

So -- let me confirm -- it sounds like you want a static glibc.modulemap for OpenBSD, bypassing the gyb generation process for that, or do you have something else in mind?

[compnerd]

So -- let me confirm -- it sounds like you want a static glibc.modulemap for OpenBSD, bypassing the gyb generation process for that, or do you have something else in mind?

Yes, I think that would be better than the current state of things. Still, less than ideal, but renaming the module for O/BSD doesn't have to be part of this change.

[3405691582]

Done.

[finagolfin]

At this point, why even have a separate OpenBSD modulemap, why not generate it from glibc.modulemap.gyb as the other platforms are doing, including my move to do the same with Bionic in #35707? I know you said "adding yet more conditionals to that modulemap impacts readability," but I don't see that simple file becoming a problem if you add OpenBSD to it.

[3405691582]

Sure, it's not too bad with only a few platforms supported, but as that grows, the conditionals become more complicated and the modulemap harder to reason about. Additionally, if we want to get to a point eventually where each libc has its own module, then we would probably want to have separate platform modulemaps anyway as it'll look cleaner in the wash.

I'm happy to do whatever, ultimately, and we can cross those bridges when we come to them.

[finagolfin]

Personally, I'd prefer if you just used that common glibc gyb file for now, but not a big deal either way.

[3405691582]

@compnerd, wdyt about that option.

[finagolfin]

I agree with Saleem that we should distinguish between the different C libraries with better naming, but as @3405691582 noted that effort is currently unapproved and most ported Swift libraries have code like this, that use the Glibc naming for C libraries that aren't glibc.

Fixing that will take that CStdlib pitch passing first or starting to name the different C libraries from these modulemaps properly, then we will need to update those Swift libraries.

[compnerd]

It is more than just naming. The gyb file is nearly illegible because there are differences between the libcs. Similar to the naming issue - why not just follow the design of the MSVC VisualC library? The C standard doesn't mandate enough to derive the module map definition from that. Additionally, there are plenty of extensions in the various different C libraries, which differ both in implementation and in coverage, that it really does make it far more difficult to make changes. There is no need for approval for splitting out the files into separate files - we are not changing the behaviour, only the implementation.

Even with the modulemaps renamed, the name of the module is controlled by the name within the modulemap definition, and while I would love to see that renamed, I can understand that bit remaining the same until we do have an evolution proposal.

[finagolfin]

The gyb file is nearly illegible because there are differences between the libcs.

This gyb file? Hardly makes a difference to me, and I think it's easier to maintain than having several separate files, but again this is just personal preference.

the name of the module is controlled by the name within the modulemap definition

Right, that was the only name I was referring to above with "we should distinguish between the different C libraries with better naming," as with the example I linked to in Foundation.

[3405691582]

Unfortunately, I have run into a problem with this change.

When building ReleaseAssert and Foundation:

1.      Swift version 5.6-dev (LLVM ec3ee55b085552d, Swift 471d062c1e6c254)
2.      Compiling with the current language version
3.      While evaluating request ExecuteSILPipelineRequest(Run pipelines { Prepa
reOptimizationPasses, EarlyModulePasses, HighLevel,Function+EarlyLoopOpt, HighLe
vel,Module+StackPromote, Serialize, MidLevel,Function, ClosureSpecialize, LowLev
el,Function, LateLoopOpt, SIL Debug Info Generator } on SIL for FoundationNetwor
king)
4.      While running pass #1397 SILModuleTransform "PerformanceSILLinker".
5.      While deserializing SIL function "$s10Foundation4DataV2eeoiySbAC_ACtFZ"
6.      While deserializing SIL function "memcmp"
7.      *** DESERIALIZATION FAILURE ***
module 'Foundation' with full misc version '5.6(5.6)/Swift version 5.6-dev (LLVM
 ec3ee55b085552d, Swift 471d062c1e6c254)'
top-level value not found
Cross-reference to module 'SwiftGlibc'
... memcmp
... with type (UnsafeRawPointer, UnsafeRawPointer, Int) -> Int32
Notes:
* 'memcmp' in module 'SwiftShims' was filtered out.
* 'memcmp' in module 'Foundation' was filtered out.
* 'memcmp' in module 'CoreFoundation' was filtered out.
* 'memcmp' in module 'CDispatch' was filtered out.
* 'memcmp' in module 'Glibc' was filtered out.
* 'memcmp' in module 'SwiftGlibc' was filtered out.
* 'memcmp' in module 'SwiftOverlayShims' was filtered out.
* 'memcmp' in module 'CFURLSessionInterface' was filtered out.

This does not happen with the old modulemap. I'll have to investigate further. I'll ping the pr back when this is resolved.

[finagolfin]

Try the fix I applied for my similar pull for Bionic, given to me by Alexis when I hit an almost identical error.

[3405691582]

Thanks, that's very helpful. Added.

I don't think it makes a lot of sense to have that particular conditional for the LibcShims.h change, but that's another pr, so for now, added that change.

[finagolfin]

@gottesmm, would you review? This is very similar to my pull to get rid of the longer modulemap for Android, #35707, that you reviewed. This will remove the last non-Windows multi-header libc modulemap from this repo.

[finagolfin]

@3405691582, you'll need to rebaterebase even.

[3405691582]

#40774 being merged means OpenBSD builds won't complete at HEAD. Are we in a position to merge or is anything else required for this pr?

[finagolfin]

@compnerd, can we get this in?

[finagolfin]

@CodaFi, would you review?

[3405691582]

Ping, it would be nice to get this merged in some form to solve the buildbreak. Please let me know if any other changes are desired here.

[CodaFi]

Sorry, you'll need to give this one more rebase since I merged @buttaface's PR

[3405691582]

Done and done.

[finagolfin]

@MaxDesiatov, would you run the CI on this?

[MaxDesiatov]

@swift-ci please smoke test

[finagolfin]

I'll submit for the 5.6 branch along with my Bionic cleanup, once this is in trunk.

[CodaFi]

@swift-ci smoke test Linux platform

[finagolfin]

Linux CI failures appear to be flakes.

[CodaFi]

@swift-ci clean smoke test Linux platform

[finagolfin]

Another Linux CI flake: single integration test failed when searching for shared libraries with error subprocess.CalledProcessError: Command '['/usr/bin/find', '/home/build-user/build/buildbot_linux/none-swift_package_sandbox_linux-x86_64', '-iname', '*.so']' died with <Signals.SIGABRT: 6>.

[finagolfin]

@CodaFi, I think you can go ahead and merge, this pull is mostly only going to affect OpenBSD.

[CodaFi]

I can't merge without blue bots

@swift-ci clean smoke test Linux platform

[finagolfin]

Failed in three minutes, didn't even start because the CI flaked.

[CodaFi]

@swift-ci clean smoke test Linux platform

[finagolfin]

@CodaFi, this pull and #40977 are ready to go in.