build: slightly improve the build for libdispatch #785

compnerd · 2023-03-31T16:16:02Z

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

I wonder if we could use another vfs overlay type thing, or just copy all of the headers into the build directory next to the module map instead of pushing things around inside of the source directory?

This is an improvement over the current state of things though.

CMakeLists.txt

src/swift/CMakeLists.txt

etcwilde · 2023-03-31T16:28:56Z

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

compnerd · 2023-03-31T16:29:30Z

That VFS idea 🤔

compnerd · 2023-03-31T21:23:38Z

@swift-ci please test

etcwilde · 2023-03-31T21:25:50Z

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

@dgregor

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!

DougGregor · 2023-04-01T00:02:58Z

@swift-ci please test

etcwilde · 2023-04-01T03:25:39Z

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 · 2023-04-01T03:26:25Z

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

DougGregor · 2023-04-01T05:35:05Z

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 · 2023-04-01T06:03:59Z

switch the Foundation build to use the installed Dispatch.

That would be a day for celebration.

Use a VFS overlay to avoid polluting the source tree.

compnerd · 2023-04-01T18:05:14Z

@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 · 2023-04-01T18:05:25Z

@swift-ci please test

etcwilde · 2023-04-01T18:13:59Z

For tracking purposes:

rdar://92756762
rdar://94289755
rdar://105782611

DougGregor · 2023-04-01T18:36:36Z

@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 · 2023-04-02T04:29:54Z

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 · 2023-04-02T04:30:52Z

@swift-ci please test Windows

DougGregor · 2023-04-02T15:06:59Z

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

compnerd · 2023-04-02T16:53:21Z

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 · 2023-04-02T17:06:15Z

So exciting!

compnerd · 2023-04-02T17:23:43Z

Please test with the following PRs:
swiftlang/swift-corelibs-xctest#437

@swift-ci please test

…on-request" This reverts commit a5167e2, reversing changes made to fa1b4ae.

…g/swift-corelibs-libdispatch#785 This allows the tests that use libdispatch to find its modulemap, plus add the libdispatch compilation flags to one test that was missing them and fix one async test on linux.

etcwilde approved these changes Mar 31, 2023

View reviewed changes

CMakeLists.txt Show resolved Hide resolved

src/swift/CMakeLists.txt Outdated Show resolved Hide resolved

etcwilde requested a review from rokhinip March 31, 2023 16:28

compnerd force-pushed the references-available-upon-request branch 2 times, most recently from 387b87f to e0517b6 Compare March 31, 2023 21:11

dispatch: attempt to use a VFS overlay

ed909bb

Use a VFS overlay to avoid polluting the source tree.

compnerd force-pushed the references-available-upon-request branch from e0517b6 to ed909bb Compare April 1, 2023 18:03

compnerd mentioned this pull request Apr 2, 2023

Tests: add vfs overlay for dipsatch swiftlang/swift-corelibs-xctest#437

Merged

compnerd merged commit a5167e2 into swiftlang:main Apr 3, 2023

compnerd deleted the references-available-upon-request branch April 3, 2023 02:08

MaxDesiatov added a commit that referenced this pull request Jun 1, 2023

Revert "Merge pull request #785 from compnerd/references-available-up…

0e6afec

…on-request" This reverts commit a5167e2, reversing changes made to fa1b4ae.

etcwilde mentioned this pull request Jun 6, 2023

Backport build and Windows fixes for 5.9 #794

Merged

finagolfin mentioned this pull request Aug 22, 2023

[build] Remove intermediate pipeline so that we can guarantee that we always have a fresh swift-dispatch when running swift tests. swiftlang/swift#65829

Merged

kateinoigakukun mentioned this pull request Nov 10, 2023

Building Swift fails on linux with module redefinition errors swiftlang/swift#62429

Closed

build: slightly improve the build for libdispatch #785

build: slightly improve the build for libdispatch #785

Uh oh!

Conversation

compnerd commented Mar 31, 2023

Uh oh!

etcwilde left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

etcwilde commented Mar 31, 2023

Uh oh!

compnerd commented Mar 31, 2023

Uh oh!

compnerd commented Mar 31, 2023

Uh oh!

etcwilde commented Mar 31, 2023

Uh oh!

DougGregor commented Apr 1, 2023

Uh oh!

etcwilde commented Apr 1, 2023

Uh oh!

etcwilde commented Apr 1, 2023

Uh oh!

DougGregor commented Apr 1, 2023

Uh oh!

etcwilde commented Apr 1, 2023

Uh oh!

compnerd commented Apr 1, 2023

Uh oh!

compnerd commented Apr 1, 2023

Uh oh!

etcwilde commented Apr 1, 2023

Uh oh!

DougGregor commented Apr 1, 2023

Uh oh!

DougGregor commented Apr 2, 2023

Uh oh!

DougGregor commented Apr 2, 2023

Uh oh!

DougGregor commented Apr 2, 2023

Uh oh!

compnerd commented Apr 2, 2023

Uh oh!

DougGregor commented Apr 2, 2023

Uh oh!

compnerd commented Apr 2, 2023

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants