Skip to content
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

StackOverflowException resolving NuGet package without version constraint from large paket.dependencies #1392

Closed
viktor-svub opened this issue Jan 15, 2016 · 11 comments

Comments

@viktor-svub
Copy link
Contributor

StackOverflowException resolving "our.proprietary.component"

  • Paket.exe 2.44.1, version 2.21 works but does not handle prereleases
  • paket.dependencies NuGet, declared without any version constraint
  • there are about 180 declared dependencies, all NuGet, all custom feeds
  • probably also transitive dependency of some of the other component/s
  • SO is thrown in \src\Paket.Core\Domain.fs, line 30, ckeckout cf9c622
    (this.GetCompareString().CompareTo(that.GetCompareString()))
  • stack trace @ https://gist.github.com/viktor-svub/c1563f0c2b98562f67ee
@forki
Copy link
Member

forki commented Jan 15, 2016

can you please try to find the last working version?

@viktor-svub
Copy link
Contributor Author

OK, the issue appears between 2.40.14 (latest working) and 2.40.15 (throws SO).

  • the full command crashing with exception is paket.exe update --no-install
  • we have 4 custom NuGet feeds, often containing different versions of same packages
  • majority of the 180 immediate dependencies are prerelease
  • majority of all the dependencies are using @~> resolution
  • the generated paket.lock is about 360 lines long

@forki
Copy link
Member

forki commented Jan 15, 2016

diff --git a/src/Paket.Core/SemVer.fs b/src/Paket.Core/SemVer.fs
index 444e688..d466672 100644
--- a/src/Paket.Core/SemVer.fs
    b/src/Paket.Core/SemVer.fs
@@ -144,6  144,8 @@ interface System.IComparable with
                     | None, None -> 0
                     | Some p, None -> -1
                     | None, Some p -> 1
                     | Some p, _ when p.ToString() = "prerelease" -> -1
                     | _, Some p when p.ToString() = "prerelease" -> 1
                     | Some left, Some right -> compare left right

             | _ -> invalidArg "yobj" "cannot compare values of different types"

that's the only change. weird

@forki
Copy link
Member

forki commented Jan 15, 2016

but I assume the real issue is that our resolver algorithm is still recursive. Need to implement BFS qith a queue...

@forki
Copy link
Member

forki commented Jan 15, 2016

@viktor-svub in the latest version I inline the tryToImprove function. This will "free" halve of the stack in your sample and should help to process significantly larger graphs. Does this work for you?

@mrinaldi do you see a chance that we can remove the recursive step function and turn it into something that manages a queue or a stack internally?

@mrinaldi
Copy link
Contributor

IIRC yes.
I'm on vacation right now, though. I'm not sure when I'll have time to look at it.
Maybe sometime next week.

@gpomykala
Copy link

I noticed the same problem (SO during paket update --no-install). The last working version is 2.40.14, 2.44.5 still fails

@forki
Copy link
Member

forki commented Jan 18, 2016

can any of you reproduce with a dependencies file that can be made public?

@gpomykala
Copy link

We were not able to reproduce it using public packages only yet. it's a mix of public libs and internal libs

@forki
Copy link
Member

forki commented Jan 22, 2016

maybe exclude the internal stuff and add public packages until it breaks again?

@forki
Copy link
Member

forki commented Feb 11, 2016

This fixed in 2.50.3

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

No branches or pull requests

4 participants