[tests] Compile BlocksRuntimeStub with stdlib compiler settings #34859
Conversation
|
@swift-ci please test |
|
Does it perhaps make sense to move the test library into the stdlib directory? That would reduce a significant amount of duplication, we just need to ensure that we clearly indicate that the BlocksRuntime here is for testing, so we could create it as |
I can look into that. I looked a little bit there, but it wasn't very clear where it would have gone. However, there will be a strange bit of the In any case, I will look into it, and we can decide over the code if it looks right or not. |
|
I think that |
Some pieces of the main CMakeLists.txt file are useful to future CMakeLists.txt. In order to avoid duplication, move those pieces into a new SwiftTestsUtils.cmake to be able to include the functions from other files. This should not modify anything in the behaviour of the build.
BlocksRuntimeStubs is built for the target SDK(s), but it was using one of the stdlib CMake helpers without setting up the same environment as the stdlib/ folder is setting. To avoid problems, like BlocksRuntimeStubs being built with the host compiler, instead of the just built compiler, and other difficult to understand problems, create a folder for it in stdlib/private/. This will allow the usage of the stdlib CMake function in the same environment that is normally used.
Just adding indentation at the foreach block for the build_flavors, which was not applying extra indentation. This should not modify the behaviour of the build.
drodriguez
force-pushed
the
test-blocks-runtime-stub-as-stdlib
branch
from
abaa66d to
0de902e
Compare
|
@swift-ci please test |
|
This broke building without the blocks runtime (Fedora 33 x86-64): |
|
There really ought to either be some CMake additions to verify that the blocks runtime is installed or the test harness needs a new "REQUIRES: block_runtime" mode. |
|
@davezarzycki Could you add the appropriate gates? |
|
I could but I'm not an expert at CMake or lit, so it'd take me some time. Also, I've already worked around the problem by installing the blocks runtime, so this won't be a high priority. If somebody else feels comfortable doing this and can, then please :-) |
|
@davezarzycki: so I imagine that you don't have a From what I understand, as long as you are building the dynamic stdlib and the tests, the stubs should be build, and the This mostly changes the CMake files, but should not have changed anything the output in the build directory, so one should have expected the same results before and after. PD: I might be misremembering, but you didn't use the build script, right? Can you share your build setup? |
|
I don't build the blocks runtime nor do I normally have it installed, so I don't need to check. It's not there. I just use cmake with a unified build setup (of the minimum set of projects). Nothing fancy. |
|
This is not about building or having Blocks Runtime installed. This is a library with an empty function stubbing the necessary pieces of Blocks Runtime that it is used during testing (precisely to avoid having to install or build blocks runtime). I also don't have Blocks Runtime installed in the system (but I am using Ubuntu). These stubs should end up in About the unified build setup, since it is not very standard, and there might be many variants, do you have some document explaining it? I would try to setup a virtual machine and help, but without a setup to reproduce it feels that I am going to be losing time in unnecessary detours. |
|
I see. I'm sorry, I hadn't realized that a stub library was/is being built. Now that I've checked, it seems like it lives in a different location than you asked about. So NOT here: It also seems like the latter directory would be empty / not exist if it weren't for that library, so ya, it seems like the build system is confused and As to unified builds, it isn't that complicated in theory but in practice one can run into lots of gotchas. The short summary is that one creates a checkout of llvm-project and then one checks out Oh, and Swift tends to refer to unified builds as being "not standalone" if that helps you better understand the build setup. |
|
Oh, it just occurred to me that it's really easy when hacking on Swift's cmake setup (or really any LLVM derived build system) to assume/forget that the CMake output directories are not always the same as the project output directories. For example, |
That's probably the problem. Sorry for not realizing the values in unified builds change slightly. The original path was relative to I am going to be away from the keyboard for a while, maybe without connectivity. If you can make that change in |
* 'main' of github.com:apple/swift: [test] Mark two SILOptimizer tests requiring asserts [test] Disable IRGen/unmanaged_objc_throw_func.swift with non-optimized stdlib [NFC] Move Migrated SDK Target List into StdlibDeploymentTarget [stdlib] Unbreak unified builds after swiftlang#34859 Update a comment in EscapeAnalysis. [concurrency] IRGen: update task/executor/context on every suspend point [auto-diff] Fix a bunch of places in the *Cloners where we were not closing borrow scopes. [test] Adjusting stdlib ocurrences where class syntax is used for protocol inheritance [test] Adding specific tests for the warning for protocol inheritance class keyword syntax deprecation [test] Adjusting test files where class syntax is used for protocol inheritance [Sema] Adding deprecation warning for protocol inheritance class keyword syntax [stdlib] Simplify buffer pointer initialization. [cxx-interop] Bail on functions that use unimportable types. Sema: Remove a dead param on TypeChecker::conformsToProtocol [CSBinding] Attempt to join any existing and viable bindings with new binding [cmake] Semi-parametrize manpage location.
A CMake refactoring in swiftlang#34859 caused failures in Mac Catalyst configurations which are not tested in open source. This commit corrects these issues. Fixes rdar://71897958.
BlocksRuntimeStubwas being build in the middle oftest/CMakeLists.txtand using a stdlib function, however, the environment intest/CMakeLists.txtis not the same as the environment insidestdlib/where that function is normally used. To avoid problems,BlocksRuntimeStubis moved into its own folder understdlib/private/, where the compiler environment will be correct.Most of the time this was working because the host compiler is modern enough to compile the simple stub in a compatible way, however for the case of Android using newer unified NDKs, the host compiler in Ubuntu 18.04 would not work.
As a step before the main change, some pieces of the
test/CMakeLists.txtare extracted into helper functions intest/cmake/modules/SwiftTestUtils.cmaketo allow them to be used in the newstdlib/private/BlocksRuntimeStub/CMakeLists.txt.Also a small cleanup is done after the main change in which the indentation of
test/CMakeLists.txtis corrected in aforeachblock.This is mostly moving code around to different files. It might be easier to check the changes per commit instead of as a whole. It might also be easier to check ignoring the white space changes (adding
?w=0to the URL, IIRC).@compnerd: you were the original author of the BlocksRuntimeStub bits. I think that besides using the compiler that the stdlib is using, these changes should not change anything.
@devincoughlin: this modified around the build flavor for macCatalyst. I think the changes are non-functional around those values, and nothing at all should change, but I would appreciate your feedback if something looks wrong.
@buttaface: this should allow the tests to compile with newer NDKs. I tried your changes on top of this and it the CI
build-scriptinvocation seems to perform the testing correctly.