Skip to content

Various fixes for GH-2967 #3018

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 2 commits into from
Feb 26, 2022
Merged

Conversation

asbjornu
Copy link
Member

@asbjornu asbjornu commented Feb 26, 2022

This PR should fix #2967.

Description

There are quite a bit of assumptions in the Mainline codepath that doesn't seem to hold true, such as all branches having a merge base. I believe it should be a viable fallback to look for the current branch's origin commit when a merge base cannot be found. That and a few other fixes (mostly null protection) is added to fix #2967.

Related Issue

Fixes #2967.

Motivation and Context

We don't want GitVersion to throw NullReferenceException. Ever.

How Has This Been Tested?

I have not been able to conjure a test that provokes this problem yet, but I'll try once I find time. I'm keeping this in draft until such a test is implemented.

Checklist:

  • My code follows the code style of this project.
  • My change requires a change to the documentation.
  • I have updated the documentation accordingly.
  • I have added tests to cover my changes.
  • All new and existing tests passed.

@asbjornu asbjornu force-pushed the feature/gh-2967-fix branch from 6a95104 to 4db6cb7 Compare February 26, 2022 09:48
@asbjornu
Copy link
Member Author

asbjornu commented Feb 26, 2022

As I expected, I'm unable to reproduce #2967 in a local test. The following attempted reproduction fails on AssertFullSemver, not by throwing a NullReferenceException:

[Test]
public void BranchWithoutMergeBaseMainlineBranchIsFound()
{
    var currentConfig = new Config
    {
        VersioningMode = VersioningMode.Mainline,
        AssemblyFileVersioningScheme = AssemblyFileVersioningScheme.MajorMinorPatchTag
    };

    using var fixture = new EmptyRepositoryFixture();
    fixture.Repository.MakeACommit();
    Commands.Checkout(fixture.Repository, fixture.Repository.CreateBranch("master"));
    fixture.Repository.Branches.Remove(fixture.Repository.Branches["main"]);
    fixture.Repository.MakeCommits(2);
    Commands.Checkout(fixture.Repository, fixture.Repository.CreateBranch("issue-branch"));
    fixture.Repository.MakeACommit();
    fixture.AssertFullSemver("0.1.3-issue-branch.1", currentConfig);
}

I'm adding the test nonetheless, but it's a bit unfortunate that we are unable to reproduce the NullReferenceException beforehand.

Add test for GitToolsGH-2967. Unfortunately, it does not reproduce the
`NullReferenceException`, but it may be useful as a regression test in
the future. ¯\_(ツ)_/¯
@asbjornu asbjornu marked this pull request as ready for review February 26, 2022 10:42
@asbjornu asbjornu requested a review from arturcic February 26, 2022 10:45
@arturcic arturcic enabled auto-merge February 26, 2022 14:40
@arturcic arturcic merged commit f207613 into GitTools:main Feb 26, 2022
@mergify
Copy link
Contributor

mergify bot commented Feb 26, 2022

Thank you @asbjornu for your contribution!

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.

[Bug] Object reference not set to an instance of an object during versioning solving
2 participants