Skip to content

Conversation

@ktoso
Copy link
Contributor

@ktoso ktoso commented Oct 28, 2022

Pull Request Description

Now that swift 5.7 is stable we really should be testing the distributed actors cluster because it is the large codebase pushing both swift concurrency and the distributed feature.

https://github.com/apple/swift-distributed-actors

This requires 5.7 so we need to add that Swuft support... not entirely sure how to do that, will reach out to someone.

Acceptance Criteria

To be accepted into the Swift source compatibility test suite, a project must:

  • be an Xcode or swift package manager project
  • support building on either Linux or macOS
  • target Linux, macOS, or iOS/tvOS/watchOS device
  • be contained in a publicly accessible git repository
  • maintain a project branch that builds against Swift 4.0 and passes any unit tests
    • it uses language features which require 5.7, so that's the minimum version
  • have maintainers who will commit to resolve issues in a timely manner
  • be compatible with the latest GM/Beta versions of Xcode and swiftpm
  • add value not already included in the suite
  • be licensed with one of the following permissive licenses:
    • Apache License, version 2.0
  • pass ./project_precommit_check script run
    • TODO

@ktoso
Copy link
Contributor Author

ktoso commented Oct 28, 2022

@swift-ci please test

@shahmishal
Copy link
Member

DistributedActorsTestKit/ActorTestKit.swift:16:18: error: module 'DistributedCluster' was not compiled for testing
@testable import DistributedCluster
~~~~~~~~~~       ^

Is this known failure?

@ktoso
Copy link
Contributor Author

ktoso commented Oct 29, 2022

Hm, I thought I handled that by

         "configuration": "debug",

🤔

But I can also make it such that we don't need that testable import, I'll look into it.

@ktoso
Copy link
Contributor Author

ktoso commented Oct 29, 2022

@swift-ci please test

1 similar comment
@ktoso
Copy link
Contributor Author

ktoso commented Jan 24, 2023

@swift-ci please test

@justice-adams-apple
Copy link
Collaborator

@ktoso 👋 Adding

         "configuration": "debug",

to the projects.json sets the default when and end-user doesn't override the build config via the cmd-line. In pr testing we use
--build-config=release

And in our nightly CI we have jobs for both debug and release builds, so we'll need the project to build with both configurations.

If you need to compile tests as part of your build_portion, add "build_tests": "true", to your build action, e.g.

      {
        "action": "BuildSwiftPackage",
        "configuration": "debug",
        "build_tests": "true",
        "tags": "sourcekit-disabled swiftpm"
      },

*You will likely need to merge main into your branch for this to work as this is relatively new functionality.

@ktoso
Copy link
Contributor Author

ktoso commented Feb 20, 2023

Thanks @justice-adams-apple , actually this is resolved I just need to bump the commit we point at I noticed.

Worse though, there is a regression in Swift that makes the library crash, I'm looking into it rdar://104583893 and will re-request tests/inclusion here.

@ktoso ktoso force-pushed the wip-distributed-actors branch from c2c3c5f to e6e0481 Compare February 21, 2023 08:36
@ktoso
Copy link
Contributor Author

ktoso commented Feb 21, 2023

Okey this should work fine I hope! We also fixed the regression now in Swift itself that this maybe could have helped prevent.

PASS: swift-distributed-actors, 5.7, e024c1, Swift Package
========================================
Action Summary:
     Passed: 1
     Failed: 0
    XFailed: 0
    UPassed: 0
      Total: 1
========================================
Repository Summary:
      Total: 1
========================================
Result: PASS
========================================
--- swift-distributed-actors checked successfully against Swift 5.7 ---

@ktoso
Copy link
Contributor Author

ktoso commented Feb 21, 2023

@swift-ci please test

@ktoso
Copy link
Contributor Author

ktoso commented Feb 22, 2023

Sweet, thanks!

@ktoso ktoso merged commit 5805211 into main Feb 22, 2023
@ktoso ktoso deleted the wip-distributed-actors branch February 22, 2023 01:42
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.

5 participants