Skip to content

Commit ff1043f

Browse files
mjcheethammjcheetham
authored
Merge pull request #203 from mjcheetham/fixtypowin
Make the system installer always append GCM Core entries, not outright set/replace existing values (and fix a typo)
2 parents 9666c94 + ba81146 commit ff1043f

File tree

10 files changed

+333
-171
lines changed

10 files changed

+333
-171
lines changed

‎src/shared/Microsoft.AzureRepos.Tests/AzureReposHostProviderTests.cs

Lines changed: 46 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -5,6 +5,7 @@
55
using System.Threading.Tasks;
66
using Microsoft.Git.CredentialManager;
77
using Microsoft.Git.CredentialManager.Authentication;
8+
using Microsoft.Git.CredentialManager.Tests;
89
using Microsoft.Git.CredentialManager.Tests.Objects;
910
using Moq;
1011
using Xunit;
@@ -14,6 +15,8 @@ namespace Microsoft.AzureRepos.Tests
1415
{
1516
public class AzureReposHostProviderTests
1617
{
18+
private static readonly string HelperKey =
19+
$"{Constants.GitConfiguration.Credential.SectionName}.{Constants.GitConfiguration.Credential.Helper}";
1720
private static readonly string AzDevUseHttpPathKey =
1821
$"{Constants.GitConfiguration.Credential.SectionName}.https://dev.azure.com.{Constants.GitConfiguration.Credential.UseHttpPath}";
1922

@@ -222,7 +225,6 @@ public async Task AzureReposHostProvider_ConfigureAsync_UseHttpPathUnset_SetsUse
222225
Assert.Equal("true", actualValues[0]);
223226
}
224227

225-
226228
[Fact]
227229
public async Task AzureReposHostProvider_UnconfigureAsync_UseHttpPathSet_RemovesEntry()
228230
{
@@ -235,5 +237,48 @@ public async Task AzureReposHostProvider_UnconfigureAsync_UseHttpPathSet_Removes
235237

236238
Assert.Empty(context.Git.GlobalConfiguration.Dictionary);
237239
}
240+
241+
[PlatformFact(Platforms.Windows)]
242+
public async Task AzureReposHostProvider_UnconfigureAsync_System_Windows_UseHttpPathSetAndManagerCoreHelper_DoesNotRemoveEntry()
243+
{
244+
var context = new TestCommandContext();
245+
var provider = new AzureReposHostProvider(context);
246+
247+
context.Git.SystemConfiguration.Dictionary[HelperKey] = new List<string> {"manager-core"};
248+
context.Git.SystemConfiguration.Dictionary[AzDevUseHttpPathKey] = new List<string> {"true"};
249+
250+
await provider.UnconfigureAsync(ConfigurationTarget.System);
251+
252+
Assert.True(context.Git.SystemConfiguration.Dictionary.TryGetValue(AzDevUseHttpPathKey, out IList<string> actualValues));
253+
Assert.Single(actualValues);
254+
Assert.Equal("true", actualValues[0]);
255+
}
256+
257+
[PlatformFact(Platforms.Windows)]
258+
public async Task AzureReposHostProvider_UnconfigureAsync_System_Windows_UseHttpPathSetNoManagerCoreHelper_RemovesEntry()
259+
{
260+
var context = new TestCommandContext();
261+
var provider = new AzureReposHostProvider(context);
262+
263+
context.Git.SystemConfiguration.Dictionary[AzDevUseHttpPathKey] = new List<string> {"true"};
264+
265+
await provider.UnconfigureAsync(ConfigurationTarget.System);
266+
267+
Assert.Empty(context.Git.SystemConfiguration.Dictionary);
268+
}
269+
270+
[PlatformFact(Platforms.Windows)]
271+
public async Task AzureReposHostProvider_UnconfigureAsync_User_Windows_UseHttpPathSetAndManagerCoreHelper_RemovesEntry()
272+
{
273+
var context = new TestCommandContext();
274+
var provider = new AzureReposHostProvider(context);
275+
276+
context.Git.GlobalConfiguration.Dictionary[HelperKey] = new List<string> {"manager-core"};
277+
context.Git.GlobalConfiguration.Dictionary[AzDevUseHttpPathKey] = new List<string> {"true"};
278+
279+
await provider.UnconfigureAsync(ConfigurationTarget.User);
280+
281+
Assert.False(context.Git.GlobalConfiguration.Dictionary.TryGetValue(AzDevUseHttpPathKey, out _));
282+
}
238283
}
239284
}

‎src/shared/Microsoft.AzureRepos/AzureReposHostProvider.cs

Lines changed: 12 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -2,6 +2,7 @@
22
// Licensed under the MIT license.
33
using System;
44
using System.Collections.Generic;
5+
using System.Linq;
56
using System.Threading.Tasks;
67
using Microsoft.Git.CredentialManager;
78
using Microsoft.Git.CredentialManager.Authentication;
@@ -260,21 +261,22 @@ public Task ConfigureAsync(ConfigurationTarget target)
260261

261262
IGitConfiguration targetConfig = _context.Git.GetConfiguration(configurationLevel);
262263

263-
if (targetConfig.TryGetValue(useHttpPathKey, out string currentValue) && currentValue.IsTruthy())
264+
if (targetConfig.TryGet(useHttpPathKey, out string currentValue) && currentValue.IsTruthy())
264265
{
265266
_context.Trace.WriteLine("Git configuration 'credential.useHttpPath' is already set to 'true' for https://dev.azure.com.");
266267
}
267268
else
268269
{
269270
_context.Trace.WriteLine("Setting Git configuration 'credential.useHttpPath' to 'true' for https://dev.azure.com...");
270-
targetConfig.SetValue(useHttpPathKey, "true");
271+
targetConfig.Set(useHttpPathKey, "true");
271272
}
272273

273274
return Task.CompletedTask;
274275
}
275276

276277
public Task UnconfigureAsync(ConfigurationTarget target)
277278
{
279+
string helperKey = $"{Constants.GitConfiguration.Credential.SectionName}.{Constants.GitConfiguration.Credential.Helper}";
278280
string useHttpPathKey = $"{KnownGitCfg.Credential.SectionName}.https://dev.azure.com.{KnownGitCfg.Credential.UseHttpPath}";
279281

280282
_context.Trace.WriteLine("Clearing Git configuration 'credential.useHttpPath' for https://dev.azure.com...");
@@ -284,7 +286,14 @@ public Task UnconfigureAsync(ConfigurationTarget target)
284286
: GitConfigurationLevel.Global;
285287

286288
IGitConfiguration targetConfig = _context.Git.GetConfiguration(configurationLevel);
287-
targetConfig.Unset(useHttpPathKey);
289+
290+
// On Windows, if there is a "manager-core" entry remaining in the system config then we must not clear
291+
// the useHttpPath option otherwise this would break the bundled version of GCM Core in Git for Windows.
292+
if (!PlatformUtils.IsWindows() || target != ConfigurationTarget.System ||
293+
targetConfig.GetAll(helperKey).All(x => !string.Equals(x, "manager-core")))
294+
{
295+
targetConfig.Unset(useHttpPathKey);
296+
}
288297

289298
return Task.CompletedTask;
290299
}

‎src/shared/Microsoft.Git.CredentialManager.Tests/ApplicationTests.cs

Lines changed: 28 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -152,6 +152,34 @@ public async Task Application_ConfigureAsync_EmptyAndGcmWithOthersBeforeAndAfter
152152
Assert.Equal(afterHelper, actualValues[3]);
153153
}
154154

155+
[Fact]
156+
public async Task Application_ConfigureAsync_EmptyAndGcmWithEmptyAfter_RemovesExistingGcmAndAddsEmptyAndGcm()
157+
{
158+
const string emptyHelper = "";
159+
const string afterHelper = "foo";
160+
const string executablePath = "/usr/local/share/gcm-core/git-credential-manager-core";
161+
string key = $"{Constants.GitConfiguration.Credential.SectionName}.{Constants.GitConfiguration.Credential.Helper}";
162+
163+
var context = new TestCommandContext();
164+
IConfigurableComponent application = new Application(context, executablePath);
165+
166+
context.Git.GlobalConfiguration.Dictionary[key] = new List<string>
167+
{
168+
emptyHelper, executablePath, emptyHelper, afterHelper
169+
};
170+
171+
await application.ConfigureAsync(ConfigurationTarget.User);
172+
173+
Assert.Single(context.Git.GlobalConfiguration.Dictionary);
174+
Assert.True(context.Git.GlobalConfiguration.Dictionary.TryGetValue(key, out var actualValues));
175+
Assert.Equal(5, actualValues.Count);
176+
Assert.Equal(emptyHelper, actualValues[0]);
177+
Assert.Equal(emptyHelper, actualValues[1]);
178+
Assert.Equal(afterHelper, actualValues[2]);
179+
Assert.Equal(emptyHelper, actualValues[3]);
180+
Assert.Equal(executablePath, actualValues[4]);
181+
}
182+
155183
[Fact]
156184
public async Task Application_UnconfigureAsync_NoHelpers_DoesNothing()
157185
{

‎src/shared/Microsoft.Git.CredentialManager.Tests/GitConfigurationTests.cs

Lines changed: 25 additions & 25 deletions
Original file line numberDiff line numberDiff line change
@@ -130,7 +130,7 @@ bool cb(string name, string value)
130130
}
131131

132132
[Fact]
133-
public void GitConfiguration_TryGetValue_Name_Exists_ReturnsTrueOutString()
133+
public void GitConfiguration_TryGet_Name_Exists_ReturnsTrueOutString()
134134
{
135135
string repoPath = CreateRepository(out string workDirPath);
136136
Git(repoPath, workDirPath, "config --local user.name john.doe").AssertSuccess();
@@ -140,14 +140,14 @@ public void GitConfiguration_TryGetValue_Name_Exists_ReturnsTrueOutString()
140140
var git = new GitProcess(trace, gitPath, repoPath);
141141
IGitConfiguration config = git.GetConfiguration();
142142

143-
bool result = config.TryGetValue("user.name", out string value);
143+
bool result = config.TryGet("user.name", out string value);
144144
Assert.True(result);
145145
Assert.NotNull(value);
146146
Assert.Equal("john.doe", value);
147147
}
148148

149149
[Fact]
150-
public void GitConfiguration_TryGetValue_Name_DoesNotExists_ReturnsFalse()
150+
public void GitConfiguration_TryGet_Name_DoesNotExists_ReturnsFalse()
151151
{
152152
string repoPath = CreateRepository();
153153

@@ -157,13 +157,13 @@ public void GitConfiguration_TryGetValue_Name_DoesNotExists_ReturnsFalse()
157157
IGitConfiguration config = git.GetConfiguration();
158158

159159
string randomName = $"{Guid.NewGuid():N}.{Guid.NewGuid():N}";
160-
bool result = config.TryGetValue(randomName, out string value);
160+
bool result = config.TryGet(randomName, out string value);
161161
Assert.False(result);
162162
Assert.Null(value);
163163
}
164164

165165
[Fact]
166-
public void GitConfiguration_TryGetValue_SectionProperty_Exists_ReturnsTrueOutString()
166+
public void GitConfiguration_TryGet_SectionProperty_Exists_ReturnsTrueOutString()
167167
{
168168
string repoPath = CreateRepository(out string workDirPath);
169169
Git(repoPath, workDirPath, "config --local user.name john.doe").AssertSuccess();
@@ -173,14 +173,14 @@ public void GitConfiguration_TryGetValue_SectionProperty_Exists_ReturnsTrueOutSt
173173
var git = new GitProcess(trace, gitPath, repoPath);
174174
IGitConfiguration config = git.GetConfiguration();
175175

176-
bool result = config.TryGetValue("user", "name", out string value);
176+
bool result = config.TryGet("user", "name", out string value);
177177
Assert.True(result);
178178
Assert.NotNull(value);
179179
Assert.Equal("john.doe", value);
180180
}
181181

182182
[Fact]
183-
public void GitConfiguration_TryGetValue_SectionProperty_DoesNotExists_ReturnsFalse()
183+
public void GitConfiguration_TryGet_SectionProperty_DoesNotExists_ReturnsFalse()
184184
{
185185
string repoPath = CreateRepository();
186186

@@ -191,13 +191,13 @@ public void GitConfiguration_TryGetValue_SectionProperty_DoesNotExists_ReturnsFa
191191

192192
string randomSection = Guid.NewGuid().ToString("N");
193193
string randomProperty = Guid.NewGuid().ToString("N");
194-
bool result = config.TryGetValue(randomSection, randomProperty, out string value);
194+
bool result = config.TryGet(randomSection, randomProperty, out string value);
195195
Assert.False(result);
196196
Assert.Null(value);
197197
}
198198

199199
[Fact]
200-
public void GitConfiguration_TryGetValue_SectionScopeProperty_Exists_ReturnsTrueOutString()
200+
public void GitConfiguration_TryGet_SectionScopeProperty_Exists_ReturnsTrueOutString()
201201
{
202202
string repoPath = CreateRepository(out string workDirPath);
203203
Git(repoPath, workDirPath, "config --local user.example.com.name john.doe").AssertSuccess();
@@ -207,14 +207,14 @@ public void GitConfiguration_TryGetValue_SectionScopeProperty_Exists_ReturnsTrue
207207
var git = new GitProcess(trace, gitPath, repoPath);
208208
IGitConfiguration config = git.GetConfiguration();
209209

210-
bool result = config.TryGetValue("user", "example.com", "name", out string value);
210+
bool result = config.TryGet("user", "example.com", "name", out string value);
211211
Assert.True(result);
212212
Assert.NotNull(value);
213213
Assert.Equal("john.doe", value);
214214
}
215215

216216
[Fact]
217-
public void GitConfiguration_TryGetValue_SectionScopeProperty_NullScope_ReturnsTrueOutUnscopedString()
217+
public void GitConfiguration_TryGet_SectionScopeProperty_NullScope_ReturnsTrueOutUnscopedString()
218218
{
219219
string repoPath = CreateRepository(out string workDirPath);
220220
Git(repoPath, workDirPath, "config --local user.name john.doe").AssertSuccess();
@@ -224,14 +224,14 @@ public void GitConfiguration_TryGetValue_SectionScopeProperty_NullScope_ReturnsT
224224
var git = new GitProcess(trace, gitPath, repoPath);
225225
IGitConfiguration config = git.GetConfiguration();
226226

227-
bool result = config.TryGetValue("user", null, "name", out string value);
227+
bool result = config.TryGet("user", null, "name", out string value);
228228
Assert.True(result);
229229
Assert.NotNull(value);
230230
Assert.Equal("john.doe", value);
231231
}
232232

233233
[Fact]
234-
public void GitConfiguration_TryGetValue_SectionScopeProperty_DoesNotExists_ReturnsFalse()
234+
public void GitConfiguration_TryGet_SectionScopeProperty_DoesNotExists_ReturnsFalse()
235235
{
236236
string repoPath = CreateRepository();
237237

@@ -243,7 +243,7 @@ public void GitConfiguration_TryGetValue_SectionScopeProperty_DoesNotExists_Retu
243243
string randomSection = Guid.NewGuid().ToString("N");
244244
string randomScope = Guid.NewGuid().ToString("N");
245245
string randomProperty = Guid.NewGuid().ToString("N");
246-
bool result = config.TryGetValue(randomSection, randomScope, randomProperty, out string value);
246+
bool result = config.TryGet(randomSection, randomScope, randomProperty, out string value);
247247
Assert.False(result);
248248
Assert.Null(value);
249249
}
@@ -259,7 +259,7 @@ public void GitConfiguration_GetString_Name_Exists_ReturnsString()
259259
var git = new GitProcess(trace, gitPath, repoPath);
260260
IGitConfiguration config = git.GetConfiguration();
261261

262-
string value = config.GetValue("user.name");
262+
string value = config.Get("user.name");
263263
Assert.NotNull(value);
264264
Assert.Equal("john.doe", value);
265265
}
@@ -275,7 +275,7 @@ public void GitConfiguration_GetString_Name_DoesNotExists_ThrowsException()
275275
IGitConfiguration config = git.GetConfiguration();
276276

277277
string randomName = $"{Guid.NewGuid():N}.{Guid.NewGuid():N}";
278-
Assert.Throws<KeyNotFoundException>(() => config.GetValue(randomName));
278+
Assert.Throws<KeyNotFoundException>(() => config.Get(randomName));
279279
}
280280

281281
[Fact]
@@ -289,7 +289,7 @@ public void GitConfiguration_GetString_SectionProperty_Exists_ReturnsString()
289289
var git = new GitProcess(trace, gitPath, repoPath);
290290
IGitConfiguration config = git.GetConfiguration();
291291

292-
string value = config.GetValue("user", "name");
292+
string value = config.Get("user", "name");
293293
Assert.NotNull(value);
294294
Assert.Equal("john.doe", value);
295295
}
@@ -306,7 +306,7 @@ public void GitConfiguration_GetString_SectionProperty_DoesNotExists_ThrowsExcep
306306

307307
string randomSection = Guid.NewGuid().ToString("N");
308308
string randomProperty = Guid.NewGuid().ToString("N");
309-
Assert.Throws<KeyNotFoundException>(() => config.GetValue(randomSection, randomProperty));
309+
Assert.Throws<KeyNotFoundException>(() => config.Get(randomSection, randomProperty));
310310
}
311311

312312
[Fact]
@@ -320,7 +320,7 @@ public void GitConfiguration_GetString_SectionScopeProperty_Exists_ReturnsString
320320
var git = new GitProcess(trace, gitPath, repoPath);
321321
IGitConfiguration config = git.GetConfiguration();
322322

323-
string value = config.GetValue("user", "example.com", "name");
323+
string value = config.Get("user", "example.com", "name");
324324
Assert.NotNull(value);
325325
Assert.Equal("john.doe", value);
326326
}
@@ -336,7 +336,7 @@ public void GitConfiguration_GetString_SectionScopeProperty_NullScope_ReturnsUns
336336
var git = new GitProcess(trace, gitPath, repoPath);
337337
IGitConfiguration config = git.GetConfiguration();
338338

339-
string value = config.GetValue("user", null, "name");
339+
string value = config.Get("user", null, "name");
340340
Assert.NotNull(value);
341341
Assert.Equal("john.doe", value);
342342
}
@@ -354,11 +354,11 @@ public void GitConfiguration_GetString_SectionScopeProperty_DoesNotExists_Throws
354354
string randomSection = Guid.NewGuid().ToString("N");
355355
string randomScope = Guid.NewGuid().ToString("N");
356356
string randomProperty = Guid.NewGuid().ToString("N");
357-
Assert.Throws<KeyNotFoundException>(() => config.GetValue(randomSection, randomScope, randomProperty));
357+
Assert.Throws<KeyNotFoundException>(() => config.Get(randomSection, randomScope, randomProperty));
358358
}
359359

360360
[Fact]
361-
public void GitConfiguration_SetValue_Local_SetsLocalConfig()
361+
public void GitConfiguration_Set_Local_SetsLocalConfig()
362362
{
363363
string repoPath = CreateRepository(out string workDirPath);
364364

@@ -367,15 +367,15 @@ public void GitConfiguration_SetValue_Local_SetsLocalConfig()
367367
var git = new GitProcess(trace, gitPath, repoPath);
368368
IGitConfiguration config = git.GetConfiguration(GitConfigurationLevel.Local);
369369

370-
config.SetValue("core.foobar", "foo123");
370+
config.Set("core.foobar", "foo123");
371371

372372
GitResult localResult = Git(repoPath, workDirPath, "config --local core.foobar");
373373

374374
Assert.Equal("foo123", localResult.StandardOutput.Trim());
375375
}
376376

377377
[Fact]
378-
public void GitConfiguration_SetValue_All_ThrowsException()
378+
public void GitConfiguration_Set_All_ThrowsException()
379379
{
380380
string repoPath = CreateRepository(out _);
381381

@@ -384,7 +384,7 @@ public void GitConfiguration_SetValue_All_ThrowsException()
384384
var git = new GitProcess(trace, gitPath, repoPath);
385385
IGitConfiguration config = git.GetConfiguration(GitConfigurationLevel.All);
386386

387-
Assert.Throws<InvalidOperationException>(() => config.SetValue("core.foobar", "test123"));
387+
Assert.Throws<InvalidOperationException>(() => config.Set("core.foobar", "test123"));
388388
}
389389

390390
[Fact]

0 commit comments

Comments
 (0)