stdlib: fix the build with macOS long tests #13132
Conversation
|
CC: @jrose-apple Sorry Jordan, don't really know who is a good person to run this past. |
|
@swift-ci please smoke test |
|
What the heck is |
|
The right answer here is probably to either use |
|
@jrose-apple well, the interesting thing is that the names are used in the mangled form, so Im not sure that the |
|
That does export it, but ideally this wouldn't be exported, just not dead-stripped. Maybe that's what |
|
@jrose-apple unfortunately, it would need to be exported for the tests to actually use it from the stdlib. |
|
But there aren't any tests using it from the stdlib. |
|
@jrose-apple oh, I thought I mentioned that this was being used by the long tests. |
|
@swift-ci please smoke test |
|
I saw that, but I can't find a test that is actually doing it. |
|
The references are from the runtime:
These show up when building: |
|
|
|
@gparker42 Can we just…not do that? I don't see any reason why the runtime would want to reference these by mangled name rather than non-mangled name. |
|
The runtime tests shouldn't be linking to both the runtime .a and the standard library. We should pick one and stick with it. This has been causing problems elsewhere too (warnings on Darwin about duplicate class implementations). Can we fix that instead? Or is that hard to fix? |
|
Hah, I just saw that and was about to look into that error too. I didnt see the |
|
It links the runtime .a, which references symbols in the stdlib. But it already loads the stdlib, I think. (If not, we should make the .a symbols weak or something so that the runtime tests can still build.) |
|
(It might link a bunch of runtime object files directly. I don't remember.) |
|
Yes, it does link against the runtime, but does not include the stdlib objects. I suppose that we should be able to avoid the double runtime link. |
|
IIRC the test executable links directly to the runtime's C++ code but not its Swift code. unittests/runtime/Stdlib.cpp contains empty placeholders for some Swift functions that are required to build but not to run. We may need to add these functions to that file. |
|
(And the test executable does this because it calls some non-exported runtime functions directly.) |
jrose-apple
dismissed
their stale review
Greg has explained the way forward!
When building swift on macOS with long tests, the build would fail with undefined references to these symbols. The undefined symbols were expected to be present in swiftCore against which we link. However, because the symbols are marked as internal, they would be dead stripped before the dylib was constructed. As a result, we would end up with undefined references to these symbols. Expose the additional internal interfaces for now so that we can build the tests.
|
@swift-ci please smoke test |
|
The linux build is a out of space issue, I think that this should be okay, it also locally passes for me. @jrose-apple or @gparker42 could we get this merged? |
|
@swift-ci please smoke test Linux platform |
When building swift on macOS with long tests, the build would fail with
undefined references to these symbols. The undefined symbols were
expected to be present in swiftCore against which we link. However,
because the symbols are marked as internal, they would be dead stripped
before the dylib was constructed. As a result, we would end up with
undefined references to these symbols. Expose the additional internal
interfaces for now so that we can build the tests.
Replace this paragraph with a description of your changes and rationale. Provide links to external references/discussions if appropriate.
Resolves SR-NNNN.