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

Make FilePath and DirectoryPath comparable by value #3075

Closed
augustoproiete opened this issue Feb 2, 2021 · 8 comments · Fixed by #3655
Closed

Make FilePath and DirectoryPath comparable by value #3075

augustoproiete opened this issue Feb 2, 2021 · 8 comments · Fixed by #3655
Assignees
Milestone

Comments

@augustoproiete
Copy link
Member

Currently FilePath and DirectoryPath comparisons are done by reference, and identical paths in different instances are considered to be different.

Example

var file1 = FilePath.FromString("./MyApp.csproj");
var file2 = FilePath.FromString("./MyApp.csproj");

Information("file1 == file2: {0}", file1 == file2);

// ...

var directory1 = DirectoryPath.FromString("./artifacts");
var directory2 = DirectoryPath.FromString("./artifacts");

Information("directory1 == directory2: {0}", directory1 == directory2);

What You Are Seeing?

C:\augustoproiete\cake\1.0.0-rc0003>dotnet cake test-compare-filepath.cake
file1 == file2: False
directory1 == directory2: False

What is Expected?

C:\augustoproiete\cake\1.0.0-rc0003>dotnet cake test-compare-filepath.cake
file1 == file2: True
directory1 == directory2: True

What version of Cake are you using?

1.0.0-rc0003

@devlead
Copy link
Member

devlead commented Feb 3, 2021

Important to think about is that any string comparison would need to take filesystem in consideration.

@augustoproiete
Copy link
Member Author

augustoproiete commented Feb 3, 2021

Important to think about is that any string comparison would need to take filesystem in consideration.

That's an interesting thought. What situation(s) do you think the user can create instances of FilePath and/or DirectoryPath that would not comparable by string based on different filesystems?

I did some initial tests based on the assumption that we would implement IEquatable<string> (and friends). on FilePath & DirectoryPath and do the comparison based on the FullPath string which goes through a lot of normalization that is filesystem-independent for the most part.

Some initial tests seem to indicate that FullPath would work regardless of the filesystem as long as we're comparing two instances of FilePath or two instances of DirectoryPath - that is, unless I missed something, LMK:

dotnet cake test-compare-filepath.cake

Running On: ***Windows***
---
relativeFilePath1: [MyProject/MyApp.csproj]
relativeFilePath2: [MyProject/MyApp.csproj]

absoluteFilePath1: [C:/augustoproiete/cake/1.0.0-rc0003/MyProject/MyApp.csproj]
absoluteFilePath2: [C:/augustoproiete/cake/1.0.0-rc0003/MyProject/MyApp.csproj]
absoluteFilePath3: [C:/augustoproiete/cake/1.0.0-rc0003/MyProject/MyApp.csproj]
absoluteFilePath4: [C:/augustoproiete/cake/1.0.0-rc0003/MyProject/MyApp.csproj]
absoluteFilePath5: [/Users/augustoproiete/cake/1.0.0-rc0003/MyProject/MyApp.csproj]
absoluteFilePath6: [/Users/augustoproiete/cake/1.0.0-rc0003/MyProject/MyApp.csproj]

relativeDirectoryPath1: [MyProject/bin]
relativeDirectoryPath2: [MyProject/bin]

absoluteDirectoryPath1: [C:/augustoproiete/cake/1.0.0-rc0003/MyProject/bin]
absoluteDirectoryPath2: [C:/augustoproiete/cake/1.0.0-rc0003/MyProject/bin]
absoluteDirectoryPath3: [C:/augustoproiete/cake/1.0.0-rc0003/MyProject/bin]
absoluteDirectoryPath4: [C:/augustoproiete/cake/1.0.0-rc0003/MyProject/bin]
absoluteDirectoryPath5: [/Users/augustoproiete/cake/1.0.0-rc0003/MyProject/bin]
absoluteDirectoryPath6: [/Users/augustoproiete/cake/1.0.0-rc0003/MyProject/bin]
dotnet cake test-compare-filepath.cake

Running On: ***OSX***
---
relativeFilePath1: [MyProject/MyApp.csproj]
relativeFilePath2: [MyProject/MyApp.csproj]

absoluteFilePath1: [/Users/augustoproiete/cake/1.0.0-rc0003/MyProject/MyApp.csproj]
absoluteFilePath2: [/Users/augustoproiete/cake/1.0.0-rc0003/MyProject/MyApp.csproj]
absoluteFilePath3: [C:/augustoproiete/cake/1.0.0-rc0003/MyProject/MyApp.csproj]
absoluteFilePath4: [C:/augustoproiete/cake/1.0.0-rc0003/MyProject/MyApp.csproj]
absoluteFilePath5: [/Users/augustoproiete/cake/1.0.0-rc0003/MyProject/MyApp.csproj]
absoluteFilePath6: [/Users/augustoproiete/cake/1.0.0-rc0003/MyProject/MyApp.csproj]

relativeDirectoryPath1: [MyProject/bin]
relativeDirectoryPath2: [MyProject/bin]

absoluteDirectoryPath1: [/Users/augustoproiete/cake/1.0.0-rc0003/MyProject/bin]
absoluteDirectoryPath2: [/Users/augustoproiete/cake/1.0.0-rc0003/MyProject/bin]
absoluteDirectoryPath3: [C:/augustoproiete/cake/1.0.0-rc0003/MyProject/bin]
absoluteDirectoryPath4: [C:/augustoproiete/cake/1.0.0-rc0003/MyProject/bin]
absoluteDirectoryPath5: [/Users/augustoproiete/cake/1.0.0-rc0003/MyProject/bin]
absoluteDirectoryPath6: [/Users/augustoproiete/cake/1.0.0-rc0003/MyProject/bin]

The only situation that might seem a little odd at first would be comparing, for example, a DirectoryPath instance with /Users/augustoproiete/cake/1.0.0-rc0003/MyProject/bin and another one with C:/augustoproiete/cake/1.0.0-rc0003/MyProject/bin, they would be considered different via FullPath string comparison, which I think would be correct.


build.cake

var relativeFilePath1 = FilePath.FromString("./MyProject/MyApp.csproj");
var relativeFilePath2 = FilePath.FromString(".\\MyProject\\MyApp.csproj");

var absoluteFilePath1 = MakeAbsolute(FilePath.FromString("./MyProject/MyApp.csproj"));
var absoluteFilePath2 = MakeAbsolute(FilePath.FromString(".\\MyProject\\MyApp.csproj"));
var absoluteFilePath3 = FilePath.FromString("C:/augustoproiete/cake/1.0.0-rc0003/MyProject/MyApp.csproj");
var absoluteFilePath4 = FilePath.FromString("C:\\augustoproiete\\cake\\1.0.0-rc0003\\MyProject\\MyApp.csproj");
var absoluteFilePath5 = FilePath.FromString("/Users/augustoproiete/cake/1.0.0-rc0003/MyProject/MyApp.csproj");
var absoluteFilePath6 = FilePath.FromString("\\Users\\augustoproiete\\cake\\1.0.0-rc0003\\MyProject\\MyApp.csproj");

var relativeDirectoryPath1 = DirectoryPath.FromString("./MyProject/bin");
var relativeDirectoryPath2 = DirectoryPath.FromString(".\\MyProject\\bin");

var absoluteDirectoryPath1 = MakeAbsolute(DirectoryPath.FromString("./MyProject/bin"));
var absoluteDirectoryPath2 = MakeAbsolute(DirectoryPath.FromString(".\\MyProject\\bin"));
var absoluteDirectoryPath3 = DirectoryPath.FromString("C:/augustoproiete/cake/1.0.0-rc0003/MyProject/bin");
var absoluteDirectoryPath4 = DirectoryPath.FromString("C:\\augustoproiete\\cake\\1.0.0-rc0003\\MyProject\\bin");
var absoluteDirectoryPath5 = DirectoryPath.FromString("/Users/augustoproiete/cake/1.0.0-rc0003/MyProject/bin");
var absoluteDirectoryPath6 = DirectoryPath.FromString("\\Users\\augustoproiete\\cake\\1.0.0-rc0003\\MyProject\\bin");

Information("Running On: {0}", Context.Environment.Platform.Family);
Information("---");

Information("relativeFilePath1: [{0}]", relativeFilePath1.FullPath);
Information("relativeFilePath2: [{0}]", relativeFilePath2.FullPath);
Information("");

Information("absoluteFilePath1: [{0}]", absoluteFilePath1.FullPath);
Information("absoluteFilePath2: [{0}]", absoluteFilePath2.FullPath);
Information("absoluteFilePath3: [{0}]", absoluteFilePath3.FullPath);
Information("absoluteFilePath4: [{0}]", absoluteFilePath4.FullPath);
Information("absoluteFilePath5: [{0}]", absoluteFilePath5.FullPath);
Information("absoluteFilePath6: [{0}]", absoluteFilePath6.FullPath);
Information("");

Information("relativeDirectoryPath1: [{0}]", relativeDirectoryPath1.FullPath);
Information("relativeDirectoryPath2: [{0}]", relativeDirectoryPath2.FullPath);
Information("");

Information("absoluteDirectoryPath1: [{0}]", absoluteDirectoryPath1.FullPath);
Information("absoluteDirectoryPath2: [{0}]", absoluteDirectoryPath2.FullPath);
Information("absoluteDirectoryPath3: [{0}]", absoluteDirectoryPath3.FullPath);
Information("absoluteDirectoryPath4: [{0}]", absoluteDirectoryPath4.FullPath);
Information("absoluteDirectoryPath5: [{0}]", absoluteDirectoryPath5.FullPath);
Information("absoluteDirectoryPath6: [{0}]", absoluteDirectoryPath6.FullPath);
Information("");

@devlead
Copy link
Member

devlead commented Feb 3, 2021

Casing would be one thing. On Windows files are case insensitive.

@augustoproiete
Copy link
Member Author

Casing would be one thing. On Windows files are case insensitive.

Make sense.

Perhaps we can use a similar approach to PathHelper and perform case-insensitive comparison on Windows, and case-sensitive otherwise.

Maybe add an exception when comparing paths that are rooted with drive letter to always be case-insensitive (even if not on Windows) - also similar approach to PathHelper.

@franciscomoloureiro
Copy link
Contributor

I can take this task.

I did some investigation and found this PathComparer class, it already has the platform dependant comparasion you discussed previously.

Should I try to reuse its logic when implementing IEquatable of FilePath and DirectoryPath

@patriksvensson
Copy link
Member

PathComparer is designed for this scenario.

@franciscomoloureiro
Copy link
Contributor

Noted. I will take care of this one then :)

@cake-build-bot
Copy link

🎉 This issue has been resolved in version v2.0.0 🎉

The release is available on:

Your GitReleaseManager bot 📦🚀

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging a pull request may close this issue.

5 participants