Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

build: build target libdispatch as necessary #35920

Merged
merged 1 commit into from Feb 12, 2021

Conversation

compnerd
Copy link
Collaborator

This should not be predicated on the host libdispatch being built, only
if concurrency is being built in multithreaded mode.

Replace this paragraph with a description of your changes and rationale. Provide links to external references/discussions if appropriate.

Resolves SR-NNNN.

This should not be predicated on the host libdispatch being built, only
if concurrency is being built in multithreaded mode.
@compnerd
Copy link
Collaborator Author

CC: @drexin

@compnerd
Copy link
Collaborator Author

@swift-ci please test

Copy link
Member

@drexin drexin left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks!

@swift-ci
Copy link
Collaborator

Build failed
Swift Test Linux Platform
Git Sha - 3b1f01d

@drexin
Copy link
Member

drexin commented Feb 11, 2021

Linux test failed because of a flaky test. The relevant part succeeded.

@drexin
Copy link
Member

drexin commented Feb 11, 2021

@swift-ci smoke test Linux

@drexin
Copy link
Member

drexin commented Feb 11, 2021

@swift-ci smoke test linux

@compnerd compnerd merged commit 9b1eccc into apple:main Feb 12, 2021
@compnerd compnerd deleted the target-libdispatch branch February 12, 2021 01:57
@davezarzycki
Copy link
Collaborator

Hi @compnerd – Just FYI, this broke my unified build setup (which doesn't use libdispatch). I'm trying to figure out a workaround but I suspect I'll create a pull request soon. Here is the error:

CMake Error at /usr/share/cmake/Modules/ExternalProject.cmake:2555 (message):
  No download info given for 'libdispatch-linux-x86_64' and its source
  directory:

   /home/dave/b/u/t/tools/swift/libdispatch-linux-x86_64-prefix/src/libdispatch-linux-x86_64

  is not an existing non-empty directory.  Please specify one of:

   * SOURCE_DIR with an existing non-empty directory
   * DOWNLOAD_COMMAND
   * URL
   * GIT_REPOSITORY
   * SVN_REPOSITORY
   * HG_REPOSITORY
   * CVS_REPOSITORY and CVS_MODULE
Call Stack (most recent call first):
  /usr/share/cmake/Modules/ExternalProject.cmake:3206 (_ep_add_download_command)
  /home/dave/ro_s/u/swift/cmake/modules/Libdispatch.cmake:76 (ExternalProject_Add)
  /home/dave/ro_s/u/swift/CMakeLists.txt:964 (include)



@davezarzycki
Copy link
Collaborator

I don't see an obvious workaround and I don't fully appreciate what this pull request is trying to solve. What does seem to be happening now is that libdispatch is now mandatory when it didn't used to be. Was this intentional? Where was the discussion?

@davezarzycki
Copy link
Collaborator

davezarzycki commented Feb 12, 2021

Digging around some more, it seems like the relationship with libdispatch has never been formalized as a CMake option, so what we end up with is a mess of code trying to infer what to do. In practice, we have a tri-state:

  1. Use host libdispatch (default for Apple platforms)
  2. Build libdispatch (default for non-Apple platforms)
  3. Don't use libdispatch (for reasons unknown)

I think I have a solution: #35939

@kubamracek
Copy link
Contributor

@compnerd I think this may also be the reason why the "minimal" bot is now red? Could you take a look? https://ci.swift.org/job/oss-swift-test-stdlib-with-toolchain-minimal/157/

@edymtt
Copy link
Contributor

edymtt commented Feb 15, 2021

@kubamracek @compnerd I would have a PR to address that leveraging the flag introduced in #35939

#35976

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

6 participants