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

System.InvalidOperationException - ignore commits-before in GitVersion.yml #2122

Closed
realrubberduckdev opened this issue Feb 21, 2020 · 20 comments · Fixed by #2318
Closed

System.InvalidOperationException - ignore commits-before in GitVersion.yml #2122

realrubberduckdev opened this issue Feb 21, 2020 · 20 comments · Fixed by #2318
Labels
Milestone

Comments

@realrubberduckdev
Copy link

realrubberduckdev commented Feb 21, 2020

There are multiple folders in our repository with separate release cadence and different versions.

Adding a new folder, we would like it versioned starting from 0.0.0 hence we want it not to count the commits before the date it got added. The GitVersion.yml looks as below:

assembly-versioning-scheme: None
mode: Mainline
next-version: 0.0.0
branches: {}
ignore:
  commits-before: '2020-02-17T13:57:12'
increment: Inherit
commit-message-incrementing: MergeMessageOnly

Running gitversion.exe or dotnet-gitversion.exe in the folder throws an exception:

An unexpected error occurred:
System.InvalidOperationException: Sequence contains no elements
   at System.Linq.ThrowHelper.ThrowNoElementsException()
   at System.Linq.Enumerable.Aggregate[TSource](IEnumerable`1 source, Func`3 func)
   at GitVersion.VersionCalculation.BaseVersionCalculator.GetBaseVersion(GitVersionContext context)
   at GitVersion.VersionCalculation.NextVersionCalculator.FindVersion(GitVersionContext context)
   at GitVersion.GitVersionFinder.FindVersion(GitVersionContext context)
   at GitVersion.GitVersionCalculator.<>c__DisplayClass14_0.<ExecuteInternal>b__0(IRepository repo)
   at GitVersion.Extensions.LibGitExtensions.WithRepository[TResult](String dotGitDirectory, Func`2 action)
   at GitVersion.GitVersionCalculator.ExecuteInternal(String targetBranch, String commitId, Config overrideConfig)
   at GitVersion.GitVersionCalculator.GetCachedGitVersionInfo(String targetBranch, String commitId, Config overrideConfig, Boolean noCache)
   at GitVersion.GitVersionCalculator.CalculateVersionVariables()
   at GitVersion.ExecCommand.Execute()
   at GitVersion.GitVersionExecutor.VerifyArgumentsAndRun(Arguments arguments) 

It seems to works without the section below to get a version.

ignore:
  commits-before: '2020-02-17T13:57:12'

But it gets 2.0.235 or so which is not how we want the contents of that folder versioned.

Versions tried with:
gitversion /version
5.1.3+Branch.master.Sha.bef8ebc0b62b3ddd0cdafe09b66d68bbfcaf90d5
dotnet-gitversion.exe /version
5.1.3+Branch.master.Sha.bef8ebc0b62b3ddd0cdafe09b66d68bbfcaf90d5

Also tried with:
dotnet-gitversion.exe /version
5.1.4-beta.1+172.Branch.master.Sha.c602fbb304b8f39aecb39656f921c5898414283d

Are we doing something wrong?
Can anyone guide us please.

@asbjornu
Copy link
Member

Sorry, but GitVersion is not designed to yield different version numbers on different paths in a single repository. Simply put: GitVersion does not support Monorepo. The only way to control how a specific part of a repo is versioned is to split it out into its own Git repository.

@realrubberduckdev
Copy link
Author

Hi @asbjornu Thanks for your response. Got a few follow-up questions.

The documentation seems to imply we can version folder differently or have we misunderstood it?

It can be the root repository directory or any subdirectory in case you have a single repository for more than one project or are restricted to commit into a subdirectory.

It, implying GitVersion.yml.

And the way we are currently trying to do, should it not work by ignoring all commits before a certain date and version then on?
The exception is not expected or is it?

@asbjornu
Copy link
Member

asbjornu commented Feb 26, 2020

This flexibility stems more from the ability to have GitVersion.yml placed anywhere in the repository (or even somewhere else) than to support Monorepo. When you configure ignore: commits-before: '2020-02-17T13:57:12', you will exclude a lot of commits that may be important for GitVersion to find a proper version source for a given commit. GitVersion failing with Sequence contains no elements tells you just that: It couldn't find any elements (commits) in a code path required to calculate a version number.

Now, I think it should be possible for GitVersion to ignore this exact exception and continue with a fallback strategy and only fail if no strategies yields a version number. So if you are able to figure out how to untie that knot in the codebase, we would warmly welcome a PR. 😃 🙏

@realrubberduckdev
Copy link
Author

Well played @asbjornu 😆

Will see what I can do. Bit busy so cannot promise anytime soon. If nobody picks this up will look to revisit. Thanks

@stale
Copy link

stale bot commented May 26, 2020

This issue has been automatically marked as stale because it has not had recent activity. After 30 days from now, it will be closed if no further activity occurs. Thank you for your contributions.

@stale stale bot added the stale label May 26, 2020
@DamianKedzior
Copy link
Contributor

Now, I think it should be possible for GitVersion to ignore this exact exception and continue with a fallback strategy and only fail if no strategies yields a version number. So if you are able to figure out how to untie that knot in the codebase, we would warmly welcome a PR. 😃 🙏

That's my thinking as well, especially when you can see in the logs:
Fallback base version: 0.1.0
which is not being used at the end and exception is being thrown.

@stale stale bot removed the stale label May 29, 2020
@asbjornu
Copy link
Member

Exactly!

@DamianKedzior
Copy link
Contributor

DamianKedzior commented Jun 1, 2020

I've started looking into this issue and so far I can't replicate it with integration test but I can replicate it when I run GitVersion tool on newly created git repo.
This is the test code (which passes):

[Test]
public void ShouldFallbackToDefaultVersionWhenAllCommitsAreSkipped2()
{
	var config = new Config
	{
		VersioningMode = GitVersion.VersionCalculation.VersioningMode.Mainline,
		Ignore = new IgnoreConfig
		{
			Before = DateTimeOffset.Now.AddHours(100)
		}
	};

	using var fixture = new EmptyRepositoryFixture();
	fixture.Repository.MakeACommit();

	fixture.AssertFullSemver("0.1.0+0", config);
}

I have noticed that configuration I pass to AssertFullSemver is not respected. While debugging I found out that IgnoreConfig has been cleared and VersioninMode has been set to ContinousDelivery.

Am I missing something obvious?

EDIT:
Found out why. Override config for tests only overrides TagPrefix. Adding more overrides helped me replicate the issue.

@DamianKedzior
Copy link
Contributor

Looks like the issue is caused by VersionFilters which apply to all version strategies. I think FallbackVersionStrategy should be excluded from version filtering. @asbjornu any thoughts?

@asbjornu
Copy link
Member

asbjornu commented Jun 2, 2020

Yes, agreed. Would you care to submit a pull request implementing this change? I just hope we don't have (many) tests depending on this. I don't think we do.

@DamianKedzior
Copy link
Contributor

I am working on the PR. I haven't found any integration tests for ignore configuration, probably that's why there are some bugs there.
I have fixed this exception already but I am adding more tests and discovering other issues.

@asbjornu
Copy link
Member

asbjornu commented Jun 2, 2020

If you're wandering into Configuration-land, please keep an eye on #2300 which refactors it quite a bit. You may want to put off your PR until after it is merged if you're going to touch many of the same files, @DamianKedzior.

@DamianKedzior
Copy link
Contributor

@asbjornu I am aware of #2300 and I am happy to see it coming. I will keep my modifications to minimum in Configuration classes, so far I think I have only 2 lines addition. If #2300 will be merged than I will just back-merge and refactor the code.
I think I will get back to work on this issue on the weekend.

@asbjornu
Copy link
Member

asbjornu commented Jun 8, 2020

@DamianKedzior, #2300 is now merged.

@DamianKedzior
Copy link
Contributor

@asbjornu , I have opened the PR which fixes this issue but I have discovered other issues related to IgnoreoConfig. For example this test will fail:

[Test]
public void ShouldFallbackToBaseVersionWhenAllCommitsAreIgnored2()
{
	using var fixture = new EmptyRepositoryFixture();
	fixture.Repository.MakeATaggedCommit("1.0.0");
	fixture.Repository.MakeATaggedCommit("2.0.0");
	var commit = fixture.Repository.MakeACommit();

	var config = new ConfigurationBuilder()
		.Add(new Config
		{
			Ignore = new IgnoreConfig
			{
				Before = commit.When()
			}
		}).Build();

	fixture.AssertFullSemver("0.1.0+0", config);
	// Actual: 0.1.0+2
}

Should I open separate issue with failing test? I was hoping to fix it within this issue but it looks like more complex issue.
When we fallback to FallbackVersionStrategy BaseVersionSource points to the first commit in the current branch, even if this commit based on IgnoreConfig should be ignored. Unfortunately settings BaseVersionSource to null is causing exceptions further in the code.

@asbjornu
Copy link
Member

@DamianKedzior, hm. I'm not sure I understand what you're expecting and I'm not sure what's the best way to deal with this and what exactly IgnoreConfig.Before is meant to do. It could mean at least two different things:

  1. Ignore the entire commit history before this date.
  2. Ignore the possible version sources before this date.

I think alternative 2 is the current implementation and as you can see by the generated version number 0.1.0+2 it does ignore the version sources, however it still counts the number of commits since the source it finds.

@DamianKedzior
Copy link
Contributor

Exactly, looks like (2) is current behavior but it still counts the commits. I am not sure if this was intended or a bug. Probably it's not a big deal but my understanding of IgnoreConfig.Before was that it will ignore all the commits before specific date so it shouldn't count the number of commits since the beginning.
On the other hand it's not a big deal if it counts them.

@asbjornu
Copy link
Member

I think this can be argued one way or another. I don't know the intention behind the original design, but I think it's reasonable that only version sources are ignored and not all commits for counting.

In other words; IgnoreConfig.Before ignores the major.minor.patch part that any commits before the given date may affect, but not whatever information those same commits may contribute to the build metadata. I think that's fine, but it should probably be documented (better).

@DamianKedzior
Copy link
Contributor

Definitely better documentation would help better understanding how this feature works.

@arturcic arturcic linked a pull request Jun 15, 2020 that will close this issue
5 tasks
@arturcic arturcic added the bug label Jun 15, 2020
@arturcic arturcic added this to the 5.3.x milestone Jun 15, 2020
@github-actions
Copy link

🎉 This issue has been resolved in version 5.3.6 🎉
The release is available on:

Your GitReleaseManager bot 📦🚀

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging a pull request may close this issue.

4 participants