GitVersion icon indicating copy to clipboard operation
GitVersion copied to clipboard

Fix issue 3074 by ignoring commits in several places

Open rominator1983 opened this issue 2 years ago • 10 comments

Description

Previously commits where only ignored (by SHA or date) in one place when getting the base versions. The info if a commit is ignored or not is now produced right when getting the commit log and can then be used where needed. This info is now also evaluated when getting the increment of a single commit out of it's commit message.

Up to now there could have been commits leading to a version raise (through their commit message) even though they were ignored by date or explicitly by SHA. This has been ammended.

Related Issue

Fixes #3074.

Motivation and Context

Bit-bucket in its default configuration adds the latest 20 commit message of a source branch to the commit of a PR merge. This multiplies the occurrences of "+semver:major" commit messages in the log which can't possibly be undone if you just see this some weeks later and there are several people on the project. When doing PRs to merge from develop to a feature branch this can result in unwanted version raises.

How Has This Been Tested?

I implemented/adapted some integration tests that were red to begin with and got green when I implemented my changes. When deactivating my changes several tests got red too. Where I did refactorings I checked if their were tests for the previous code by uncommenting parts of it (See changes in BaseVersionCalculator).

Screenshots (if appropriate):

Checklist:

  • [x] My code follows the code style of this project. (I was using VS which should enforce .editorconfig)

... My change requires a change to the documentation. (The documentation already stated that you could ignore single commits by SHA which was not true for all cases. So the code now conforms better to the documented behavior)

... I have updated the documentation accordingly. (No.)

  • [x] I have added tests to cover my changes.
  • [x] All new and existing tests passed. (There were 10 red tests on main to begin with. See description in https://github.com/GitTools/GitVersion/issues/3074)

rominator1983 avatar Aug 24 '22 20:08 rominator1983

it seems to me the Mac tests have some instability since there is an error because of a corupt git repository or something.

rominator1983 avatar Aug 24 '22 21:08 rominator1983

this one seems to fail in https://github.com/GitTools/GitVersion/runs/8004381283?check_suite_focus=true on macos latest but nowhere else. Is this flaky? Would a restart (don't know if I could do that) help?

Failed CacheKeyForWorktree [2 m 2 s]
    Error Message:
     System.Exception : There was an unknown problem with the Git repository you provided
    ----> LibGit2Sharp.LibGit2SharpException : SecureTransport error: connection closed via error
    Stack Trace:
       at GitVersion.GitRepository.Clone(String sourceUrl, String workdirPath, AuthenticationInfo auth) in /Users/runner/work/GitVersion/GitVersion/src/GitVersion.LibGit2Sharp/Git/GitRepository.cs:line 139
     at GitVersion.IgnoredFilteringGitRepositoryDecorator.Clone(String sourceUrl, String workdirPath, AuthenticationInfo auth) in /Users/runner/work/GitVersion/GitVersion/src/GitVersion.LibGit2Sharp/Git/IgnoredFilteringGitRepositoryDecorator.cs:line 46
     at GitVersion.GitPreparer.<>c__DisplayClass16_0.<CloneRepository>b__0() in /Users/runner/work/GitVersion/GitVersion/src/GitVersion.Core/Core/GitPreparer.cs:line 142
     at GitVersion.Helpers.RetryAction`1.<>c__DisplayClass1_0.<Execute>b__0() in /Users/runner/work/GitVersion/GitVersion/src/GitVersion.Core/Helpers/RetryAction.cs:line 16
     at Polly.Policy`1.<>c__DisplayClass11_0.<Execute>b__0(Context _, CancellationToken _)
     at Polly.Retry.RetryEngine.Implementation[TResult](Func`3 action, Context context, CancellationToken cancellationToken, ExceptionPredicates shouldRetryExceptionPredicates, ResultPredicates`1 shouldRetryResultPredicates, Action`4 onRetry, Int32 permittedRetryCount, IEnumerable`1 sleepDurationsEnumerable, Func`4 sleepDurationProvider)
     at Polly.Retry.RetryPolicy`1.Implementation(Func`3 action, Context context, CancellationToken cancellationToken)
     at Polly.Policy`1.Execute(Func`3 action, Context context, CancellationToken cancellationToken)
     at Polly.Policy`1.Execute(Func`1 action)
     at GitVersion.Helpers.RetryAction`2.Execute(Func`1 operation) in /Users/runner/work/GitVersion/GitVersion/src/GitVersion.Core/Helpers/RetryAction.cs:line 35
     at GitVersion.Helpers.RetryAction`1.Execute(Action operation) in /Users/runner/work/GitVersion/GitVersion/src/GitVersion.Core/Helpers/RetryAction.cs:line 14
     at GitVersion.GitPreparer.CloneRepository(String repositoryUrl, String gitDirectory, AuthenticationInfo auth) in /Users/runner/work/GitVersion/GitVersion/src/GitVersion.Core/Core/GitPreparer.cs:line 142
     at GitVersion.GitPreparer.CreateDynamicRepository(String targetBranch) in /Users/runner/work/GitVersion/GitVersion/src/GitVersion.Core/Core/GitPreparer.cs:line 120
     at GitVersion.GitPreparer.PrepareInternal(GitVersionOptions gitVersionOptions) in /Users/runner/work/GitVersion/GitVersion/src/GitVersion.Core/Core/GitPreparer.cs:line 57
     at GitVersion.GitPreparer.Prepare() in /Users/runner/work/GitVersion/GitVersion/src/GitVersion.Core/Core/GitPreparer.cs:line 48
     at GitVersion.Core.Tests.GitVersionExecutorTests.CacheKeyForWorktree() in /Users/runner/work/GitVersion/GitVersion/src/GitVersion.Core.Tests/Core/GitVersionExecutorTests.cs:line 98
  --LibGit2SharpException
     at LibGit2Sharp.Core.Ensure.HandleError(Int32 result) in /_/LibGit2Sharp/Core/Ensure.cs:line 136
     at LibGit2Sharp.Core.Proxy.git_clone(String url, String workdir, GitCloneOptions& opts) in /_/LibGit2Sharp/Core/Proxy.cs:line 280
     at LibGit2Sharp.Repository.Clone(String sourceUrl, String workdirPath, CloneOptions options) in /_/LibGit2Sharp/Repository.cs:line 793
     at GitVersion.GitRepository.Clone(String sourceUrl, String workdirPath, AuthenticationInfo auth) in /Users/runner/work/GitVersion/GitVersion/src/GitVersion.LibGit2Sharp/Git/GitRepository.cs:line 114
    Standard Output Messages:
   Initialized empty Git repository in /private/var/folders/24/8k48jl6d249_n_qfxwsl6xvm0000gn/T/TestRepositories/9f4b4e91-97f8-4645-9fc3-26f4037c4639/.git/
   Created git repository at '/var/folders/24/8k48jl6d249_n_qfxwsl6xvm0000gn/T/TestRepositories/9f4b4e91-97f8-4645-9fc3-26f4037c4639'
   **Visualisation of test:**
   
   @startuml
   @enduml
   

rominator1983 avatar Aug 24 '22 21:08 rominator1983

Ok. the test CacheKeyForWorktree was breaking in 3 consecutive runs on macos latest and is green anywhere else. Curiously enough similiar code is also run in other unit tests and it's always this test that breaks. The code that breaks is already in the RetryAction. I added a [Retry(10)] to the test in order to find out more about it.

Did anyone already look into that?

rominator1983 avatar Aug 25 '22 06:08 rominator1983

I noticed that as well, the macos with netcoreapp3.1 tests. For now re-running the failed job helps, but we will sunset .netcoreapp3.1 soon anyway, so we might disable the tests for this specific configuration

arturcic avatar Aug 25 '22 08:08 arturcic

I noticed that as well, the macos with netcoreapp3.1 tests. For now re-running the failed job helps, but we will sunset .netcoreapp3.1 soon anyway, so we might disable the tests for this specific configuration

Thx for the info. Do you think I should remove the Retry-Attribute that I just added to the test? Since the code itself is also doing a retry I figured it would do no real harm to anyone.

rominator1983 avatar Aug 25 '22 11:08 rominator1983

@rominator1983 please rebase your PR on main, I have disabled the tests for macos netcoreapp3.1

arturcic avatar Aug 25 '22 11:08 arturcic

please rebase you PR on main, I have disabled the tests for macos netcoreapp3.1

arturcic avatar Aug 25 '22 11:08 arturcic

Thanks @arturcic ! I just pushed my rebase and of course removed the Retry-attribute on the test.

rominator1983 avatar Aug 25 '22 15:08 rominator1983

When do you think can this be made available?

rominator1983 avatar Sep 19 '22 10:09 rominator1983

I'm a bit uneasy about all the removed and modified tests. I would also like @ibvhefe to take this PR for a spin to see if it solves their issue.

asbjornu avatar Sep 19 '22 13:09 asbjornu

Sorry. I need this for a private project and I barely find the time. As I have no push permissions, here is a unit test that would fail. I am unsure about it. Does the test make sense?

Add this to IgnoreBeforeScenarios.cs

    [Test]
    public void CommitsWithSemverCommentsAreIgnored()
    {
        using var fixture = new EmptyRepositoryFixture();
        fixture.Repository.MakeATaggedCommit("1.0.0");
        fixture.Repository.CreateBranch("feature/foo");
        fixture.Checkout("feature/foo");
        fixture.Repository.MakeACommit("2");
        fixture.MakeACommit("3 +semver:minor");
        fixture.Checkout(MainBranch);
        fixture.Repository.MergeNoFF("feature/foo");
        var commit = fixture.Repository.MakeACommit("4 +semver:major");
        var config = new ConfigurationBuilder()
            .Add(new Config
            {
                VersioningMode = GitVersion.VersionCalculation.VersioningMode.Mainline,
                Ignore = new IgnoreConfig
                {
                    Before = DateTime.Now.AddMinutes(1)
                }
            }).Build();
        fixture.AssertFullSemver("1.0.0", config);
    }

ibvhefe avatar Sep 28 '22 21:09 ibvhefe

@ibvhefe thanks for the provided test. Does it fail on @rominator1983's fix/IgnoreAllCommits_Issue3074 branch or on GitTools:main?

asbjornu avatar Sep 29 '22 11:09 asbjornu

On both

Holen Sie sich Outlook für Androidhttps://aka.ms/AAb9ysg


From: Asbjørn Ulsberg @.> Sent: Thursday, September 29, 2022 1:23:55 PM To: GitTools/GitVersion @.> Cc: ibvhefe @.>; Mention @.> Subject: Re: [GitTools/GitVersion] Fix issue 3074 by ignoring commits in several places (PR #3173)

@ibvhefehttps://github.com/ibvhefe thanks for the provided test. Does it fail on @rominator1983https://github.com/rominator1983's fix/IgnoreAllCommits_Issue3074 branchhttps://github.com/rominator1983/GitVersion/tree/fix/IgnoreAllCommits_Issue3074 or on GitTools:main?

— Reply to this email directly, view it on GitHubhttps://github.com/GitTools/GitVersion/pull/3173#issuecomment-1262140551, or unsubscribehttps://github.com/notifications/unsubscribe-auth/ADSIBBJYZG43JBJW63RPYQDWAV34XANCNFSM57QUBR5A. You are receiving this because you were mentioned.Message ID: @.***>

ibvhefe avatar Sep 29 '22 11:09 ibvhefe

Think I'm gonna check it out then. Thanks for the feedback and most of all the test

rominator1983 avatar Sep 29 '22 12:09 rominator1983

@rominator1983 please rebase your PR on top of the current state of the main branch

arturcic avatar Mar 19 '23 10:03 arturcic

Ok guys. I'm gonna rage quit this. My original code is not applicable in any way after you let it sit there silently for half a year. So I would have to completely dig into that code AGAIN and invest the original amount of time I already put into this once more in order to find out which places would have to be edited.

And since there were a lot of changes upstream I don't exactly know if the original problem does still occur anyways. So...

rominator1983 avatar Mar 20 '23 06:03 rominator1983