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

Use built-in git path interpolation for config settings #439

Merged
merged 5 commits into from
Sep 13, 2021
Merged
Show file tree
Hide file tree
Changes from 1 commit
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
12 changes: 12 additions & 0 deletions src/shared/Microsoft.Git.CredentialManager.Tests/GitTests.cs
Original file line number Diff line number Diff line change
Expand Up @@ -168,6 +168,18 @@ public void Git_GetRemotes_RemoteNoFetchOnlyPull_ReturnsRemote()
AssertRemote(name, null, pushUrl, remotes[0]);
}

[Fact]
public void Git_Version_ReturnsVersion()
{
string gitPath = GetGitPath();
var trace = new NullTrace();

var git = new GitProcess(trace, gitPath, Path.GetTempPath());
GitVersion version = git.Version;

Assert.NotEqual(new GitVersion(), version);
}

#region Test Helpers

private static void AssertRemote(string expectedName, string expectedUrl, GitRemote remote)
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,23 @@
using System;
using Xunit;

namespace Microsoft.Git.CredentialManager.Tests
{
public class GitVersionTests
{
[Theory]
[InlineData(null, 1)]
[InlineData("2", 1)]
[InlineData("3", -1)]
[InlineData("2.33", 0)]
[InlineData("2.32.0", 1)]
[InlineData("2.33.0.windows.0.1", 0)]
[InlineData("2.33.0.2", -1)]
public void GitVersion_CompareTo_2_33_0(string input, int expectedCompare)
{
GitVersion baseline = new GitVersion(2, 33, 0);
GitVersion actual = new GitVersion(input);
Assert.Equal(expectedCompare, baseline.CompareTo(actual));
}
}
}
37 changes: 36 additions & 1 deletion src/shared/Microsoft.Git.CredentialManager/Git.cs
Original file line number Diff line number Diff line change
@@ -1,13 +1,18 @@
using System;
using System.Collections.Generic;
using System.Diagnostics;
using System.Text;
using System.Text.RegularExpressions;
using System.Threading.Tasks;

namespace Microsoft.Git.CredentialManager
{
public interface IGit
{
/// <summary>
/// The version of the Git executable tied to this instance.
/// </summary>
GitVersion Version { get; }

/// <summary>
/// Return the path to the current repository, or null if this instance is not
/// scoped to a Git repository.
Expand Down Expand Up @@ -66,6 +71,36 @@ public GitProcess(ITrace trace, string gitPath, string workingDirectory = null)
_workingDirectory = workingDirectory;
}

private GitVersion _version;
public GitVersion Version
{
get
{
if (_version is null)
{
using (var git = CreateProcess("version"))
{
git.Start();

string data = git.StandardOutput.ReadToEnd();
git.WaitForExit();

Match match = Regex.Match(data, @"^git version (?'value'.*)");
if (match.Success)
{
_version = new GitVersion(match.Groups["value"].Value);
}
else
{
_version = new GitVersion();
}
}
}

return _version;
}
}

public IGitConfiguration GetConfiguration()
{
return new GitProcessConfiguration(_trace, this);
Expand Down
26 changes: 20 additions & 6 deletions src/shared/Microsoft.Git.CredentialManager/GitConfiguration.cs
Original file line number Diff line number Diff line change
Expand Up @@ -110,6 +110,8 @@ public interface IGitConfiguration

public class GitProcessConfiguration : IGitConfiguration
{
private static readonly GitVersion TypeConfigMinVersion = new GitVersion(2, 18, 0);

private readonly ITrace _trace;
private readonly GitProcess _git;

Expand Down Expand Up @@ -448,14 +450,26 @@ private static string GetLevelFilterArg(GitConfigurationLevel level)
}
}

private static string GetCanonicalizeTypeArg(GitConfigurationType type)
private string GetCanonicalizeTypeArg(GitConfigurationType type)
{
return type switch
if (_git.Version >= TypeConfigMinVersion)
{
return type switch
{
GitConfigurationType.Bool => "--type=bool",
GitConfigurationType.Path => "--type=path",
_ => null
};
}
else
{
GitConfigurationType.Bool => "--bool",
GitConfigurationType.Path => "--path",
_ => null
};
return type switch
{
GitConfigurationType.Bool => "--bool",
GitConfigurationType.Path => "--path",
_ => null
};
}
}

public static string QuoteCmdArg(string str)
Expand Down
152 changes: 152 additions & 0 deletions src/shared/Microsoft.Git.CredentialManager/GitVersion.cs
Original file line number Diff line number Diff line change
@@ -0,0 +1,152 @@
using System;
using System.Collections.Generic;
using System.Linq;

namespace Microsoft.Git.CredentialManager
{
public class GitVersion : IComparable, IComparable<GitVersion>
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Implementing IComparable on types can be hard to get right; looks like you've done a great job - nice! 😁

Did you consider also implementing the IEquatable and IEquatable<GitVersion> interfaces?
The implementation is already there in your overridden bool Equals(object) method but adding the interface will mean should we ever put GitVersion in a dictionary or set, that collection can know it can be equated.

This change does not block this PR by the way..

{
private List<int> components;

public GitVersion(string versionString)
{
if (versionString is null)
{
components = new List<int>();
return;
}

string[] splitVersion = versionString.Split('.');
components = new List<int>(splitVersion.Length);

foreach (string part in splitVersion)
{
if (Int32.TryParse(part, out int component))
{
components.Add(component);
}
else
{
// Exit at the first non-integer component
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Makes sense, this would turn 2.33.0.vfs.2.3 into [2, 33, 0].

TBH version comparing is (in my experience) somewhat of an ill-defined problem, and the more simplistic we can make it without breaking our use cases, the better.

As a point of reference, I would like to link to the version comparison in Git for Windows' installer and the code in git update-git-for-windows. As you can see, they both do different things, and neither of them are complete.

I also had a brief hunch that there might be something in .NET, and there is: System.Version. Alas, it seems that the only way to parse a string into an instance of that class is very limited and would choke on any letters (such as vfs).

All this is to say: I think this implementation is nice and small, and good enough for our purposes. Thank you for implementing it!

break;
}
}
}

public GitVersion(params int[] components)
{
this.components = components.ToList();
}

public override string ToString()
{
return string.Join(".", this.components);
}

public int CompareTo(object obj)
{
if (obj is null)
{
return 1;
}

GitVersion other = obj as GitVersion;
if (other == null)
{
throw new ArgumentException("A GitVersion object is required for comparison.", "obj");
}

return CompareTo(other);
}

public int CompareTo(GitVersion other)
{
if (other is null)
{
return 1;
}

// Compare for as many components as the two versions have in common. If a
// component does not exist in a components list, it is assumed to be 0.
int thisCount = this.components.Count, otherCount = other.components.Count;
for (int i = 0; i < Math.Max(thisCount, otherCount); i++)
{
int thisComponent = i < thisCount ? this.components[i] : 0;
int otherComponent = i < otherCount ? other.components[i] : 0;
if (thisComponent != otherComponent)
{
return thisComponent.CompareTo(otherComponent);
}
}

// No discrepencies found in versions
return 0;
}

public static int Compare(GitVersion left, GitVersion right)
{
if (object.ReferenceEquals(left, right))
{
return 0;
}

if (left is null)
{
return -1;
}

return left.CompareTo(right);
}

public override bool Equals(object obj)
{
GitVersion other = obj as GitVersion;
if (other is null)
{
return false;
}

return this.CompareTo(other) == 0;
}

public override int GetHashCode()
{
return ToString().GetHashCode();
}

public static bool operator ==(GitVersion left, GitVersion right)
{
if (left is null)
{
return right is null;
}

return left.Equals(right);
}

public static bool operator !=(GitVersion left, GitVersion right)
{
return !(left == right);
}

public static bool operator <(GitVersion left, GitVersion right)
{
return Compare(left, right) < 0;
}

public static bool operator >(GitVersion left, GitVersion right)
{
return Compare(left, right) > 0;
}

public static bool operator <=(GitVersion left, GitVersion right)
{
return Compare(left, right) <= 0;
}

public static bool operator >=(GitVersion left, GitVersion right)
{
return Compare(left, right) >= 0;
}
}
}
1 change: 1 addition & 0 deletions src/shared/TestInfrastructure/Objects/TestGit.cs
Original file line number Diff line number Diff line change
Expand Up @@ -22,6 +22,7 @@ public TestGit(bool insideRepo = true)
}

#region IGit
GitVersion IGit.Version => new GitVersion(2, 33, 0);

string IGit.GetCurrentRepository() => CurrentRepository;

Expand Down