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.

Sign up for GitHub

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

Jump to bottom

#2034 StackOverflow when using increment: Inherit: new unit test #2059

Closed
wants to merge 2 commits into from
Closed

#2034 StackOverflow when using increment: Inherit: new unit test #2059

wants to merge 2 commits into from

Conversation

dzenchar
Copy link

This PR consists of the new unit test that reproduces an issue #2034 StackOverflow exception while using increment: Inherit config, and branches merging

@dzenchar dzenchar changed the title #2034 StackOverflow when using increment: Inheri: new unit test #2034 StackOverflow when using increment: Inherit: new unit test Jan 21, 2020
dzenchar
dzenchar commented Jan 21, 2020
View reviewed changes
src/GitVersionCore.Tests/IntegrationTests/MainlineDevelopmentMode.cs

fixture.Checkout("develop");
fixture.MergeNoFF("master");
fixture.AssertFullSemver(currentConfig, "0.1.3-alpha.1");
Copy link
Author

Choose a reason for hiding this comment

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

StackOverflow exception here

@asbjornu
Copy link
Member

Hm. I had hoped the test failing would give us a stack trace from where the StackOverflowException occurred, allowing us to pinpoint the line of code causing the issue. Seems like we need to invest a bit more into debugging this, as is often the case when StackOverflowException is involved. Well, well. Thanks for the failing test, @dzenchar! 😃

@asbjornu asbjornu added the bug label Jan 21, 2020
@dzenchar
Copy link
Author

Hi. Thanks to you!
I was able to get the stack trace locally, see below:

GitVersionCore.dll!GitVersion.Configuration.BranchConfigurationCalculator.CalculateWhenMultipleParents(LibGit2Sharp.IRepository repository, LibGit2Sharp.Commit currentCommit, ref LibGit2Sharp.Branch currentBranch, LibGit2Sharp.Branch[] excludedBranches) Line 190
	at C:\Repos\GitVersion\src\GitVersionCore\Configuration\BranchConfigurationCalculator.cs(190)
GitVersionCore.dll!GitVersion.Configuration.BranchConfigurationCalculator.InheritBranchConfiguration(LibGit2Sharp.Branch targetBranch, GitVersion.Configuration.BranchConfig branchConfiguration, System.Collections.Generic.IList<LibGit2Sharp.Branch> excludedInheritBranches) Line 63
	at C:\Repos\GitVersion\src\GitVersionCore\Configuration\BranchConfigurationCalculator.cs(63)
GitVersionCore.dll!GitVersion.Configuration.BranchConfigurationCalculator.GetBranchConfiguration(LibGit2Sharp.Branch targetBranch, System.Collections.Generic.IList<LibGit2Sharp.Branch> excludedInheritBranches) Line 40
	at C:\Repos\GitVersion\src\GitVersionCore\Configuration\BranchConfigurationCalculator.cs(40)
GitVersionCore.dll!GitVersion.Configuration.BranchConfigurationCalculator.InheritBranchConfiguration(LibGit2Sharp.Branch targetBranch, GitVersion.Configuration.BranchConfig branchConfiguration, System.Collections.Generic.IList<LibGit2Sharp.Branch> excludedInheritBranches) Line 170
	at C:\Repos\GitVersion\src\GitVersionCore\Configuration\BranchConfigurationCalculator.cs(170)
GitVersionCore.dll!GitVersion.Configuration.BranchConfigurationCalculator.GetBranchConfiguration(LibGit2Sharp.Branch targetBranch, System.Collections.Generic.IList<LibGit2Sharp.Branch> excludedInheritBranches) Line 40
	at C:\Repos\GitVersion\src\GitVersionCore\Configuration\BranchConfigurationCalculator.cs(40)
GitVersionCore.dll!GitVersion.Configuration.BranchConfigurationCalculator.InheritBranchConfiguration(LibGit2Sharp.Branch targetBranch, GitVersion.Configuration.BranchConfig branchConfiguration, System.Collections.Generic.IList<LibGit2Sharp.Branch> excludedInheritBranches) Line 170
	at C:\Repos\GitVersion\src\GitVersionCore\Configuration\BranchConfigurationCalculator.cs(170)
GitVersionCore.dll!GitVersion.Configuration.BranchConfigurationCalculator.GetBranchConfiguration(LibGit2Sharp.Branch targetBranch, System.Collections.Generic.IList<LibGit2Sharp.Branch> excludedInheritBranches) Line 40
	at C:\Repos\GitVersion\src\GitVersionCore\Configuration\BranchConfigurationCalculator.cs(40)
GitVersionCore.dll!GitVersion.Configuration.BranchConfigurationCalculator.InheritBranchConfiguration(LibGit2Sharp.Branch targetBranch, GitVersion.Configuration.BranchConfig branchConfiguration, System.Collections.Generic.IList<LibGit2Sharp.Branch> excludedInheritBranches) Line 170

... (a long cycle of repeated calls)

	at C:\Repos\GitVersion\src\GitVersionCore\Configuration\BranchConfigurationCalculator.cs(170)
GitVersionCore.dll!GitVersion.Configuration.BranchConfigurationCalculator.GetBranchConfiguration(LibGit2Sharp.Branch targetBranch, System.Collections.Generic.IList<LibGit2Sharp.Branch> excludedInheritBranches) Line 40
	at C:\Repos\GitVersion\src\GitVersionCore\Configuration\BranchConfigurationCalculator.cs(40)
GitVersionCore.dll!GitVersion.Configuration.BranchConfigurationCalculator.InheritBranchConfiguration(LibGit2Sharp.Branch targetBranch, GitVersion.Configuration.BranchConfig branchConfiguration, System.Collections.Generic.IList<LibGit2Sharp.Branch> excludedInheritBranches) Line 170
	at C:\Repos\GitVersion\src\GitVersionCore\Configuration\BranchConfigurationCalculator.cs(170)
GitVersionCore.dll!GitVersion.Configuration.BranchConfigurationCalculator.GetBranchConfiguration(LibGit2Sharp.Branch targetBranch, System.Collections.Generic.IList<LibGit2Sharp.Branch> excludedInheritBranches) Line 40
	at C:\Repos\GitVersion\src\GitVersionCore\Configuration\BranchConfigurationCalculator.cs(40)
GitVersionCore.dll!GitVersion.GitVersionContext.CalculateEffectiveConfiguration() Line 92
	at C:\Repos\GitVersion\src\GitVersionCore\GitVersionContext.cs(92)
GitVersionCore.dll!GitVersion.GitVersionContext.GitVersionContext(LibGit2Sharp.IRepository repository, GitVersion.Logging.ILog log, LibGit2Sharp.Branch currentBranch, GitVersion.Configuration.Config configuration, bool onlyTrackedBranches, string commitId) Line 63
	at C:\Repos\GitVersion\src\GitVersionCore\GitVersionContext.cs(63)
GitVersionCore.dll!GitVersion.GitVersionContext.GitVersionContext(LibGit2Sharp.IRepository repository, GitVersion.Logging.ILog log, string targetBranch, GitVersion.Configuration.Config configuration, bool onlyTrackedBranches, string commitId) Line 18
	at C:\Repos\GitVersion\src\GitVersionCore\GitVersionContext.cs(18)
GitVersionCore.Tests.dll!GitVersionCore.Tests.GitToolsTestingExtensions.GetVersion(GitTools.Testing.RepositoryFixtureBase fixture, GitVersion.Configuration.Config configuration, LibGit2Sharp.IRepository repository, string commitId, bool onlyTrackedBranches, string targetBranch) Line 34
	at C:\Repos\GitVersion\src\GitVersionCore.Tests\GitToolsTestingExtensions.cs(34)
GitVersionCore.Tests.dll!GitVersionCore.Tests.GitToolsTestingExtensions.AssertFullSemver(GitTools.Testing.RepositoryFixtureBase fixture, GitVersion.Configuration.Config configuration, string fullSemver, LibGit2Sharp.IRepository repository, string commitId, bool onlyTrackedBranches, string targetBranch) Line 62
	at C:\Repos\GitVersion\src\GitVersionCore.Tests\GitToolsTestingExtensions.cs(62)
GitVersionCore.Tests.dll!GitVersionCore.Tests.IntegrationTests.MainlineDevelopmentMode.MergingMasterBranchToDevelopWithInheritIncrementShouldIncrementDevelopPatch() Line 453
	at C:\Repos\GitVersion\src\GitVersionCore.Tests\IntegrationTests\MainlineDevelopmentMode.cs(453)

@asbjornu
Copy link
Member

Thanks for the stack trace! It reduces the problem down to these two lines, which invoke each other perpetually:

GitVersion/src/GitVersionCore/Configuration/BranchConfigurationCalculator.cs

Line 40 in f0e5275

matchingBranches = InheritBranchConfiguration(targetBranch, matchingBranches, excludedInheritBranches);

GitVersion/src/GitVersionCore/Configuration/BranchConfigurationCalculator.cs

Line 170 in f0e5275

var inheritingBranchConfig = GetBranchConfiguration(chosenBranch, excludedInheritBranches);

I find it a bit troubling that GetBranchConfiguration() and InheritBranchConfiguration() both invokes the other. Circular dependencies should be avoided, I think. A refactoring of both methods to rely on a third (terminal) one would be good, I suppose. There's an attempt to guard against infinite loops on line 151, but it doesn't work in this case:

GitVersion/src/GitVersionCore/Configuration/BranchConfigurationCalculator.cs

Lines 150 to 151 in f0e5275

// To prevent infinite loops, make sure that a new branch was chosen.
if (targetBranch.IsSameBranch(chosenBranch))

I think the following comment describes the situation well:

GitVersion/src/GitVersionCore/Configuration/BranchConfigurationCalculator.cs

Line 51 in f0e5275

// TODO I think we need to take a fresh approach to this.. it's getting really complex with heaps of edge cases

Base automatically changed from master to main January 31, 2021 12:46
@arturcic arturcic force-pushed the main branch 4 times, most recently from 9801764 to b7a7608 Compare March 4, 2023 13:33
@arturcic arturcic mentioned this pull request Mar 19, 2023
Closed
7 tasks
@arturcic
Copy link
Member

Closed in favor of #3445

@arturcic arturcic closed this Mar 19, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers
No reviews
Assignees
No one assigned
Labels
bug failing test case help wanted
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
3 participants
@dzenchar @asbjornu @arturcic