Skip to content

Fix MakeRelative regression in v16.10 (rebased to vs16.10 branch) #6513

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

Merged
rainersigwald merged 2 commits into from
Jun 8, 2021
Merged

Fix MakeRelative regression in v16.10 (rebased to vs16.10 branch) #6513

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
149c776
Fix MakeRelative regression in v16.10
dsparkplug Jun 2, 2021
fe2c68f
Pulled in some external changes to Microsoft.Build.cs
dsparkplug Jun 3, 2021
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
4 changes: 2 additions & 2 deletions ref/Microsoft.Build/net/Microsoft.Build.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1574,8 +1574,8 @@ protected GraphBuildOptions(Microsoft.Build.Graph.GraphBuildOptions original) {
public virtual bool Equals(Microsoft.Build.Graph.GraphBuildOptions other) { throw null; }
public override bool Equals(object obj) { throw null; }
public override int GetHashCode() { throw null; }
public static bool operator ==(Microsoft.Build.Graph.GraphBuildOptions r1, Microsoft.Build.Graph.GraphBuildOptions r2) { throw null; }
public static bool operator !=(Microsoft.Build.Graph.GraphBuildOptions r1, Microsoft.Build.Graph.GraphBuildOptions r2) { throw null; }
public static bool operator ==(Microsoft.Build.Graph.GraphBuildOptions left, Microsoft.Build.Graph.GraphBuildOptions right) { throw null; }
public static bool operator !=(Microsoft.Build.Graph.GraphBuildOptions left, Microsoft.Build.Graph.GraphBuildOptions right) { throw null; }
protected virtual bool PrintMembers(System.Text.StringBuilder builder) { throw null; }
public override string ToString() { throw null; }
public virtual Microsoft.Build.Graph.GraphBuildOptions <Clone>$() { throw null; }
Expand Down
4 changes: 2 additions & 2 deletions ref/Microsoft.Build/netstandard/Microsoft.Build.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1568,8 +1568,8 @@ protected GraphBuildOptions(Microsoft.Build.Graph.GraphBuildOptions original) {
public virtual bool Equals(Microsoft.Build.Graph.GraphBuildOptions other) { throw null; }
public override bool Equals(object obj) { throw null; }
public override int GetHashCode() { throw null; }
public static bool operator ==(Microsoft.Build.Graph.GraphBuildOptions r1, Microsoft.Build.Graph.GraphBuildOptions r2) { throw null; }
public static bool operator !=(Microsoft.Build.Graph.GraphBuildOptions r1, Microsoft.Build.Graph.GraphBuildOptions r2) { throw null; }
public static bool operator ==(Microsoft.Build.Graph.GraphBuildOptions left, Microsoft.Build.Graph.GraphBuildOptions right) { throw null; }
public static bool operator !=(Microsoft.Build.Graph.GraphBuildOptions left, Microsoft.Build.Graph.GraphBuildOptions right) { throw null; }
Comment on lines +1571 to +1572
Copy link
Member

@rainersigwald rainersigwald Jun 3, 2021

Choose a reason for hiding this comment

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

Sorry about this; we fixed if for 16.11 but not 16.10.

protected virtual bool PrintMembers(System.Text.StringBuilder builder) { throw null; }
public override string ToString() { throw null; }
public virtual Microsoft.Build.Graph.GraphBuildOptions <Clone>$() { throw null; }
Expand Down
7 changes: 6 additions & 1 deletion src/Shared/FileUtilities.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1078,7 +1078,12 @@ internal static string MakeRelative(string basePath, string path)
{
sb.Append(splitPath[i]).Append(Path.DirectorySeparatorChar);
}
sb.Length--;

if (fullPath[fullPath.Length - 1] != Path.DirectorySeparatorChar)
Copy link
Member

@danmoseley danmoseley Jun 4, 2021

Choose a reason for hiding this comment

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

should it also check Path.AlternateDirectorySeparatorChar?

Copy link
Author

@dsparkplug dsparkplug Jun 5, 2021

Choose a reason for hiding this comment

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

My understanding is that the fullPath has already been normalised within the Path.GetFullPath function so should only contain Path.DirectorySeparatorChar?

Copy link
Contributor

@Forgind Forgind Jun 7, 2021

Choose a reason for hiding this comment

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

Looks like that's true for unix. Windows calls down into the kernel, but I'm assuming it's the same.

Copy link
Author

@dsparkplug dsparkplug Jun 7, 2021

Choose a reason for hiding this comment

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

Sure is:

Microsoft (R) Visual C# Interactive Compiler version 3.10.0-4.21269.26 ()
Loading context from 'CSharpInteractive.rsp'.
Type "#help" for more information.
> System.Console.WriteLine(System.IO.Path.GetFullPath("C:/abc/def/"));
C:\abc\def\
> System.Console.WriteLine(System.IO.Path.GetFullPath("C:/abc/def"));
C:\abc\def
>

{
sb.Length--;
}

return StringBuilderCache.GetStringAndRelease(sb);
}

Expand Down
37 changes: 37 additions & 0 deletions src/Shared/UnitTests/FileUtilities_Tests.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -97,6 +97,24 @@ public void MakeRelativeTests()
Assert.Equal(@"\\host\path\file", FileUtilities.MakeRelative(@"c:\abc\def", @"\\host\path\file"));
Assert.Equal(@"\\host\d$\file", FileUtilities.MakeRelative(@"c:\abc\def", @"\\host\d$\file"));
Assert.Equal(@"..\fff\ggg.hh", FileUtilities.MakeRelative(@"c:\foo\bar\..\abc\cde", @"c:\foo\bar\..\abc\fff\ggg.hh"));

/* Directories */
Assert.Equal(@"def\", FileUtilities.MakeRelative(@"c:\abc\", @"c:\abc\def\"));
Assert.Equal(@"..\", FileUtilities.MakeRelative(@"c:\abc\def\xyz\", @"c:\abc\def\"));
Assert.Equal(@"..\ttt\", FileUtilities.MakeRelative(@"c:\abc\def\xyz\", @"c:\abc\def\ttt\"));
Assert.Equal(@".", FileUtilities.MakeRelative(@"c:\abc\def\", @"c:\abc\def\"));

/* Directory + File */
Assert.Equal(@"def", FileUtilities.MakeRelative(@"c:\abc\", @"c:\abc\def"));
Assert.Equal(@"..\..\ghi", FileUtilities.MakeRelative(@"c:\abc\def\xyz\", @"c:\abc\ghi"));
Assert.Equal(@"..\ghi", FileUtilities.MakeRelative(@"c:\abc\def\xyz\", @"c:\abc\def\ghi"));
Assert.Equal(@"..\ghi", FileUtilities.MakeRelative(@"c:\abc\def\", @"c:\abc\ghi"));

/* File + Directory */
Assert.Equal(@"def\", FileUtilities.MakeRelative(@"c:\abc", @"c:\abc\def\"));
Assert.Equal(@"..\", FileUtilities.MakeRelative(@"c:\abc\def\xyz", @"c:\abc\def\"));
Assert.Equal(@"..\ghi\", FileUtilities.MakeRelative(@"c:\abc\def\xyz", @"c:\abc\def\ghi\"));
Assert.Equal(@".", FileUtilities.MakeRelative(@"c:\abc\def", @"c:\abc\def\"));
}
else
{
Expand All @@ -106,6 +124,25 @@ public void MakeRelativeTests()
Assert.Equal(@"../ttt/foo.cpp", FileUtilities.MakeRelative(@"/abc/def/xyz/", @"/abc/def/ttt/foo.cpp"));
Assert.Equal(@"foo.cpp", FileUtilities.MakeRelative(@"/abc/def", @"foo.cpp"));
Assert.Equal(@"../fff/ggg.hh", FileUtilities.MakeRelative(@"/foo/bar/../abc/cde", @"/foo/bar/../abc/fff/ggg.hh"));

/* Directories */
Assert.Equal(@"def/", FileUtilities.MakeRelative(@"/abc/", @"/abc/def/"));
Assert.Equal(@"../", FileUtilities.MakeRelative(@"/abc/def/xyz/", @"/abc/def/"));
Assert.Equal(@"../ttt/", FileUtilities.MakeRelative(@"/abc/def/xyz/", @"/abc/def/ttt/"));
Assert.Equal(@".", FileUtilities.MakeRelative(@"/abc/def/", @"/abc/def/"));

/* Directory + File */
Assert.Equal(@"def", FileUtilities.MakeRelative(@"/abc/", @"/abc/def"));
Assert.Equal(@"../../ghi", FileUtilities.MakeRelative(@"/abc/def/xyz/", @"/abc/ghi"));
Assert.Equal(@"../ghi", FileUtilities.MakeRelative(@"/abc/def/xyz/", @"/abc/def/ghi"));
Assert.Equal(@"../ghi", FileUtilities.MakeRelative(@"/abc/def/", @"/abc/ghi"));

/* File + Directory */
Assert.Equal(@"def/", FileUtilities.MakeRelative(@"/abc", @"/abc/def/"));
Assert.Equal(@"../", FileUtilities.MakeRelative(@"/abc/def/xyz", @"/abc/def/"));
Assert.Equal(@"../ghi/", FileUtilities.MakeRelative(@"/abc/def/xyz", @"/abc/def/ghi/"));
Assert.Equal(@".", FileUtilities.MakeRelative(@"/abc/def", @"/abc/def/"));

}
}

Expand Down