Skip to content

[6.4] Ensure targets merged into products by the build system can still be selected using --target#10281

Merged
owenv merged 1 commit into
swiftlang:release/6.4.xfrom
owenv:owenv/target
Jul 9, 2026
Merged

[6.4] Ensure targets merged into products by the build system can still be selected using --target#10281
owenv merged 1 commit into
swiftlang:release/6.4.xfrom
owenv:owenv/target

Conversation

@owenv

@owenv owenv commented Jul 9, 2026

Copy link
Copy Markdown
Contributor

Closes #10275
Closes #9138

When generating PIF, a package target representing an executable or test target may be merged into a product. These remain targets in the user-facing model, and should be selectable using --target on the CLI. Update BuildSubset.pifTargetName to account for this case, fixup an existing test for executables, and add a new one for tests.

@owenv

owenv commented Jul 9, 2026

Copy link
Copy Markdown
Contributor Author

@swift-ci test

@bkhouri bkhouri left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Looks good, though I have a few non-blocking comments related to the tests

Comment on lines +568 to +569
#expect(error.stderr.contains("Could not find target named 'notarealtarget'") ||
error.stderr.contains("no target named 'notarealtarget'"))

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

suggestion: can we split the expectation to each build system validate the expected error message?

Suggested change
#expect(error.stderr.contains("Could not find target named 'notarealtarget'") ||
error.stderr.contains("no target named 'notarealtarget'"))
let expectedErrorMessage = switch buildSystem {
case .native:
// to be updated with expected string
"Could not find target named 'notarealtarget'"
case .swiftbuild:
// to be updated with expected string
"no target named 'notarealtarget'"
}
#expect(error.stderr.contains(expectedErrorMessage)

Comment on lines +545 to +558
let result = try await build(
["--target", "exec2"],
packagePath: fullPath,
configuration: data.config,
buildSystem: buildSystem,
)
switch buildSystem {
case .native:
#expect(result.binContents.contains("exec2.build"))
#expect(!result.binContents.contains("exec1.build"))
case .swiftbuild, .xcode:
#expect(result.binContents.contains(executableName("exec2")))
#expect(!result.binContents.contains(executableName("exec1")))
}

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

suggestion: can we wrap this in a do{} block to ensure the two use cases will always be tested?

Suggested change
let result = try await build(
["--target", "exec2"],
packagePath: fullPath,
configuration: data.config,
buildSystem: buildSystem,
)
switch buildSystem {
case .native:
#expect(result.binContents.contains("exec2.build"))
#expect(!result.binContents.contains("exec1.build"))
case .swiftbuild, .xcode:
#expect(result.binContents.contains(executableName("exec2")))
#expect(!result.binContents.contains(executableName("exec1")))
}
do {
let result = try await build(
["--target", "exec2"],
packagePath: fullPath,
configuration: data.config,
buildSystem: buildSystem,
)
switch buildSystem {
case .native:
#expect(result.binContents.contains("exec2.build"))
#expect(!result.binContents.contains("exec1.build"))
case .swiftbuild, .xcode:
#expect(result.binContents.contains(executableName("exec2")))
#expect(!result.binContents.contains(executableName("exec1")))
}
}

@owenv

owenv commented Jul 9, 2026

Copy link
Copy Markdown
Contributor Author

I'll look at some of the test cleanups in a followup since they're comparatively minor

@owenv owenv merged commit df45aa1 into swiftlang:release/6.4.x Jul 9, 2026
54 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

2 participants