Skip to content

Fixup several warnings #9009

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.

Sign up for GitHub

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

Open
wants to merge 4 commits into
base: main
Choose a base branch
from
Open

Fixup several warnings #9009

Show file tree
Hide file tree
Changes from 3 commits
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
df13ad4
Fixup several warnings
plemarquand Aug 11, 2025
4855cdd
Move off deprecated function
plemarquand Aug 12, 2025
a006a63
Attempt to fix failing tests (unrelated?)
plemarquand Aug 13, 2025
2b57abc
Address comments
plemarquand Aug 14, 2025
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
8 changes: 4 additions & 4 deletions Sources/Commands/Utilities/PluginDelegate.swift
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -423,8 +423,8 @@ final class PluginDelegate: PluginInvocationDelegate {
func lookupDescription(
for moduleName: String,
destination: BuildParameters.Destination
) throws -> ModuleBuildDescription? {
try buildPlan.buildModules.first {
) -> ModuleBuildDescription? {
buildPlan.buildModules.first {
$0.module.name == moduleName && $0.buildParameters.destination == destination
}
}
Expand All @@ -434,9 +434,9 @@ final class PluginDelegate: PluginInvocationDelegate {
// historically how this was setup. Ideally we should be building for both "host"
// and "target" if module is configured for them but that would require changing
// `PluginInvocationSymbolGraphResult` to carry multiple directories.
let description = if let targetDescription = try lookupDescription(for: targetName, destination: .target) {
let description = if let targetDescription = lookupDescription(for: targetName, destination: .target) {
targetDescription
} else if let hostDescription = try lookupDescription(for: targetName, destination: .host) {
} else if let hostDescription = lookupDescription(for: targetName, destination: .host) {
hostDescription
} else {
throw InternalError("could not find a target named: \(targetName)")
Expand Down
6 changes: 3 additions & 3 deletions Sources/Workspace/Workspace+Delegation.swift
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -216,7 +216,7 @@ extension WorkspaceDelegate {
}
}

struct WorkspaceManifestLoaderDelegate: ManifestLoader.Delegate {
struct WorkspaceManifestLoaderDelegate: ManifestLoader.Delegate, @unchecked Sendable {
private weak var workspaceDelegate: Workspace.Delegate?

init(workspaceDelegate: Workspace.Delegate) {
Expand Down Expand Up @@ -279,7 +279,7 @@ struct WorkspaceManifestLoaderDelegate: ManifestLoader.Delegate {
}
}

struct WorkspaceRepositoryManagerDelegate: RepositoryManager.Delegate {
struct WorkspaceRepositoryManagerDelegate: RepositoryManager.Delegate, @unchecked Sendable {
private weak var workspaceDelegate: Workspace.Delegate?

init(workspaceDelegate: Workspace.Delegate) {
Expand Down Expand Up @@ -335,7 +335,7 @@ struct WorkspaceRepositoryManagerDelegate: RepositoryManager.Delegate {
}
}

struct WorkspaceRegistryDownloadsManagerDelegate: RegistryDownloadsManager.Delegate {
struct WorkspaceRegistryDownloadsManagerDelegate: RegistryDownloadsManager.Delegate, @unchecked Sendable {
private weak var workspaceDelegate: Workspace.Delegate?

init(workspaceDelegate: Workspace.Delegate) {
Expand Down
23 changes: 12 additions & 11 deletions Sources/_InternalTestSupport/GitRepositoryExtensions.swift
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -13,6 +13,7 @@
import SourceControl

import class Basics.AsyncProcess
import class TSCBasic.Process
Copy link
Contributor

Choose a reason for hiding this comment

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

suggestion: can we use AsyncProcess that is located in SwiftPM instead of having to rely on a TSC API?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Switching from Process.checkNonZeroExit to AsyncProcess.checkNonZeroExit would require many methods to be converted to async. While probably worthwhile, it would would be best addressed in a standalone PR.


import enum TSCUtility.Git

Expand All @@ -21,7 +22,7 @@ import enum TSCUtility.Git
package extension GitRepository {
/// Create the repository using git init.
func create() throws {
try systemQuietly([Git.tool, "-C", self.path.pathString, "init"])
try Process.checkNonZeroExit(args: Git.tool, "-C", self.path.pathString, "init")
}

/// Returns current branch name. If HEAD is on a detached state, this returns HEAD.
Expand All @@ -38,36 +39,36 @@ package extension GitRepository {

/// Stage a file.
func stage(file: String) throws {
try systemQuietly([Git.tool, "-C", self.path.pathString, "add", file])
try Process.checkNonZeroExit(args: Git.tool, "-C", self.path.pathString, "add", file)
}

/// Stage multiple files.
func stage(files: String...) throws {
try systemQuietly([Git.tool, "-C", self.path.pathString, "add"] + files)
try Process.checkNonZeroExit(arguments: [Git.tool, "-C", self.path.pathString, "add"] + files)
}

/// Stage entire unstaged changes.
func stageEverything() throws {
try systemQuietly([Git.tool, "-C", self.path.pathString, "add", "."])
try Process.checkNonZeroExit(args: Git.tool, "-C", self.path.pathString, "add", ".")
}

/// Commit the staged changes. If the message is not provided a dummy message will be used for the commit.
func commit(message: String? = nil) throws {
// FIXME: We don't need to set these every time but we usually only commit once or twice for a test repo.
try systemQuietly([Git.tool, "-C", self.path.pathString, "config", "user.email", "[email protected]"])
try systemQuietly([Git.tool, "-C", self.path.pathString, "config", "user.name", "Example Example"])
try systemQuietly([Git.tool, "-C", self.path.pathString, "config", "commit.gpgsign", "false"])
try systemQuietly([Git.tool, "-C", self.path.pathString, "config", "tag.gpgsign", "false"])
try systemQuietly([Git.tool, "-C", self.path.pathString, "commit", "-m", message ?? "Add some files."])
try Process.checkNonZeroExit(args: Git.tool, "-C", self.path.pathString, "config", "user.email", "[email protected]")
try Process.checkNonZeroExit(args: Git.tool, "-C", self.path.pathString, "config", "user.name", "Example Example")
try Process.checkNonZeroExit(args: Git.tool, "-C", self.path.pathString, "config", "commit.gpgsign", "false")
try Process.checkNonZeroExit(args: Git.tool, "-C", self.path.pathString, "config", "tag.gpgsign", "false")
try Process.checkNonZeroExit(args: Git.tool, "-C", self.path.pathString, "commit", "-m", message ?? "Add some files.")
}

/// Tag the git repo.
func tag(name: String) throws {
try systemQuietly([Git.tool, "-C", self.path.pathString, "tag", name])
try Process.checkNonZeroExit(args: Git.tool, "-C", self.path.pathString, "tag", name)
}

/// Push the changes to specified remote and branch.
func push(remote: String, branch: String) throws {
try systemQuietly([Git.tool, "-C", self.path.pathString, "push", remote, branch])
try Process.checkNonZeroExit(args: Git.tool, "-C", self.path.pathString, "push", remote, branch)
}
}
2 changes: 1 addition & 1 deletion Sources/_InternalTestSupport/InMemoryGitRepository.swift
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -396,7 +396,7 @@ extension InMemoryGitRepository: WorkingCheckout {
extension InMemoryGitRepository: @unchecked Sendable {}

/// This class implement provider for in memory git repository.
public final class InMemoryGitRepositoryProvider: RepositoryProvider {
public final class InMemoryGitRepositoryProvider: RepositoryProvider, @unchecked Sendable {
/// Contains the repository added to this provider.
public var specifierMap = ThreadSafeKeyValueStore<RepositorySpecifier, InMemoryGitRepository>()

Expand Down
15 changes: 7 additions & 8 deletions Sources/_InternalTestSupport/misc.swift
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -32,12 +32,12 @@ import Testing
import func XCTest.XCTFail
import struct XCTest.XCTSkip

import class TSCBasic.Process
Copy link
Contributor

Choose a reason for hiding this comment

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

issue: Can we use AsyncProcess located in SwiftPM instead of depending on a TSC API.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

That would require marking the utility method as async and then cascading that across all its usages. Probably worth doing, but in a separate patch

import struct TSCBasic.ByteString
import struct Basics.AsyncProcessResult

import enum TSCUtility.Git

@_exported import func TSCTestSupport.systemQuietly
@_exported import enum TSCTestSupport.StringPattern

@available(*, deprecated, message: "Use CiEnvironment.runningInSmokeTestPipeline")
Expand Down Expand Up @@ -193,7 +193,6 @@ public enum TestError: Error {
case platformNotSupported
}

@available(*, deprecated, message: "Migrate test to Swift Testing and use 'fixture' instead")
@discardableResult public func fixtureXCTest<T>(
name: String,
createGitRepo: Bool = true,
Expand Down Expand Up @@ -305,7 +304,7 @@ fileprivate func setup(
#if os(Windows)
try localFileSystem.copy(from: srcDir, to: dstDir)
#else
try systemQuietly("cp", "-R", "-H", srcDir.pathString, dstDir.pathString)
try Process.checkNonZeroExit(args: "cp", "-R", "-H", srcDir.pathString, dstDir.pathString)
#endif

// Ensure we get a clean test fixture.
Expand Down Expand Up @@ -361,17 +360,17 @@ public func initGitRepo(
try localFileSystem.writeFileContents(file, bytes: "")
}

try systemQuietly([Git.tool, "-C", dir.pathString, "init"])
try systemQuietly([Git.tool, "-C", dir.pathString, "config", "user.email", "[email protected]"])
try systemQuietly([Git.tool, "-C", dir.pathString, "config", "user.name", "Example Example"])
try systemQuietly([Git.tool, "-C", dir.pathString, "config", "commit.gpgsign", "false"])
try Process.checkNonZeroExit(args: Git.tool, "-C", dir.pathString, "init")
try Process.checkNonZeroExit(args: Git.tool, "-C", dir.pathString, "config", "user.email", "[email protected]")
try Process.checkNonZeroExit(args: Git.tool, "-C", dir.pathString, "config", "user.name", "Example Example")
try Process.checkNonZeroExit(args: Git.tool, "-C", dir.pathString, "config", "commit.gpgsign", "false")
let repo = GitRepository(path: dir)
try repo.stageEverything()
try repo.commit(message: "msg")
for tag in tags {
try repo.tag(name: tag)
}
try systemQuietly([Git.tool, "-C", dir.pathString, "branch", "-m", "main"])
try Process.checkNonZeroExit(args: Git.tool, "-C", dir.pathString, "branch", "-m", "main")
} catch {
XCTFail("\(error.interpolationDescription)", file: file, line: line)
}
Expand Down
8 changes: 5 additions & 3 deletions Tests/CommandsTests/PackageCommandTests.swift
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -538,9 +538,11 @@ class PackageCommandTestCase: CommandsBuildProviderTestCase {
let enumerator = try XCTUnwrap(FileManager.default.enumerator(at: URL(fileURLWithPath: path.pathString), includingPropertiesForKeys: nil), file: file, line: line)

var symbolGraphURL: URL?
for case let url as URL in enumerator where url.lastPathComponent == "Bar.symbols.json" {
symbolGraphURL = url
break
while let object = enumerator.nextObject() {
if let url = object as? URL, url.lastPathComponent == "Bar.symbols.json" {
symbolGraphURL = url
break
}
}

let symbolGraphData: Data
Expand Down
10 changes: 5 additions & 5 deletions Tests/FunctionalTests/MiscellaneousTests.swift
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -277,8 +277,8 @@ final class MiscellaneousTestCase: XCTestCase {
// Create a shared library.
let input = systemModule.appending(components: "Sources", "SystemModule.c")
let triple = try UserToolchain.default.targetTriple
let output = systemModule.appending("libSystemModule\(triple.dynamicLibraryExtension)")
try systemQuietly([executableName("clang"), "-shared", input.pathString, "-o", output.pathString])
let output = systemModule.appending("libSystemModule\(triple.dynamicLibraryExtension)")
try await AsyncProcess.checkNonZeroExit(args: executableName("clang"), "-shared", input.pathString, "-o", output.pathString)

let pcFile = fixturePath.appending("libSystemModule.pc")

Expand Down Expand Up @@ -686,7 +686,7 @@ final class MiscellaneousTestCase: XCTestCase {
guard case SwiftPMError.executionFailure(_, _, let stderr) = error else {
return XCTFail("invalid error \(error)")
}
XCTAssert(stderr.contains("error: You don’t have permission"), "expected permissions error. stderr: '\(stderr)'")
XCTAssert(stderr.contains("error: You don’t have permission") || stderr.contains("invalid access"), "expected permissions error. stderr: '\(stderr)'")
}
XCTAssertNoSuchPath(customCachePath)
}
Expand Down Expand Up @@ -721,7 +721,7 @@ final class MiscellaneousTestCase: XCTestCase {
guard case SwiftPMError.executionFailure(_, _, let stderr) = error else {
return XCTFail("invalid error \(error)")
}
XCTAssert(stderr.contains("error: You don’t have permission"), "expected permissions error. stderr: '\(stderr)'")
XCTAssert(stderr.contains("error: You don’t have permission") || stderr.contains("invalid access"), "expected permissions error. stderr: '\(stderr)'")
Copy link
Contributor

Choose a reason for hiding this comment

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

issue (possibly-blocking): I this is also update a test case. do we know if this new expectation is expected, or whether it's a regression?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

This change is unrelated to my patch, but its because this test is failing on main. I had to fix it to get CI to pass. To verify, make sure you do a swift package update first.

Copy link
Contributor

Choose a reason for hiding this comment

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

I understand it's failing on main, but could this mean a regression was introduced somewhere? That is, test is there for a reason and may have actually served its purpose.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I wasn't able to track down what specifically caused this to change, but this failure stems from the underlying API moving from the Foundation based file system APIs to the tools-support-core FileSystem.

Copy link
Contributor

Choose a reason for hiding this comment

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

It would be ideal to understand the root cause and determine whether the is the expected behaviour or a regression, instead of simply updating the test to reflect the current behaviour - which may be a regression.

Copy link
Contributor Author

@plemarquand plemarquand Aug 15, 2025 •
edited
Loading

Choose a reason for hiding this comment

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

Its likely this change: https://github.com/swiftlang/swift-tools-support-core/pull/521/files#diff-4a8cbeddae09f50e3bf71a7b4f098244ee50dad510e6359a24f2678a5bca10d5R506, converting the NSError's produced inside various FileSystem functions into FileSystemErrors.

}
XCTAssertNoSuchPath(customConfigPath)
}
Expand Down Expand Up @@ -756,7 +756,7 @@ final class MiscellaneousTestCase: XCTestCase {
guard case SwiftPMError.executionFailure(_, _, let stderr) = error else {
return XCTFail("invalid error \(error)")
}
XCTAssert(stderr.contains("error: You don’t have permission"), "expected permissions error. stderr: '\(stderr)'")
XCTAssert(stderr.contains("error: You don’t have permission") || stderr.contains("invalid access"), "expected permissions error. stderr: '\(stderr)'")
}
}
#endif
Expand Down
2 changes: 1 addition & 1 deletion Tests/FunctionalTests/ModuleMapTests.swift
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -30,7 +30,7 @@ final class ModuleMapsTestCase: XCTestCase {
let outdir = fixturePath.appending(components: rootpkg, ".build", triple.platformBuildPathComponent, "debug")
try makeDirectories(outdir)
let output = outdir.appending("libfoo\(triple.dynamicLibraryExtension)")
try systemQuietly([executableName("clang"), "-shared", input.pathString, "-o", output.pathString])
try await AsyncProcess.checkNonZeroExit(args: executableName("clang"), "-shared", input.pathString, "-o", output.pathString)

var Xld = ["-L", outdir.pathString]
#if os(Linux) || os(Android)
Expand Down
2 changes: 1 addition & 1 deletion Tests/FunctionalTests/StaticBinaryLibrary.swift
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -24,7 +24,7 @@ struct StaticBinaryLibraryTests {

try await withKnownIssue {
try await fixture(name: "BinaryLibraries") { fixturePath in
let (stdout, stderr) = try await executeSwiftRun(
let (stdout, _) = try await executeSwiftRun(
fixturePath.appending("Static").appending("Package1"),
"Example",
extraArgs: ["--experimental-prune-unused-dependencies"],
Expand Down
2 changes: 1 addition & 1 deletion Tests/IntegrationTests/SwiftPMTests.swift
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -20,7 +20,7 @@ import struct SPMBuildCore.BuildSystemProvider
private struct SwiftPMTests {
@Test(.requireHostOS(.macOS))
func binaryTargets() async throws {
try await withKnownIssue("error: the path does not point to a valid framework:") {
await withKnownIssue("error: the path does not point to a valid framework:") {
try await binaryTargetsFixture { fixturePath in
do {
await withKnownIssue("error: local binary target ... does not contain a binary artifact") {
Expand Down
2 changes: 1 addition & 1 deletion Tests/PackageGraphTests/ModulesGraphTests.swift
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2283,7 +2283,7 @@ struct ModulesGraphTests {
let observability = try loadUnsafeModulesGraph(toolsVersion: .v6_1)

#expect(observability.diagnostics.count == 3)
try testDiagnostics(observability.diagnostics) { result in
testDiagnostics(observability.diagnostics) { result in
var expectedMetadata = ObservabilityMetadata()
expectedMetadata.moduleName = "Foo2"
let diagnostic1 = try #require(result.checkUnordered(
Expand Down
18 changes: 6 additions & 12 deletions Tests/PackageSigningTests/SigningTests.swift
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -786,10 +786,8 @@ struct SigningTests {
return
}
switch signingEntity {
case .recognized(_, let name, let organizationalUnit, let organization):
#expect(name != nil)
#expect(organizationalUnit != nil)
#expect(organization != nil)
case .recognized:
break
case .unrecognized(let name, let organizationalUnit, let organization):
#expect(name != nil)
#expect(organizationalUnit != nil)
Expand Down Expand Up @@ -841,10 +839,8 @@ struct SigningTests {
return
}
switch signingEntity {
case .recognized(_, let name, let organizationalUnit, let organization):
#expect(name != nil)
#expect(organizationalUnit != nil)
#expect(organization != nil)
case .recognized:
break
case .unrecognized(let name, let organizationalUnit, let organization):
#expect(name != nil)
#expect(organizationalUnit != nil)
Expand Down Expand Up @@ -896,10 +892,8 @@ struct SigningTests {
return
}
switch signingEntity {
case .recognized(_, let name, let organizationalUnit, let organization):
#expect(name != nil)
#expect(organizationalUnit != nil)
#expect(organization != nil)
case .recognized:
break
case .unrecognized(let name, let organizationalUnit, let organization):
#expect(name != nil)
#expect(organizationalUnit != nil)
Expand Down
Loading