Skip to content

Fix two small issues with handling of plugin triples #5945

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
Dec 8, 2022

Conversation

neonichu
Copy link
Contributor

@neonichu neonichu commented Dec 5, 2022

  • We were not stringently using version-less triples in all comparisons
  • We were not merging the supported triples of all executable info entries into the final toolNamesToTriples which could lead to incorrect errors about unsupported platforms

rdar://101096803

@neonichu neonichu self-assigned this Dec 5, 2022
@neonichu
Copy link
Contributor Author

neonichu commented Dec 5, 2022

@swift-ci please smoke test

@tomerd
Copy link
Contributor

tomerd commented Dec 5, 2022

possible to add a test?

@neonichu
Copy link
Contributor Author

neonichu commented Dec 5, 2022

Not sure what is going in the Windows build:

C:\Users\swift-ci\jenkins\workspace\swiftpm-PR-windows\sourcekit-lsp\Sources\LanguageServerProtocol\Messages.swift:24:3: error: cannot find 'CodeActionResolveRequest' in scope

  CodeActionResolveRequest.self,

  ^~~~~~~~~~~~~~~~~~~~~~~~

cc @compnerd

@neonichu
Copy link
Contributor Author

neonichu commented Dec 5, 2022

BTW, it looks like SourceKit-LSP itself is not running Windows for its CI, that seems wrong. Either it should be removed from SwiftPM's preset as well or it should be running Windows builds on its own PRs. cc @shahmishal

@neonichu
Copy link
Contributor Author

neonichu commented Dec 8, 2022

@swift-ci please smoke test linux

@neonichu
Copy link
Contributor Author

neonichu commented Dec 8, 2022

@swift-ci please smoke test windows

@neonichu
Copy link
Contributor Author

neonichu commented Dec 8, 2022

@swift-ci please smoke test

@neonichu
Copy link
Contributor Author

neonichu commented Dec 8, 2022

Ah yes, PluginInvocationTests.testParseArtifactsDoesNotCheckPlatformVersion cannot work on non-macOS

- We were not stringently using version-less triples in all comparisons
- We were not merging the supported triples of all executable info entries into the final `toolNamesToTriples` which could lead to incorrect errors about unsupported platforms

rdar://101096803
@neonichu
Copy link
Contributor Author

neonichu commented Dec 8, 2022

@swift-ci please smoke test

@neonichu neonichu merged commit 1c9477e into main Dec 8, 2022
@neonichu neonichu deleted the fix-plugin-triples branch December 8, 2022 23:06
neonichu added a commit that referenced this pull request Dec 9, 2022
Until now, we had two almost identical code paths for processing accessible tools and e.g. the command plugin code didn't get the fixes from #5945. This PR consolidates the two and also fixes a bug regarding executable path handling of vended tools.
neonichu added a commit that referenced this pull request Dec 15, 2022
Until now, we had two almost identical code paths for processing accessible tools and e.g. the command plugin code didn't get the fixes from #5945. This PR consolidates the two and also fixes a bug regarding executable path handling of vended tools.
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.

3 participants