Skip to content

Commit 19a28c5

Browse files
committed
Fixes nbgv get-commits to require version matching
We previously were only looking at version height and (optional) commit ID. We were (amazingly enough) not bothering to look at the major.minor version specified in the version.json file.
1 parent c60a072 commit 19a28c5

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)