Skip to content
This repository was archived by the owner on Dec 5, 2024. It is now read-only.

Commit 5f963c3

Browse files
StanleyGoldmanStanleyGoldman
authored
Merge pull request #754 from github-for-unity/fixes/version-comparison
Fix version comparisons bugs
2 parents edc0931 + 14c357f commit 5f963c3

File tree

6 files changed

+75
-61
lines changed

6 files changed

+75
-61
lines changed

‎common/SolutionInfo.cs

Lines changed: 6 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -6,8 +6,8 @@
66
using System.Runtime.InteropServices;
77

88
[assembly: AssemblyProduct("GitHub for Unity")]
9-
[assembly: AssemblyVersion(System.AssemblyVersionInformation.Version)]
10-
[assembly: AssemblyFileVersion(System.AssemblyVersionInformation.Version)]
9+
[assembly: AssemblyVersion(System.AssemblyVersionInformation.VersionForAssembly)]
10+
[assembly: AssemblyFileVersion(System.AssemblyVersionInformation.VersionForAssembly)]
1111
[assembly: AssemblyInformationalVersion(System.AssemblyVersionInformation.Version)]
1212
[assembly: ComVisible(false)]
1313
[assembly: AssemblyCompany("GitHub, Inc.")]
@@ -31,6 +31,9 @@
3131
namespace System
3232
{
3333
internal static class AssemblyVersionInformation {
34-
internal const string Version = "0.33.1";
34+
// this is for the AssemblyVersion and AssemblyVersion attributes, which can't handle alphanumerics
35+
internal const string VersionForAssembly = "0.33.1";
36+
// Actual real version
37+
internal const string Version = "0.33.1-beta";
3538
}
3639
}

‎src/GitHub.Api/Application/ApplicationConfiguration.cs

Lines changed: 0 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -5,18 +5,6 @@ namespace GitHub.Unity
55
public static class ApplicationConfiguration
66
{
77
public const int DefaultWebTimeout = 3000;
8-
9-
static ApplicationConfiguration()
10-
{
11-
var executingAssembly = typeof(ApplicationConfiguration).Assembly;
12-
AssemblyName = executingAssembly.GetName();
13-
}
14-
15-
/// <summary>
16-
/// The currently executing assembly.
17-
/// </summary>
18-
public static AssemblyName AssemblyName { get; }
19-
208
public static int WebTimeout { get; set; } = DefaultWebTimeout;
219
}
2210
}

‎src/GitHub.Api/Metrics/UsageTracker.cs

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -37,7 +37,7 @@ public UsageTracker(IMetricsService metricsService, ISettings userSettings,
3737
this.usageLoader = usageLoader;
3838
this.metricsService = metricsService;
3939
this.userId = userId;
40-
this.appVersion = ApplicationConfiguration.AssemblyName.Version.ToString();
40+
this.appVersion = ApplicationInfo.Version;
4141
this.unityVersion = unityVersion;
4242
this.instanceId = instanceId;
4343

‎src/GitHub.Api/Primitives/TheVersion.cs

Lines changed: 52 additions & 41 deletions
Original file line numberDiff line numberDiff line change
@@ -41,12 +41,12 @@ public static TheVersion Parse(string version)
4141
return default(TheVersion).Initialize(version);
4242
}
4343

44-
private TheVersion Initialize(string version)
44+
private TheVersion Initialize(string theVersion)
4545
{
4646
if (initialized)
4747
return this;
4848

49-
this.Version = version?.Trim() ?? String.Empty;
49+
this.Version = theVersion?.Trim() ?? String.Empty;
5050

5151
isAlpha = false;
5252
isBeta = false;
@@ -57,7 +57,7 @@ private TheVersion Initialize(string version)
5757
special = null;
5858
parts = 0;
5959

60-
if (String.IsNullOrEmpty(version))
60+
if (String.IsNullOrEmpty(theVersion))
6161
return this;
6262

6363
intParts = new int[PART_COUNT];
@@ -66,15 +66,16 @@ private TheVersion Initialize(string version)
6666
for (var i = 0; i < PART_COUNT; i++)
6767
stringParts[i] = intParts[i].ToString();
6868

69-
var match = regex.Match(version);
69+
var match = regex.Match(theVersion);
7070
if (!match.Success)
7171
{
72-
LogHelper.Error(new ArgumentException("Invalid version: " + version, "version"));
72+
LogHelper.Error(new ArgumentException("Invalid version: " + theVersion, "theVersion"));
7373
return this;
7474
}
7575

7676
major = int.Parse(match.Groups["major"].Value);
77-
intParts[0] = major;
77+
intParts[parts] = major;
78+
stringParts[parts] = major.ToString();
7879
parts = 1;
7980

8081
var minorMatch = match.Groups["minor"];
@@ -83,39 +84,42 @@ private TheVersion Initialize(string version)
8384

8485
if (minorMatch.Success)
8586
{
86-
parts++;
8787
if (!int.TryParse(minorMatch.Value, out minor))
8888
{
8989
special = minorMatch.Value.TrimEnd();
90-
stringParts[parts - 1] = special;
90+
stringParts[parts] = special ?? "0";
9191
}
9292
else
9393
{
94-
intParts[parts - 1] = minor;
94+
intParts[parts] = minor;
95+
stringParts[parts] = minor.ToString();
96+
parts++;
9597

9698
if (patchMatch.Success)
9799
{
98-
parts++;
99100
if (!int.TryParse(patchMatch.Value, out patch))
100101
{
101102
special = patchMatch.Value.TrimEnd();
102-
stringParts[parts - 1] = special;
103+
stringParts[parts] = special ?? "0";
103104
}
104105
else
105106
{
106-
intParts[parts - 1] = patch;
107+
intParts[parts] = patch;
108+
stringParts[parts] = patch.ToString();
109+
parts++;
107110

108111
if (buildMatch.Success)
109112
{
110-
parts++;
111113
if (!int.TryParse(buildMatch.Value, out build))
112114
{
113115
special = buildMatch.Value.TrimEnd();
114-
stringParts[parts - 1] = special;
116+
stringParts[parts] = special ?? "0";
115117
}
116118
else
117119
{
118-
intParts[parts - 1] = build;
120+
intParts[parts] = build;
121+
stringParts[parts] = build.ToString();
122+
parts++;
119123
}
120124
}
121125
}
@@ -196,19 +200,19 @@ public bool Equals(TheVersion other)
196200
if (String.IsNullOrEmpty(rhs.Version))
197201
return true;
198202

199-
for (var i = 0; i < PART_COUNT; i++)
203+
for (var i = 0; i < lhs.parts && i < rhs.parts; i++)
200204
{
201205
if (lhs.intParts[i] != rhs.intParts[i])
202206
return lhs.intParts[i] > rhs.intParts[i];
203207
}
204208

205209
for (var i = 1; i < PART_COUNT; i++)
206210
{
207-
if (lhs.stringParts[i] != rhs.stringParts[i])
208-
{
209-
return GreaterThan(lhs.stringParts[i], rhs.stringParts[i]);
210-
}
211+
var ret = CompareVersionStrings(lhs.stringParts[i], rhs.stringParts[i]);
212+
if (ret != 0)
213+
return ret > 0;
211214
}
215+
212216
return false;
213217
}
214218

@@ -227,35 +231,42 @@ public bool Equals(TheVersion other)
227231
return lhs < rhs || lhs == rhs;
228232
}
229233

230-
private static bool GreaterThan(string lhs, string rhs)
234+
private static int CompareVersionStrings(string lhs, string rhs)
231235
{
232-
var lhsNonDigitPos = IndexOfFirstNonDigit(lhs);
233-
var rhsNonDigitPos = IndexOfFirstNonDigit(rhs);
236+
int lhsNonDigitPos;
237+
var lhsNumber = GetNumberFromVersionString(lhs, out lhsNonDigitPos);
234238

235-
var lhsNumber = -1;
236-
if (lhsNonDigitPos > -1)
237-
{
238-
int.TryParse(lhs.Substring(0, lhsNonDigitPos), out lhsNumber);
239-
}
240-
else
241-
{
242-
int.TryParse(lhs, out lhsNumber);
243-
}
239+
int rhsNonDigitPos;
240+
var rhsNumber = GetNumberFromVersionString(rhs, out rhsNonDigitPos);
241+
242+
if (lhsNumber != rhsNumber)
243+
return lhsNumber.CompareTo(rhsNumber);
244+
245+
if (lhsNonDigitPos < 0 && rhsNonDigitPos < 0)
246+
return 0;
247+
248+
// versions with alphanumeric characters are always lower than ones without
249+
// i.e. 1.1alpha is lower than 1.1
250+
if (lhsNonDigitPos < 0)
251+
return 1;
252+
if (rhsNonDigitPos < 0)
253+
return -1;
254+
return lhs.Substring(lhsNonDigitPos).CompareTo(rhs.Substring(rhsNonDigitPos));
255+
}
244256

245-
var rhsNumber = -1;
246-
if (rhsNonDigitPos > -1)
257+
private static int GetNumberFromVersionString(string lhs, out int nonDigitPos)
258+
{
259+
nonDigitPos = IndexOfFirstNonDigit(lhs);
260+
var number = -1;
261+
if (nonDigitPos > -1)
247262
{
248-
int.TryParse(rhs.Substring(0, rhsNonDigitPos), out rhsNumber);
263+
int.TryParse(lhs.Substring(0, nonDigitPos), out number);
249264
}
250265
else
251266
{
252-
int.TryParse(rhs, out rhsNumber);
267+
int.TryParse(lhs, out number);
253268
}
254-
255-
if (lhsNumber != rhsNumber)
256-
return lhsNumber > rhsNumber;
257-
258-
return lhs.Substring(lhsNonDigitPos > -1 ? lhsNonDigitPos : 0).CompareTo(rhs.Substring(rhsNonDigitPos > -1 ? rhsNonDigitPos : 0)) > 0;
269+
return number;
259270
}
260271

261272
private static int IndexOfFirstNonDigit(string str)

‎src/UnityExtension/Assets/Editor/UnityTests/VersionTests.cs

Lines changed: 14 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -157,9 +157,21 @@ public void ComparisonWorks()
157157
version2 = TheVersion.Parse("1.2.3alpha1");
158158
Assert.IsTrue(version1 >= version2);
159159

160-
version1 = TheVersion.Parse("0.32.0");
161-
version2 = TheVersion.Parse("0.33.0-beta");
160+
version1 = TheVersion.Parse("0.33.0-beta");
161+
version2 = TheVersion.Parse("0.32.0");
162+
Assert.IsTrue(version1 > version2);
163+
164+
version1 = TheVersion.Parse("0.33.2");
165+
version2 = TheVersion.Parse("0.33.3-beta");
166+
Assert.IsTrue(version1 < version2);
167+
168+
version1 = TheVersion.Parse("0.33.3-alpha");
169+
version2 = TheVersion.Parse("0.33.3-beta");
162170
Assert.IsTrue(version1 < version2);
171+
172+
version1 = TheVersion.Parse("0.33.3");
173+
version2 = TheVersion.Parse("0.33.3-beta");
174+
Assert.IsTrue(version1 > version2);
163175
}
164176

165177
[Test]

‎src/tests/IntegrationTests/Metrics/MetricsTests.cs

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -29,7 +29,7 @@ class MetricsTests : BaseIntegrationTest
2929
public void IncrementMetricsWorks(string measureName)
3030
{
3131
var userId = Guid.NewGuid().ToString();
32-
var appVersion = ApplicationConfiguration.AssemblyName.Version.ToString();
32+
var appVersion = ApplicationInfo.Version;
3333
var unityVersion = "2017.3f1";
3434
var instanceId = Guid.NewGuid().ToString();
3535
var usageLoader = Substitute.For<IUsageLoader>();
@@ -54,7 +54,7 @@ public void LoadingWorks()
5454
{
5555
InitializeEnvironment(TestBasePath, false, false);
5656
var userId = Guid.NewGuid().ToString();
57-
var appVersion = ApplicationConfiguration.AssemblyName.Version.ToString();
57+
var appVersion = ApplicationInfo.Version;
5858
var unityVersion = "2017.3f1";
5959
var instanceId = Guid.NewGuid().ToString();
6060
var usageStore = new UsageStore();

0 commit comments

Comments
 (0)