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.

Sign up for GitHub

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

Emit warning for projects detected in multiple drives #549

Merged
merged 2 commits into from
Nov 16, 2023
Merged

Emit warning for projects detected in multiple drives #549

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
f28434f
Emit warning for projects detected in multiple drives
mruxmohan4 Nov 14, 2023
3085741
PR feedback
mruxmohan4 Nov 15, 2023
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
65 changes: 50 additions & 15 deletions src/Microsoft.VisualStudio.SlnGen.UnitTests/SlnFileTests.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -10,6 +10,7 @@
using System.Collections.Generic;
using System.IO;
using System.Linq;
using System.Text;
using Xunit;

namespace Microsoft.VisualStudio.SlnGen.UnitTests
Expand Down Expand Up @@ -106,7 +107,7 @@ public void CustomConfigurationAndPlatforms()

string solutionFilePath = GetTempFileName();

slnFile.Save(solutionFilePath, useFolders: false);
slnFile.Save(solutionFilePath, useFolders: false, new TestLogger());

SolutionFile solutionFile = SolutionFile.Parse(solutionFilePath);

Expand Down Expand Up @@ -193,7 +194,7 @@ public void CustomConfigurationAndPlatformsWithAlwaysBuildDisabled()

string solutionFilePath = GetTempFileName();

slnFile.Save(solutionFilePath, useFolders: false, alwaysBuild: false);
slnFile.Save(solutionFilePath, useFolders: false, new TestLogger(), alwaysBuild: false);

SolutionFile solutionFile = SolutionFile.Parse(solutionFilePath);

Expand Down Expand Up @@ -232,7 +233,7 @@ public void CustomConfigurationAndPlatforms_IgnoresInvalidValues()

string solutionFilePath = GetTempFileName();

slnFile.Save(solutionFilePath, useFolders: false);
slnFile.Save(solutionFilePath, useFolders: false, new TestLogger());

SolutionFile solutionFile = SolutionFile.Parse(solutionFilePath);

Expand Down Expand Up @@ -314,7 +315,7 @@ public void CustomConfigurationAndPlatforms_MapsAnyCPU()

string solutionFilePath = GetTempFileName();

slnFile.Save(solutionFilePath, useFolders: false);
slnFile.Save(solutionFilePath, useFolders: false, new TestLogger());

SolutionFile solutionFile = SolutionFile.Parse(solutionFilePath);

Expand Down Expand Up @@ -354,7 +355,7 @@ public void ExistingSolutionIsReused()

slnFile.AddProjects(new[] { project });

slnFile.Save(path, useFolders: false);
slnFile.Save(path, useFolders: false, new TestLogger());

SlnFile.TryParseExistingSolution(path, out Guid solutionGuid, out _).ShouldBeTrue();

Expand Down Expand Up @@ -554,7 +555,7 @@ public void ProjectConfigurationPlatformOrderingSameAsProjects()

string path = Path.GetTempFileName();

slnFile.Save(path, useFolders: false);
slnFile.Save(path, useFolders: false, new TestLogger());

string directoryName = new DirectoryInfo(TestRootPath).Name;

Expand Down Expand Up @@ -636,7 +637,7 @@ public void ProjectSolutionFolders()

slnFile.AddProjects(projects, new Dictionary<string, Guid>(), projects[1].FullPath);
slnFile.AddSolutionItems(solutionItems);
slnFile.Save(solutionFilePath, useFolders: false);
slnFile.Save(solutionFilePath, useFolders: false, new TestLogger());

SolutionFile s = SolutionFile.Parse(solutionFilePath);

Expand Down Expand Up @@ -672,7 +673,7 @@ public void SaveToCustomLocationCreatesDirectory()

SlnFile slnFile = new SlnFile();

slnFile.Save(fullPath, useFolders: false);
slnFile.Save(fullPath, useFolders: false, new TestLogger());

File.Exists(fullPath).ShouldBeTrue();
}
Expand Down Expand Up @@ -891,7 +892,7 @@ public void WithFoldersDoNotIgnoreMainProject()

slnFile.AddProjects(projects, new Dictionary<string, Guid>(), projects[1].FullPath);
slnFile.AddSolutionItems(solutionItems);
slnFile.Save(solutionFilePath, true);
slnFile.Save(solutionFilePath, true, new TestLogger());

SolutionFile solutionFile = SolutionFile.Parse(solutionFilePath);

Expand Down Expand Up @@ -949,7 +950,7 @@ public void WithFoldersIgnoreMainProject()

slnFile.AddProjects(projects, new Dictionary<string, Guid>());
slnFile.AddSolutionItems(solutionItems);
slnFile.Save(solutionFilePath, true);
slnFile.Save(solutionFilePath, true, new TestLogger());

SolutionFile solutionFile = SolutionFile.Parse(solutionFilePath);

Expand Down Expand Up @@ -1016,7 +1017,7 @@ public void WithFoldersDoesNotCreateRootFolder(bool ignoreMainProject, bool coll

slnFile.AddProjects(projects, new Dictionary<string, Guid>(), ignoreMainProject ? null : projects[1].FullPath);
slnFile.AddSolutionItems(solutionItems);
slnFile.Save(solutionFilePath, useFolders: true, collapseFolders: collapseFolders);
slnFile.Save(solutionFilePath, useFolders: true, new TestLogger(), collapseFolders: collapseFolders);

SolutionFile solutionFile = SolutionFile.Parse(solutionFilePath);

Expand Down Expand Up @@ -1072,7 +1073,7 @@ public void VisualStudioVersionIsWritten()
SolutionGuid = new Guid("{6370DE27-36B7-44AE-B47A-1ECF4A6D740A}"),
};

slnFile.Save(solutionFilePath, useFolders: false);
slnFile.Save(solutionFilePath, useFolders: false, new TestLogger());

File.ReadAllText(solutionFilePath).ShouldBe(
@"Microsoft Visual Studio Solution File, Format Version 12.00
Expand Down Expand Up @@ -1109,7 +1110,7 @@ public void Save_WithSolutionItemsAddedToSpecificFolder_SolutionItemsExistInSpec
slnFile.AddSolutionItems("docs", new[] { Path.Combine(this.TestRootPath, "README.md") });

// Act
slnFile.Save(solutionFilePath, useFolders: false);
slnFile.Save(solutionFilePath, useFolders: false, new TestLogger());

// Assert
File.ReadAllText(solutionFilePath).ShouldBe(
Expand All @@ -1135,6 +1136,40 @@ public void Save_WithSolutionItemsAddedToSpecificFolder_SolutionItemsExistInSpec
StringCompareShould.IgnoreLineEndings);
}

[Fact]
public void EmitWarningForProjectsOnMultipleDrives()
{
bool isWindowsPlatform = Utility.RunningOnWindows;
SlnProject projectA = new ()
{
Name = "ProjectA",
FullPath = isWindowsPlatform ? @"A:\ProjectA\ProjectA.vcxitems" : "/dev/ProjectA/ProjectA.vcxitems",
ProjectGuid = new Guid("C95D800E-F016-4167-8E1B-1D3FF94CE2E2"),
ProjectTypeGuid = new Guid("88152E7E-47E3-45C8-B5D3-DDB15B2F0435"),
};

SlnProject projectB = new ()
{
Name = "ProjectB",
FullPath = isWindowsPlatform ? @"B:\ProjectB\ProjectB.vcxitems" : "/mnt/ProjectB/ProjectB.vcxitems",
ProjectGuid = new Guid("EAD108BE-AC70-41E6-A8C3-450C545FDC0E"),
ProjectTypeGuid = new Guid("F38341C3-343F-421A-AE68-94CD9ADCD32F"),
};

TestLogger logger = new ();
SlnFile slnFile = new ();
SlnProject[] projects = new[] { projectA, projectB };
string solutionFilePath = @$"X:\{Path.GetRandomFileName()}";
StringBuilderTextWriter writer = new (new StringBuilder(), new List<string>());

slnFile.AddProjects(projects);
slnFile.Save(solutionFilePath, writer, useFolders: true, logger);

logger.Errors.Count.ShouldBe(0);
logger.Warnings.Count.ShouldBe(1);
logger.Warnings.FirstOrDefault().Message.ShouldContain("Detected folder on a different drive from the root solution path");
}

[Theory]
[InlineData(true)]
[InlineData(false)]
Expand All @@ -1155,7 +1190,7 @@ public void SlnProject_IsBuildable_ReflectedAsProjectConfigurationInSolutionIncl
};

slnFile.AddProjects(new[] { slnProject });
slnFile.Save(solutionFilePath, useFolders: false);
slnFile.Save(solutionFilePath, useFolders: false, new TestLogger());

ValidateProjectInSolution(
(slnProject, projectInSolution) =>
Expand All @@ -1178,7 +1213,7 @@ private void ValidateProjectInSolution(Action<SlnProject, ProjectInSolution> cus
SlnFile slnFile = new SlnFile();

slnFile.AddProjects(projects);
slnFile.Save(solutionFilePath, useFolders);
slnFile.Save(solutionFilePath, useFolders, new TestLogger());

SolutionFile solutionFile = SolutionFile.Parse(solutionFilePath);

Expand Down
42 changes: 42 additions & 0 deletions src/Microsoft.VisualStudio.SlnGen.UnitTests/StringBuilderTextWriter.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,42 @@
// Copyright (c) Microsoft Corporation.
//
// Licensed under the MIT license.

using System.Collections.Generic;
using System.IO;
using System.Text;

namespace Microsoft.VisualStudio.SlnGen.UnitTests
{
/// <summary>
/// Extends <see cref="TextWriter" /> for unit tests to capture writing text.
/// </summary>
public class StringBuilderTextWriter : TextWriter
{
private readonly List<string> _lines;
private readonly StringBuilder _lineStringBuilder = new StringBuilder();
private readonly StringBuilder _stringBuilder;

public StringBuilderTextWriter(StringBuilder stringBuilder, List<string> lines)
{
_stringBuilder = stringBuilder;
_lines = lines;
}

public override Encoding Encoding => Encoding.Unicode;

public override void Write(char value)
{
_lineStringBuilder.Append(value);

_stringBuilder.Append(value);

if (value == '\n')
{
_lines.Add(_lineStringBuilder.ToString());

_lineStringBuilder.Clear();
}
}
}
}
29 changes: 0 additions & 29 deletions src/Microsoft.VisualStudio.SlnGen.UnitTests/TestConsole.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -63,34 +63,5 @@ public event ConsoleCancelEventHandler CancelKeyPress
public void ResetColor()
{
}

private class StringBuilderTextWriter : TextWriter
{
private readonly List<string> _lines;
private readonly StringBuilder _lineStringBuilder = new StringBuilder();
private readonly StringBuilder _stringBuilder;

public StringBuilderTextWriter(StringBuilder stringBuilder, List<string> lines)
{
_stringBuilder = stringBuilder;
_lines = lines;
}

public override Encoding Encoding => Encoding.Unicode;

public override void Write(char value)
{
_lineStringBuilder.Append(value);

_stringBuilder.Append(value);

if (value == '\n')
{
_lines.Add(_lineStringBuilder.ToString());

_lineStringBuilder.Clear();
}
}
}
}
}
31 changes: 25 additions & 6 deletions src/Microsoft.VisualStudio.SlnGen/SlnFile.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -213,7 +213,7 @@ public static (string solutionFileFullPath, int customProjectTypeGuidCount, int

if (!logger.HasLoggedErrors)
{
solution.Save(solutionFileFullPath, arguments.EnableFolders(), arguments.EnableCollapseFolders(), arguments.EnableAlwaysBuild());
solution.Save(solutionFileFullPath, arguments.EnableFolders(), logger, arguments.EnableCollapseFolders(), arguments.EnableAlwaysBuild());
}

return (solutionFileFullPath, customProjectTypeGuids.Count, solutionItems.Count, solution.SolutionGuid);
Expand Down Expand Up @@ -362,9 +362,10 @@ public void AddSolutionItems(string folderPath, IEnumerable<string> items)
/// </summary>
/// <param name="path">The full path to the file to write to.</param>
/// <param name="useFolders">Specifies if folders should be created.</param>
/// <param name="logger">A <see cref="ISlnGenLogger" /> to use for logging.</param>
/// <param name="collapseFolders">An optional value indicating whether or not folders containing a single item should be collapsed into their parent folder.</param>
/// <param name="alwaysBuild">An optional value indicating whether or not to always include the project in the build even if it has no matching configuration.</param>
public void Save(string path, bool useFolders, bool collapseFolders = false, bool alwaysBuild = true)
public void Save(string path, bool useFolders, ISlnGenLogger logger, bool collapseFolders = false, bool alwaysBuild = true)
{
string directoryName = Path.GetDirectoryName(path);

Expand All @@ -377,7 +378,7 @@ public void Save(string path, bool useFolders, bool collapseFolders = false, boo

using StreamWriter writer = new StreamWriter(fileStream, Encoding.UTF8);

Save(path, writer, useFolders, collapseFolders, alwaysBuild);
Save(path, writer, useFolders, logger, collapseFolders, alwaysBuild);
}

/// <summary>
Expand All @@ -386,9 +387,10 @@ public void Save(string path, bool useFolders, bool collapseFolders = false, boo
/// <param name="rootPath">A root path for the solution to make other paths relative to.</param>
/// <param name="writer">The <see cref="TextWriter" /> to save the solution file to.</param>
/// <param name="useFolders">Specifies if folders should be created.</param>
/// <param name="logger">A <see cref="ISlnGenLogger" /> to use for logging.</param>
/// <param name="collapseFolders">An optional value indicating whether or not folders containing a single item should be collapsed into their parent folder.</param>
/// <param name="alwaysBuild">An optional value indicating whether or not to always include the project in the build even if it has no matching configuration.</param>
internal void Save(string rootPath, TextWriter writer, bool useFolders, bool collapseFolders = false, bool alwaysBuild = true)
internal void Save(string rootPath, TextWriter writer, bool useFolders, ISlnGenLogger logger, bool collapseFolders = false, bool alwaysBuild = true)
{
writer.WriteLine(Header, _fileFormatVersion);

Expand All @@ -400,7 +402,6 @@ internal void Save(string rootPath, TextWriter writer, bool useFolders, bool col
}

List<SlnProject> sortedProjects = _projects.OrderBy(i => i.IsMainProject ? 0 : 1).ThenBy(i => i.FullPath).ToList();

foreach (SlnProject project in sortedProjects)
{
string solutionPath = project.FullPath.ToRelativePath(rootPath).ToSolutionPath();
Copy link
Contributor Author

@mruxmohan4 mruxmohan4 Nov 15, 2023 •
edited
Loading

Choose a reason for hiding this comment

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

@jeffkl - Should multiple drives be detected before this section as well (as in, should this solution path, which is written to the stream, use absolute paths)?

Copy link
Collaborator

Choose a reason for hiding this comment

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

I don't think detecting multiple drives is the right thing to do. Instead, if the drive that the current project is different from the drive that the solution file is being saved to, we need to use rooted absolute paths for that project's entry. If the project is on the same drive, the path should be relative. If one or more projects are on a different drive, then emit the warning. Does that sound right to you?

Copy link
Contributor Author

@mruxmohan4 mruxmohan4 Nov 15, 2023 •
edited
Loading

Choose a reason for hiding this comment

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

I added a condition to detect whether the current project's drive is different from that of the solution file to ensure absolute paths are used if that differs, but I still needed to include the hasFullPath check since that led to the ArgumentException (the hierarchy includes a topmost-level folder that has "" for its absolute path).

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Latest test output:

Microsoft Visual Studio Solution File, Format Version 12.00
Project("{8BC9CEB8-8B4A-11D0-8D11-00A0C91BC942}") = "ProjectA", "C:\src\test\slngen-vcxitems\ProjectA\ProjectA.vcxitems", "{369B6908-BF24-4C02-8BB0-4139FD699D1D}"
EndProject
Project("{8BC9CEB8-8B4A-11D0-8D11-00A0C91BC942}") = "ProjectB", "ProjectB.vcxitems", "{905547D1-E9AF-4D80-8F83-2751E83D4557}"
EndProject
Project("{2150E333-8FDC-42A3-9474-1A3956D46DE8}") = "ProjectA", "C:\src\test\slngen-vcxitems\ProjectA", "{18234187-1FD6-4D2F-A26A-F9FC5FD67E9E}"
EndProject
Project("{2150E333-8FDC-42A3-9474-1A3956D46DE8}") = "slngen-vcxitems", "C:\src\test\slngen-vcxitems", "{048D148C-0E53-4770-A80E-40E40B181400}"
EndProject
Project("{2150E333-8FDC-42A3-9474-1A3956D46DE8}") = "test", "C:\src\test", "{59CC9E02-BE3C-4BCD-96A1-3243B7D7036F}"
EndProject
Project("{2150E333-8FDC-42A3-9474-1A3956D46DE8}") = "src", "C:\src", "{D275FB7E-3968-465B-9679-3E738EB5E583}"
EndProject
Project("{2150E333-8FDC-42A3-9474-1A3956D46DE8}") = "ProjectB", "..\ProjectB", "{642B108B-866B-4B78-AD36-FAF88B6ECD4E}"
EndProject
Global
	GlobalSection(SolutionConfigurationPlatforms) = preSolution
		Debug|x64 = Debug|x64
		Release|x64 = Release|x64
	EndGlobalSection
	GlobalSection(ProjectConfigurationPlatforms) = postSolution
	EndGlobalSection
	GlobalSection(SolutionProperties) = preSolution
		HideSolutionNode = FALSE
	EndGlobalSection
	GlobalSection(NestedProjects) = preSolution
		{369B6908-BF24-4C02-8BB0-4139FD699D1D} = {18234187-1FD6-4D2F-A26A-F9FC5FD67E9E}
		{18234187-1FD6-4D2F-A26A-F9FC5FD67E9E} = {048D148C-0E53-4770-A80E-40E40B181400}
		{048D148C-0E53-4770-A80E-40E40B181400} = {59CC9E02-BE3C-4BCD-96A1-3243B7D7036F}
		{59CC9E02-BE3C-4BCD-96A1-3243B7D7036F} = {D275FB7E-3968-465B-9679-3E738EB5E583}
		{D275FB7E-3968-465B-9679-3E738EB5E583} = {BBAA5F6C-FCF6-4088-8AB6-C7650C6BE6AB}
		{905547D1-E9AF-4D80-8F83-2751E83D4557} = {642B108B-866B-4B78-AD36-FAF88B6ECD4E}
		{642B108B-866B-4B78-AD36-FAF88B6ECD4E} = {F8F3097E-FE30-46C8-B7CD-C73AEFE68DCE}
	EndGlobalSection
	GlobalSection(ExtensibilityGlobals) = postSolution
		SolutionGuid = {D0FD770F-48DF-419B-9E83-07C49EEFFD41}
	EndGlobalSection
	GlobalSection(SharedMSBuildProjectFiles) = preSolution
	EndGlobalSection
EndGlobal

Expand Down Expand Up @@ -441,9 +442,27 @@ internal void Save(string rootPath, TextWriter writer, bool useFolders, bool col

if (hierarchy != null)
{
bool logDriveWarning = false;
string rootPathDrive = Path.GetPathRoot(rootPath);
foreach (SlnFolder folder in hierarchy.Folders)
{
string projectSolutionPath = (useFolders ? folder.FullPath.ToRelativePath(rootPath) : folder.FullPath).ToSolutionPath();
bool useSeparateDrive = false;
bool hasFullPath = !string.IsNullOrEmpty(folder.FullPath);
if (hasFullPath)
{
string folderPathDrive = Path.GetPathRoot(folder.FullPath);
if (!string.Equals(rootPathDrive, folderPathDrive, StringComparison.OrdinalIgnoreCase))
{
useSeparateDrive = true;
if (!logDriveWarning)
{
logger.LogWarning($"Detected folder on a different drive from the root solution path {rootPath}. This folder should not be committed to source control since it does not contain a simple, relative path and is not guaranteed to work across machines.");
logDriveWarning = true;
}
}
}

string projectSolutionPath = (useFolders && !useSeparateDrive && hasFullPath ? folder.FullPath.ToRelativePath(rootPath) : folder.FullPath).ToSolutionPath();

// Try to preserve the folder GUID if a matching relative folder path was parsed from an existing solution
if (ExistingProjectGuids != null && ExistingProjectGuids.TryGetValue(projectSolutionPath, out Guid projectGuid))
Expand Down
2 changes: 1 addition & 1 deletion src/Microsoft.VisualStudio.SlnGen/SlnFolder.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -35,7 +35,7 @@ public SlnFolder(string path)
}

/// <summary>
/// Gets the <see cref="Guid" /> of the folder.
/// Gets or sets the <see cref="Guid" /> of the folder.
/// </summary>
public Guid FolderGuid { get; set; }

Expand Down