Skip to content

Commit c2e4d50

Browse files
authored
Merge pull request #682 from dotnet/fixGetCommits
Fixes `nbgv get-commits` to require version matching
2 parents c60a072 + 19a28c5 commit c2e4d50

File tree

4 files changed

+65
-44
lines changed

4 files changed

+65
-44
lines changed

src/NerdBank.GitVersioning.Tests/GitExtensionsTests.cs

Lines changed: 12 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -129,6 +129,18 @@ public void GetCommitsFromVersion_WithPathFilters()
129129
LibGit2GitExtensions.GetCommitsFromVersion(this.Context, new Version(1, 2, 3)).OrderBy(c => c.Sha));
130130
}
131131

132+
[Fact]
133+
public void GetCommitsFromVersion_WithMajorMinorChecks()
134+
{
135+
Commit v1_0_50 = this.WriteVersionFile(new VersionOptions { Version = SemanticVersion.Parse("1.0.50-preview.{height}") });
136+
Commit v1_1_50 = this.WriteVersionFile(new VersionOptions { Version = SemanticVersion.Parse("1.1.50-preview.{height}") });
137+
138+
Assert.Empty(LibGit2GitExtensions.GetCommitsFromVersion(this.Context, new Version(1, 0)));
139+
Assert.Empty(LibGit2GitExtensions.GetCommitsFromVersion(this.Context, new Version(1, 0, 49)));
140+
Assert.Equal(v1_0_50, Assert.Single(LibGit2GitExtensions.GetCommitsFromVersion(this.Context, new Version(1, 0, 50))));
141+
Assert.Equal(v1_1_50, Assert.Single(LibGit2GitExtensions.GetCommitsFromVersion(this.Context, new Version(1, 1, 50))));
142+
}
143+
132144
[Theory]
133145
[InlineData("2.2", "2.2-alpha.{height}", 1, 1, true)]
134146
[InlineData("2.2", "2.3", 1, 1, true)]

src/NerdBank.GitVersioning/LibGit2/LibGit2GitExtensions.cs

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -174,7 +174,7 @@ public static IEnumerable<Commit> GetCommitsFromVersion(LibGit2Context context,
174174
var tracker = new GitWalkTracker(context);
175175
var possibleCommits = from commit in GetCommitsReachableFromRefs(context.Repository)
176176
let commitVersionOptions = tracker.GetVersion(commit)
177-
where commitVersionOptions is not null
177+
where commitVersionOptions?.Version?.IsMatchingVersion(version) is true
178178
where !IsCommitIdMismatch(version, commitVersionOptions, commit)
179179
where !IsVersionHeightMismatch(version, commitVersionOptions, commit, tracker)
180180
select commit;

src/NerdBank.GitVersioning/SemanticVersion.cs

Lines changed: 51 additions & 25 deletions
Original file line numberDiff line numberDiff line change
@@ -152,6 +152,28 @@ internal SemanticVersion.Position? VersionHeightPosition
152152
}
153153
}
154154

155+
/// <summary>
156+
/// Gets the position in a computed version that the first 16 bits of a git commit ID should appear, if any.
157+
/// </summary>
158+
internal SemanticVersion.Position? GitCommitIdPosition
159+
{
160+
get
161+
{
162+
// We can only store the git commit ID info after there was a place to put the version height.
163+
// We don't want to store the commit ID (which is effectively a random integer) in the revision slot
164+
// if the version height does not appear, or only appears later (in the -prerelease tag) since that
165+
// would mess up version ordering.
166+
if (this.VersionHeightPosition == SemanticVersion.Position.Build)
167+
{
168+
return SemanticVersion.Position.Revision;
169+
}
170+
else
171+
{
172+
return null;
173+
}
174+
}
175+
}
176+
155177
/// <summary>
156178
/// Gets a value indicating whether this instance is the default "0.0" instance.
157179
/// </summary>
@@ -286,40 +308,44 @@ internal static bool WillVersionChangeResetVersionHeight(SemanticVersion first,
286308
return false;
287309
}
288310

289-
internal static int ReadVersionPosition(Version version, SemanticVersion.Position position)
311+
internal static int ReadVersionPosition(Version version, Position position)
290312
{
291313
Requires.NotNull(version, nameof(version));
292314

293-
switch (position)
315+
return position switch
294316
{
295-
case SemanticVersion.Position.Major:
296-
return version.Major;
297-
case SemanticVersion.Position.Minor:
298-
return version.Minor;
299-
case SemanticVersion.Position.Build:
300-
return version.Build;
301-
case SemanticVersion.Position.Revision:
302-
return version.Revision;
303-
default:
304-
throw new ArgumentOutOfRangeException(nameof(position), position, "Must be one of the 4 integer parts.");
305-
}
317+
Position.Major => version.Major,
318+
Position.Minor => version.Minor,
319+
Position.Build => version.Build,
320+
Position.Revision => version.Revision,
321+
_ => throw new ArgumentOutOfRangeException(nameof(position), position, "Must be one of the 4 integer parts."),
322+
};
306323
}
307324

308-
internal int ReadVersionPosition(SemanticVersion.Position position)
325+
internal int ReadVersionPosition(Position position) => ReadVersionPosition(this.Version, position);
326+
327+
/// <summary>
328+
/// Checks whether a given version may have been produced by this semantic version.
329+
/// </summary>
330+
/// <param name="version">The version to test.</param>
331+
/// <returns><see langword="true"/> if the <paramref name="version"/> is a match; <see langword="false"/> otherwise.</returns>
332+
internal bool IsMatchingVersion(Version version)
309333
{
310-
switch (position)
334+
Position lastPositionToConsider = Position.Revision;
335+
if (this.VersionHeightPosition <= lastPositionToConsider)
311336
{
312-
case SemanticVersion.Position.Major:
313-
return this.Version.Major;
314-
case SemanticVersion.Position.Minor:
315-
return this.Version.Minor;
316-
case SemanticVersion.Position.Build:
317-
return this.Version.Build;
318-
case SemanticVersion.Position.Revision:
319-
return this.Version.Revision;
320-
default:
321-
throw new ArgumentOutOfRangeException(nameof(position), position, "Must be one of the 4 integer parts.");
337+
lastPositionToConsider = this.VersionHeightPosition.Value - 1;
322338
}
339+
340+
for (Position i = Position.Major; i <= lastPositionToConsider; i++)
341+
{
342+
if (this.ReadVersionPosition(i) != ReadVersionPosition(version, i))
343+
{
344+
return false;
345+
}
346+
}
347+
348+
return true;
323349
}
324350

325351
/// <summary>

src/NerdBank.GitVersioning/VersionOptions.cs

Lines changed: 1 addition & 18 deletions
Original file line numberDiff line numberDiff line change
@@ -433,24 +433,7 @@ internal SemanticVersion.Position? VersionHeightPosition
433433
/// Gets the position in a computed version that the first 16 bits of a git commit ID should appear, if any.
434434
/// </summary>
435435
[JsonIgnore]
436-
internal SemanticVersion.Position? GitCommitIdPosition
437-
{
438-
get
439-
{
440-
// We can only store the git commit ID info after there was a place to put the version height.
441-
// We don't want to store the commit ID (which is effectively a random integer) in the revision slot
442-
// if the version height does not appear, or only appears later (in the -prerelease tag) since that
443-
// would mess up version ordering.
444-
if (this.VersionHeightPosition == SemanticVersion.Position.Build)
445-
{
446-
return SemanticVersion.Position.Revision;
447-
}
448-
else
449-
{
450-
return null;
451-
}
452-
}
453-
}
436+
internal SemanticVersion.Position? GitCommitIdPosition => this.version?.GitCommitIdPosition;
454437

455438
/// <summary>
456439
/// Gets the debugger display for this instance.

0 commit comments

Comments
 (0)