swiftlang / swift-corelibs-libdispatch

The libdispatch Project, (a.k.a. Grand Central Dispatch), for concurrency on multicore hardware
swift.org
Apache License 2.0
2.47k stars 460 forks source link

build: slightly improve the build for libdispatch #785

Closed compnerd closed 1 year ago

compnerd commented 1 year ago

Avoid polluting the build directory a small amount given that we can use -fmodule-map-file= for the C/C++ build of libdispatch. Unfortunately, for the Swift build, we need to have the file copied over due to the umbrella header resolution.

Hopefully this reduces some of the race conditions that we have seen in the build.

Thanks to @dgregor for reminding me of the flag!

etcwilde commented 1 year ago

CC @rokhinip, this is the source of our little race condition in CI that crops up from time to time.

compnerd commented 1 year ago

That VFS idea :thinking:

compnerd commented 1 year ago

@swift-ci please test

etcwilde commented 1 year ago

Good. I like it. No more copying things around in the source directories during the build.

DougGregor commented 1 year ago

@swift-ci please test

etcwilde commented 1 year ago

Linux failure:

/home/build-user/swift-corelibs-foundation/Sources/Foundation/DispatchData+DataProtocol.swift:14:8: error: cannot load underlying module for 'Dispatch'
import Dispatch
       ^

Windows failure:

CMake Error at dispatch/cmake_install.cmake:59 (file):
  file INSTALL cannot find
  "C:/Users/swift-ci/jenkins/workspace/swift-corelibs-libdispatch-PR-windows/swift-corelibs-libdispatch/dispatch/module.modulemap":
  File exists.
Call Stack (most recent call first):
  cmake_install.cmake:42 (include)
etcwilde commented 1 year ago

Hmm, Linux VFS overlay not quite pointing in the right place maybe?

DougGregor commented 1 year ago

Hmm. The Foundation build has include paths pointing into the Dispatch source directory:

-I /home/build-user/build/buildbot_linux/libdispatch-linux-x86_64/src/swift/swift -I /home/build-user/build/buildbot_linux/libdispatch-linux-x86_64 -I /home/build-user/swift-corelibs-libdispatch -I /home/build-user/swift-corelibs-libdispatch/src -I /home/build-user/build/buildbot_linux/libdispatch-linux-x86_64/src -I /home/build-user/swift-corelibs-libdispatch/src/BlocksRuntime

So we either need to also add the VFS flags to the Foundation build, or we need to first install Dispatch and then switch the Foundation build to use the installed Dispatch.

etcwilde commented 1 year ago

switch the Foundation build to use the installed Dispatch.

That would be a day for celebration.

compnerd commented 1 year ago

@DougGregor correct, I had missed that I had given the wrong visibility to the flags. This is now resolved and -vfsoverlay should propagate properly when building against the build tree. I think that the now current state should build both against an uninstalled as well as installed dispatch. If you use -D dispatch_DIR=... you will prefer the build tree and if you do not, it will prefer the "system version".

compnerd commented 1 year ago

@swift-ci please test

etcwilde commented 1 year ago

For tracking purposes:

DougGregor commented 1 year ago

@DougGregor correct, I had missed that I had given the wrong visibility to the flags. This is now resolved and -vfsoverlay should propagate properly when building against the build tree. I think that the now current state should build both against an uninstalled as well as installed dispatch. If you use -D dispatch_DIR=... you will prefer the build tree and if you do not, it will prefer the "system version".

Sounds like exactly what we want. 🤞

DougGregor commented 1 year ago

The Linux build got much farther! Now the XCTest tests are failing with

# command stderr:
<unknown>:0: error: cannot load underlying module for 'Dispatch'
DougGregor commented 1 year ago

@swift-ci please test Windows

DougGregor commented 1 year ago

Windows got to the same point; now it's the XCTest failures, but Foundation built and tested properly (!)

compnerd commented 1 year ago

Ah, nice! The XCTest failures are not surprising. Fortunately, I had recently enhanced that path for Windows - we just need to make sure that we are passing along the dispatch flags to the tests, not just the build.

DougGregor commented 1 year ago

So exciting!

compnerd commented 1 year ago

Please test with the following PRs: https://github.com/apple/swift-corelibs-xctest/pull/437

@swift-ci please test