Skip to content

Commit

Permalink
Only set debug path when MSBuildDebugEngine is set (#6792)
Browse files Browse the repository at this point in the history
In #6639 I thought it would be a good idea to always expose exceptions by always setting msbuild's debug path, fallbacking to the current working directory. But that's a breaking change.

Also, some processes (like VS helpers) do not propagate the user's CWD and thus end up writing under Program Files which fails with permission exceptions.

So only set the debug path to the current working directory when `MSBuildDebugEngine` is set.
  • Loading branch information
cdmihai authored Aug 30, 2021
1 parent 414393f commit d816e47
Show file tree
Hide file tree
Showing 3 changed files with 46 additions and 3 deletions.
38 changes: 38 additions & 0 deletions src/Build.UnitTests/BackEnd/DebugUtils_tests.cs
Original file line number Diff line number Diff line change
@@ -0,0 +1,38 @@
// Copyright (c) Microsoft. All rights reserved.
// Licensed under the MIT license. See LICENSE file in the project root for full license information.

using System;
using System.IO;
using System.Linq;
using Microsoft.Build.Shared;
using Shouldly;
using Xunit;

namespace Microsoft.Build.UnitTests
{
public class DebugUtils_Tests
{
[Fact]
public void DumpExceptionToFileShouldWriteInTempPathByDefault()
{
Directory.GetFiles(Path.GetTempPath(), "MSBuild_*failure.txt").ShouldBeEmpty();

string[] exceptionFiles = null;

try
{
ExceptionHandling.DumpExceptionToFile(new Exception("hello world"));
exceptionFiles = Directory.GetFiles(Path.GetTempPath(), "MSBuild_*failure.txt");
}
finally
{
exceptionFiles.ShouldNotBeNull();
exceptionFiles.ShouldHaveSingleItem();

var exceptionFile = exceptionFiles.First();
File.ReadAllText(exceptionFile).ShouldContain("hello world");
File.Delete(exceptionFile);
}
}
}
}
2 changes: 1 addition & 1 deletion src/Shared/CommunicationsUtilities.cs
Original file line number Diff line number Diff line change
Expand Up @@ -183,7 +183,7 @@ internal static Dictionary<string, string> GetEnvironmentVariables()
// The DebugUtils static constructor can set the MSBUILDDEBUGPATH environment variable to propagate the debug path to out of proc nodes.
// Need to ensure that constructor is called before this method returns in order to capture its env var write.
// Otherwise the env var is not captured and thus gets deleted when RequiestBuilder resets the environment based on the cached results of this method.
ErrorUtilities.VerifyThrowInternalNull(DebugUtils.DebugPath, nameof(DebugUtils.DebugPath));
ErrorUtilities.VerifyThrowInternalNull(DebugUtils.ProcessInfoString, nameof(DebugUtils.DebugPath));
#endif

Dictionary<string, string> table = new Dictionary<string, string>(200, StringComparer.OrdinalIgnoreCase); // Razzle has 150 environment variables
Expand Down
9 changes: 7 additions & 2 deletions src/Shared/Debugging/DebugUtils.cs
Original file line number Diff line number Diff line change
Expand Up @@ -21,11 +21,11 @@ private enum NodeMode
static DebugUtils()
{
string environmentDebugPath = Environment.GetEnvironmentVariable("MSBUILDDEBUGPATH");
var debugDirectory = environmentDebugPath ?? Path.Combine(Directory.GetCurrentDirectory(), "MSBuild_Logs");
var debugDirectory = environmentDebugPath;

if (Traits.Instance.DebugEngine)
{
FileUtilities.EnsureDirectoryExists(debugDirectory);
debugDirectory ??= Path.Combine(Directory.GetCurrentDirectory(), "MSBuild_Logs");

// Out of proc nodes do not know the startup directory so set the environment variable for them.
if (string.IsNullOrWhiteSpace(environmentDebugPath))
Expand All @@ -34,6 +34,11 @@ static DebugUtils()
}
}

if (debugDirectory is not null)
{
FileUtilities.EnsureDirectoryExists(debugDirectory);
}

DebugPath = debugDirectory;
}

Expand Down

0 comments on commit d816e47

Please sign in to comment.