Carthage: New resolver crash with incompatible requirements

[vytis]

• carthage install method: [ ] .pkg, [ ] homebrew, [x] source
• which carthage: Running from Xcode
• carthage version: Latest master (088ab9982cc483f87e0ae20532dc437285c1dc5c)
• xcodebuild -version: Xcode 9.3 (9E145)
• Are you using --no-build? No
• Are you using --no-use-binaries? No
• Are you using --use-submodules? No
• Are you using --cache-builds? No
• Are you using --new-resolver? Yes

Cartfile

github "wireapp/wire-ios-request-strategy" ~> 83.0
github "wireapp/avs-ios-binaries" "4.1.131"
github "wireapp/ZipArchive" "v2.1.2"

Cartfile.private

github "wireapp/wire-ios-mocktransport" ~> 74.0
github "wireapp/wire-ios-testing" ~> 14.0

Carthage Output

*** Fetching wire-ios-testing
*** Fetching wire-ios-request-strategy
*** Fetching wire-ios-mocktransport
*** Fetching wire-ios-system
*** Fetching ocmock
*** Fetching wire-ios-data-model
*** Fetching wire-ios-protos
*** Fetching wire-ios-cryptobox
*** Fetching wire-ios-transport

Caught signal triggered by the Swift runtime!
Illegal instruction

Unfortunately, this is probably a bug in Swift and not Carthage. If
this is preventing you from doing work, please file an issue and we'll
do our best to work around it:
https://github.com/Carthage/Carthage/issues/new

Please also consider filing a radar with Apple, containing the version
of Carthage and any crash report found in Console.app.

Actual outcome
Carthage prints lots of output to console with no meaningful information.

Expected outcome
Without using --new-resolver there is at least some error that it's about invalid dependencies:

*** Fetching wire-ios-mocktransport
*** Fetching wire-ios-request-strategy
*** Fetching wire-ios-testing
*** Fetching wire-ios-transport
*** Fetching ocmock
*** Fetching wire-ios-cryptobox
*** Fetching wire-ios-system
*** Fetching wire-ios-protos
*** Fetching wire-ios-data-model
*** Fetching protobuf-objc
*** Fetching wire-ios-utilities
*** Fetching wire-ios-images
*** Fetching wire-ios-link-preview
*** Fetching ono
*** Fetching PINCache
Could not pick a version for github "wireapp/wire-ios-system", due to mutually incompatible requirements:
    ~> 20.0.0 (github "wireapp/wire-ios-testing")
    ~> 16.0.0 (github "wireapp/wire-ios-images")

Investigation

It's pretty easy to reproduce this error with Xcode attached and the problem seems to be force-unwrapping the error:

I could give it a go to fix it, but some hints would be appreciated! When I tried stepping over the code it seems to produce the correct error but at some point during the recursive process the permutations becomes empty...

[jdhealy]

/cc @BobElDevil

[jdhealy]

I might, by happenstance, already have an unpublished fix for this — let me verify and report back.

[jdhealy]

Alright! Just tested that patch out and saw your expected result get output.

Will get this up as a PR, once I figure out a good way to make a unit test from this. 👍

diff --git a/Source/CarthageKit/NewResolver.swift b/Source/CarthageKit/NewResolver.swift
index b402355b..1eaf7e2c 100644
--- a/Source/CarthageKit/NewResolver.swift
+++ b/Source/CarthageKit/NewResolver.swift
@@ -100,22 +100,30 @@ public struct NewResolver: ResolverProtocol {
                ) -> Result<DependencyGraph, CarthageError> {
                return self.nodePermutations(for: dependencies, in: baseGraph, withParent: parent)
                        .flatMap { permutations in
-                               // Only throw an error if no valid graph was produced by any of the permutations
-                               var errResult: Result<DependencyGraph, CarthageError>? = nil
-                               for nextGraphResult in permutations {
-                                       // Immediately fail if graph creation fails
-                                       guard case let .success(nextGraph) = nextGraphResult else { return errResult! }
-
-                                       let nextResult = self.process(graph: nextGraph)
-                                       switch nextResult {
-                                       case .success:
-                                               return nextResult
-                                       case .failure:
-                                               errResult = errResult ?? nextResult
+                               return sequence(state: (
+                                       permutations.makeIterator(),
+                                       guaranteedStop: false
+                               )) {
+                                       guard $0.1 == false else { return nil /* because the last value emitted was the stopper */ }
+
+                                       guard let dependencyGraphOrStopworthyError = $0.0.next() /* from iterator */ else { return nil }
+
+                                       switch dependencyGraphOrStopworthyError.map(self.process) {
+                                       case .failure(let stopworthyError):
+                                               $0.1 = true // guaranteedStop
+                                               return .failure(stopworthyError)
+                                       case .success(.success(let processedGraph)):
+                                               $0.1 = true // guaranteedStop
+                                               return .success(processedGraph)
+                                       case .success(.failure(let error)):
+                                               // might be last element, might not
+                                               return .failure(error)
                                        }
-                               }
-                               return errResult!
-                       }
+                               }.suffix(1).makeIterator().next() ?? .failure(
+                                       CarthageError.internalError(description: "no elements returned from iteration over `permutations`")
+                               )
+               }
+
        }

        /// Processes the next unvisited node in the graph.

[BobElDevil]

@jdhealy I know that @jasonboyle was actually looking at this same issue recently. Mentioning him here in case he has an in progress branch we can compare notes with.

I believe the underlying cause is that we ran into a situation where even though the NodePermutations object was non-empty, it returned nil on it's very first pass because all states contained in it are in the errorCache.

Ideally the fix would be to figure out what the error returned by the errorCache was, so we could throw that error instead of a Carthage internal error.

[jdhealy]

@BobElDevil, okay, so unlike @vytis’s Cartfiles when resolved with the WIP patch I posted, you and @jasonboyle experienced a crash/issue, which you believe would hit the internalError:

(description: "no elements returned from iteration over `permutations`")

…from my patch.

Unlike @vytis’s Cartfiles which output the might be last element, might not .failure:

Could not pick a version for github "wireapp/wire-ios-system", due to mutually incompatible requirements:
    ~> 20.0.0 (github "wireapp/wire-ios-testing")
    ~> 16.0.0 (github "wireapp/wire-ios-images")

Did I interpret that correctly?

[BobElDevil]

@jdhealy No, I think they're the same issue. @vytis described a crash before your patch, because it was unwrapping the error, so it's the same issue (a non-empty node permutation struct still returning nothing from its iteration). The thing to keep in mind is that when we throw a failure that just means we will continue down a different permutation path, so when the failure in your patch gets thrown, it continued on, eventually running out of permutations and throwing the last error it hit, which is that incompatibility you mentioned

[jdhealy]

so when the failure in your patch gets thrown, it continued on, eventually running out of permutations and throwing the last error it hit, which is that incompatibility you mentioned

@BobElDevil, about what I reported 😅—

so, for carthage bootstrap (which I was testing with) the --new-resolver flag gets disregarded and my patch never got executed — so, as such, I ended up seeing the same as the expected result (which is what one would see without the --new-resolver).

after patching:

-                   return project.updateDependencies(shouldCheckout: options.checkoutAfterUpdate, buildOptions: options.buildOptions)
+                   return project.updateDependencies(shouldCheckout: options.checkoutAfterUpdate, useNewResolver: true, buildOptions: options.buildOptions)

and running ./carthage bootstrap --new-resolver --no-build --no-use-binaries with @vytis’s provided Cartfiles, I _did hit_ the internalError:

(description: "no elements returned from iteration over permutations")

seems to me that you were right on with:

I believe the underlying cause is that we ran into a situation where even though the NodePermutations object was non-empty, it returned nil on it's very first pass because all states contained in it are in the errorCache.

[werner77]

Just verified with my rewritten resolver (https://github.com/Carthage/Carthage/pull/2455) and it works find with this test case.