Skip to content

better diagnostics when potentially duplicate packages are found #6317

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

Conversation

tomerd
Copy link
Contributor

@tomerd tomerd commented Mar 23, 2023

motivation: with the introduction of the regsitry, there is a higher likelyhood of running into duplicate packages

changes: perform a heuristics to try and determine if the packages are duplicate and provide a better diagnostics instead of the standard "duplicate target" one

@tomerd
Copy link
Contributor Author

tomerd commented Mar 23, 2023

@neonichu @abertelrud for initial review, still need to add tests, but I think this is putting us in a slightly better footing

current diagnostics having swift-nio from source control and swiftpm-test.swift-nio-8 from registry:

error: multiple targets named 'CNIOAtomics' in: 'swift-nio', 'swiftpm-test.swift-nio-8'; consider using the `moduleAliases` parameter in manifest to provide unique names
error: multiple targets named 'CNIODarwin' in: 'swift-nio', 'swiftpm-test.swift-nio-8'; consider using the `moduleAliases` parameter in manifest to provide unique names
error: multiple targets named 'CNIOLinux' in: 'swift-nio', 'swiftpm-test.swift-nio-8'; consider using the `moduleAliases` parameter in manifest to provide unique names
error: multiple targets named 'CNIOSHA1' in: 'swift-nio', 'swiftpm-test.swift-nio-8'; consider using the `moduleAliases` parameter in manifest to provide unique names
error: multiple targets named 'NIO' in: 'swift-nio', 'swiftpm-test.swift-nio-8'; consider using the `moduleAliases` parameter in manifest to provide unique names
error: multiple targets named 'NIOConcurrencyHelpers' in: 'swift-nio', 'swiftpm-test.swift-nio-8'; consider using the `moduleAliases` parameter in manifest to provide unique names
error: multiple targets named 'NIOFoundationCompat' in: 'swift-nio', 'swiftpm-test.swift-nio-8'; consider using the `moduleAliases` parameter in manifest to provide unique names
error: multiple targets named 'NIOHTTP1' in: 'swift-nio', 'swiftpm-test.swift-nio-8'; consider using the `moduleAliases` parameter in manifest to provide unique names
error: multiple targets named 'NIOTLS' in: 'swift-nio', 'swiftpm-test.swift-nio-8'; consider using the `moduleAliases` parameter in manifest to provide unique names
error: multiple targets named 'NIOWebSocket' in: 'swift-nio', 'swiftpm-test.swift-nio-8'; consider using the `moduleAliases` parameter in manifest to provide unique names

with this change:

error: multiple identical targets 'CNIOAtomics', 'CNIODarwin', 'CNIOLinux', 'CNIOSHA1', 'NIO', 'NIOConcurrencyHelpers', 'NIOFoundationCompat', 'NIOHTTP1', 'NIOTLS', 'NIOWebSocket' appear in registry package 'swiftpm-test.swift-nio-8' and source control package 'swift-nio', this may indicate that the two packages are the same and can be de-duplicated by activating the automatic source-control to registry replacement, or by using mirrors

also with this change, for potentially duplicate packages not across scm/registry:

error: multiple identical targets 'CNIOAtomics', 'CNIODarwin', 'CNIOHTTPParser', 'CNIOLinux', 'CNIOSHA1', 'NIO', 'NIOChatClient', 'NIOChatServer', 'NIOConcurrencyHelpers', 'NIOEchoClient', 'NIOEchoServer', 'NIOFoundationCompat', 'NIOHTTP1', 'NIOHTTP1Server', 'NIOMulticastChat', 'NIOPerformanceTester', 'NIOTLS', 'NIOUDPEchoServer', 'NIOWebSocket', 'NIOWebSocketServer', '_NIO1APIShims' appear in package 'swiftpm-test.swift-nio-7' and 'swiftpm-test.swift-nio-8', this may indicate that the two packages are the same and can be de-duplicated by using mirrors

@tomerd tomerd force-pushed the fix/better-diagnostics-for-potentially-duplicate-pacakges branch 2 times, most recently from db170bd to 742aa34 Compare March 23, 2023 05:06
@neonichu
Copy link
Contributor

Would it make sense to not even mention the entire list of target names, just to make the message a little more readable? e.g. "'CNIOAtomics' and X more targets"

@tomerd tomerd changed the title [wip] better diagnostics when potentially duplicate packages are found better diagnostics when potentially duplicate packages are found Mar 25, 2023
@tomerd
Copy link
Contributor Author

tomerd commented Mar 25, 2023

@swift-ci smoke test

@tomerd
Copy link
Contributor Author

tomerd commented Mar 25, 2023

@neonichu this is ready for review

@tomerd
Copy link
Contributor Author

tomerd commented Mar 25, 2023

@swift-ci smoke test windows

@tomerd
Copy link
Contributor Author

tomerd commented Mar 27, 2023

@swift-ci smoke test

tomerd added 2 commits March 27, 2023 09:54
motivation: with the intriduction of the regsitry, there is a higher likelyhood of running into duplicate packages

changes: perform a heuristics to try and determin if the packages are duplicate and provide a better diagnostics instead of the standard "duplicate product" one
@tomerd tomerd force-pushed the fix/better-diagnostics-for-potentially-duplicate-pacakges branch from 3bd6e26 to 20c3cb8 Compare March 27, 2023 16:56
@tomerd
Copy link
Contributor Author

tomerd commented Mar 27, 2023

@swift-ci smoke test

@tomerd
Copy link
Contributor Author

tomerd commented Mar 27, 2023

@swift-ci smoke test windows

@tomerd
Copy link
Contributor Author

tomerd commented Mar 27, 2023

windows failures seems unrelated. cc @compnerd

@tomerd tomerd merged commit 12e124b into swiftlang:main Mar 27, 2023
tomerd added a commit to tomerd/swift-package-manager that referenced this pull request Mar 27, 2023
…ftlang#6317)

motivation: with the introduction of the regsitry, there is a higher likelihood of running into duplicate packages

changes: perform a heuristics to try and determin if the packages are duplicate and provide a better diagnostics instead of the standard "duplicate product" one
@compnerd
Copy link
Member

@tomerd yeah, @etcwilde is aware of this flake, it occurs on macOS as well it seems.

@etcwilde
Copy link
Contributor

@_spi(MainActorUtilities) import _Concurrency
 ^

C:\Users\swift-ci\jenkins\workspace\swiftpm-PR-windows\swift\test\Concurrency\Runtime\startOnMainActor.swift:44:14: error: generic parameter 'Success' could not be inferred
    let t2 = Task.startOnMainActor {
             ^

C:\Users\swift-ci\jenkins\workspace\swiftpm-PR-windows\swift\test\Concurrency\Runtime\startOnMainActor.swift:44:14: error: generic parameter 'Failure' could not be inferred
    let t2 = Task.startOnMainActor {
             ^

C:\Users\swift-ci\jenkins\workspace\swiftpm-PR-windows\swift\test\Concurrency\Runtime\startOnMainActor.swift:44:14: note: explicitly specify the generic arguments to fix this issue
    let t2 = Task.startOnMainActor {
             ^
                 <<#Success: Sendable#>, <#Failure: Error#>>

C:\Users\swift-ci\jenkins\workspace\swiftpm-PR-windows\swift\test\Concurrency\Runtime\startOnMainActor.swift:44:19: error: type 'Task<Success, Failure>' has no member 'startOnMainActor'
    let t2 = Task.startOnMainActor {
             ~~~~ ^~~~~~~~~~~~~~~~

C:\Users\swift-ci\jenkins\workspace\swiftpm-PR-windows\swift\test\Concurrency\Runtime\startOnMainActor.swift:79:14: error: generic parameter 'Success' could not be inferred
    let t2 = Task.startOnMainActor {
             ^

C:\Users\swift-ci\jenkins\workspace\swiftpm-PR-windows\swift\test\Concurrency\Runtime\startOnMainActor.swift:79:14: error: generic parameter 'Failure' could not be inferred
    let t2 = Task.startOnMainActor {
             ^

C:\Users\swift-ci\jenkins\workspace\swiftpm-PR-windows\swift\test\Concurrency\Runtime\startOnMainActor.swift:79:14: note: explicitly specify the generic arguments to fix this issue
    let t2 = Task.startOnMainActor {
             ^
                 <<#Success: Sendable#>, <#Failure: Error#>>

C:\Users\swift-ci\jenkins\workspace\swiftpm-PR-windows\swift\test\Concurrency\Runtime\startOnMainActor.swift:79:19: error: type 'Task<Success, Failure>' has no member 'startOnMainActor'
    let t2 = Task.startOnMainActor {
             ~~~~ ^~~~~~~~~~~~~~~~

error: command failed with exit status: 1

Yeah, this was the failure that's been hitting me locally. It has multiple levels of weirdness. First is, why are we picking up swift interfaces for the stdlib. We should be using the swiftmodule. Second is, why are we building swiftinterfaces on Windows? We don't have ABI-stability and library evolution on Windows.

This PR will paper over the issue by emitting private swift interfaces for the stdlib and runtimes, but shouldn't be necessary: swiftlang/swift#64587

@tomerd yeah, @etcwilde is aware of this flake, it occurs on macOS as well it seems.

Yep, and it affects Linux. It affects everything, but on every other test run, and not while in a position to actually debug it.

tomerd added a commit that referenced this pull request Mar 27, 2023
…) (#6336)

motivation: with the introduction of the regsitry, there is a higher likelihood of running into duplicate packages

changes: perform a heuristics to try and determin if the packages are duplicate and provide a better diagnostics instead of the standard "duplicate product" one
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