Skip to content

Fix incorrect ConfiguredTargets for Package.swift w/ multiple workspaces #1545

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

Merged
merged 1 commit into from
Jul 16, 2024

Conversation

plemarquand
Copy link
Contributor

@plemarquand plemarquand commented Jul 2, 2024

In a project with multiple folders each containing a Package.swift, the bestWorkspace found in workspaceForDocument(uri:) was always the first one encountered.

fileHandlingCapability(for:) checks if there are configured targets for the Package.swift and if there are, the workspace is chosen since the Package.swift is determined to be part of the workspace. However the check in configuredTargets(for:) always returned a target for any Package.swift, even if that Package.swift was not part of the workspace associated with the BuildSystem.

Ultimately this manifested as code completion not working in all but one of the Package.swift files in a multi workspace project.

Some work was done in #1210 to address swiftlang/vscode-swift#768, which is where this issue originated from. However while verifying swiftlang/vscode-swift#768 I found that #1210 didn't fully address code completion of Package.swift files in multi workspace projects.

Also took the opportunity to fix a few deprecation warnings.

@plemarquand plemarquand requested a review from ahoppen as a code owner July 2, 2024 17:55
@plemarquand plemarquand force-pushed the multi-package-root-fix branch from d2088d3 to ecffdf0 Compare July 2, 2024 20:37
Copy link
Member

@ahoppen ahoppen left a comment

Choose a reason for hiding this comment

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

Thanks! Could you create a release/6.0 cherry-pick PR as well again?

@ahoppen
Copy link
Member

ahoppen commented Jul 2, 2024

@swift-ci Please test

@ahoppen ahoppen enabled auto-merge July 2, 2024 20:45
auto-merge was automatically disabled July 3, 2024 12:41

Head branch was pushed to by a user without write access

@plemarquand plemarquand force-pushed the multi-package-root-fix branch from ecffdf0 to 9a6e155 Compare July 3, 2024 12:41
In a project with multiple folders each containing a Package.swift, the
`bestWorkspace` found in `workspaceForDocument(uri:)` was always the
first one encountered.

`fileHandlingCapability(for:)` checks if there are configured targets
for the Package.swift and if there are, the workspace is chosen since
the Package.swift is determined to be part of the workspace. However the
check in `configuredTargets(for:)` always returned a target for any
`Package.swift`, even if that `Package.swift` was not part of the
workspace associated with the BuildSystem.

Ultimately this manifested as code completion not working in all but one
of the Package.swift files in a multi workspace project.

Some work was done in swiftlang#1210 to address swiftlang/vscode-swift#768, which
is where this issue originated from. However while verifying
swiftlang/vscode-swift#768 I found that swiftlang#1210 didn't fully address code
completion of `Package.swift` files in multi workspace projects.
@plemarquand plemarquand force-pushed the multi-package-root-fix branch from 9a6e155 to 707b8e5 Compare July 3, 2024 13:24
@ahoppen
Copy link
Member

ahoppen commented Jul 3, 2024

@swift-ci Please test

@ahoppen
Copy link
Member

ahoppen commented Jul 3, 2024

@swift-ci Please test Windows

@plemarquand
Copy link
Contributor Author

@ahoppen when you get a moment I think this is ready to be merged.

@ahoppen ahoppen merged commit 4460c5c into swiftlang:main Jul 16, 2024
3 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
Development

Successfully merging this pull request may close these issues.

2 participants