Skip to content

Fall back to master if main is missing #3037

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 4 commits into from
Apr 13, 2022

Conversation

asbjornu
Copy link
Member

@asbjornu asbjornu commented Mar 8, 2022

Fall back to master if main is missing.

Description

Since the default branch was changed from master to main in #2573, some scenarios have stopped working as explained in #2590. This PR attempts to fix that by falling back to master if main is missing. Hopefully, this shouldn't break any existing tests.

Related Issue

Fixes #2590.

Motivation and Context

The change introduced in #2573 wasn't meant to be breaking, so we should do what we can to remedy any breaking it may have lead to.

How Has This Been Tested?

I haven't written any tests for this yet. I'm not sure how to reproduce it, so if all tests are still green I think this should be merged even without a regression test.

Screenshots (if appropriate):

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
Copy link
Member Author

asbjornu commented Mar 8, 2022

Thinking more about this, why aren't we using the configured branch name regex for the main branch? The main branch could be foobar without breaking, imho.

@asbjornu
Copy link
Member Author

asbjornu commented Mar 9, 2022

Hm, tests seem to fail at random. I at least don't understand what I've done in the PR which causes the following test to fail in some environments, while being successful in others.

   Shouldly.ShouldAssertException : result.ExitCode
  Failed VerifyAzurePipelinesPullRequest("refs/pull-requests/5/merge") [281 ms]
  Error Message:
    should be
0
    but was
1
  Stack Trace:
     at GitVersion.App.Tests.PullRequestInBuildAgentTest.VerifyPullRequestVersionIsCalculatedProperly(String pullRequestRef, Dictionary`2 env) in /home/runner/work/GitVersion/GitVersion/src/GitVersion.App.Tests/PullRequestInBuildAgentTest.cs:line 163
   at System.Runtime.ExceptionServices.ExceptionDispatchInfo.Throw()
   at System.Runtime.CompilerServices.TaskAwaiter.ThrowForNonSuccess(Task task)
   at System.Runtime.CompilerServices.TaskAwaiter.HandleNonSuccessAndDebuggerNotification(Task task)
   at System.Runtime.CompilerServices.TaskAwaiter.GetResult()
   at GitVersion.App.Tests.PullRequestInBuildAgentTest.VerifyAzurePipelinesPullRequest(String pullRequestRef) in /home/runner/work/GitVersion/GitVersion/src/GitVersion.App.Tests/PullRequestInBuildAgentTest.cs:line 29
   at System.Runtime.ExceptionServices.ExceptionDispatchInfo.Throw()
   at System.Runtime.CompilerServices.TaskAwaiter.ThrowForNonSuccess(Task task)
   at System.Runtime.CompilerServices.TaskAwaiter.HandleNonSuccessAndDebuggerNotification(Task task)
   at System.Runtime.CompilerServices.TaskAwaiter.ValidateEnd(Task task)
   at System.Runtime.CompilerServices.TaskAwaiter`1.GetResult()
   at NUnit.Framework.Internal.TaskAwaitAdapter.GenericAdapter`1.GetResult()
   at NUnit.Framework.Internal.AsyncToSyncAdapter.Await(Func`1 invoke)
   at NUnit.Framework.Internal.Commands.TestMethodCommand.RunTestMethod(TestExecutionContext context)
   at NUnit.Framework.Internal.Commands.TestMethodCommand.Execute(TestExecutionContext context)
   at NUnit.Framework.Internal.Execution.SimpleWorkItem.<>c__DisplayClass4_0.<PerformWork>b__0()
   at NUnit.Framework.Internal.ContextUtils.<>c__DisplayClass1_0`1.<DoIsolated>b__0(Object _)
   at System.Threading.ExecutionContext.RunInternal(ExecutionContext executionContext, ContextCallback callback, Object state)
--- End of stack trace from previous location ---
   at System.Runtime.ExceptionServices.ExceptionDispatchInfo.Throw()
   at System.Threading.ExecutionContext.RunInternal(ExecutionContext executionContext, ContextCallback callback, Object state)
   at System.Threading.ExecutionContext.Run(ExecutionContext executionContext, ContextCallback callback, Object state)
   at NUnit.Framework.Internal.ContextUtils.DoIsolated(ContextCallback callback, Object state)
   at NUnit.Framework.Internal.ContextUtils.DoIsolated[T](Func`1 func)
   at NUnit.Framework.Internal.Execution.SimpleWorkItem.PerformWork()

Do you have any ideas, @arturcic?

@arturcic
Copy link
Member

arturcic commented Mar 9, 2022

Usually these test are failing for the build agents because the environment variables are not properly reset

@asbjornu
Copy link
Member Author

asbjornu commented Mar 9, 2022

Are the environment variables not reset between different builds, or within the same build? If it's within the same build, is the problem related to parallelization? Can we make the tests execute serially to ensure a proper cleanup between each test execution?

@arturcic
Copy link
Member

arturcic commented Mar 9, 2022

Are the environment variables not reset between different builds, or within the same build? If it's within the same build, is the problem related to parallelization? Can we make the tests execute serially to ensure a proper cleanup between each test execution?

It needs more investigation

Fall back to `master` if `main` is missing. Fixes GitToolsGH-2590.
Refactor `ConfigExtensions.CalculateEffectiveConfiguration()` into
`EffectiveConfiguration.ctor()`.
@asbjornu asbjornu changed the base branch from main to support/5.x April 12, 2022 17:35
@asbjornu asbjornu force-pushed the feature/gh-2590 branch 5 times, most recently from 9c7e713 to 33a583b Compare April 13, 2022 17:46
Use the `Config` class to find the `main` branch via its configured
regex instead of using a hard coded branch name.
@asbjornu
Copy link
Member Author

Seems like the problem was the following line attempting to resolve Lazy<GitVersionContext>.Value in TrackReleaseBranchesVersionStrategy.ctor:

this.configuration = versionContext.Value.Configuration.Configuration;

I've fixed this now by resolving the configuration more lazily inside the MainTagsVersions method and will cherry-pick and push the rest of the commits which I hope will also stay green. 🤞🏼

Fall back to the `master` configuration regex if `main` is missing from
the configuration. If none of them are found, do a verbatim string
search for branches named `main` with fallback to `master`.
@asbjornu asbjornu marked this pull request as ready for review April 13, 2022 18:22
@asbjornu asbjornu requested a review from arturcic April 13, 2022 18:22
@asbjornu asbjornu merged commit 3892733 into GitTools:support/5.x Apr 13, 2022
@asbjornu asbjornu deleted the feature/gh-2590 branch April 13, 2022 18:49
@mergify
Copy link
Contributor

mergify bot commented Apr 13, 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] 5.6.5 Not finding Tag in master repo
2 participants