Fix an issue with FastFetch when deleting files #1730
Conversation
| } | ||
|
|
||
| deletedPaths.Add(filePath); |
There was a problem hiding this comment.
This line here is really the heart of the badness. I believe if we just deleted this one line, it would not only fix the bug but also greatly reduce the performance problem. We know for sure that 'filePath' is a file, and really 'deletedPaths' should always have been 'deletedFolders'. It would also greatly reduce the performance problem, as it'd reduce it from O(n^2) to something more like O(n logn) (because the number of directories is more or less log(number-of-files).
There was a problem hiding this comment.
I like that this has a performance improvement as well.
| @@ -90,10 +90,10 @@ public void CanParseBackwardsDiff() | |||
| diffBackwards.RequiredBlobs.Count.ShouldEqual(10); | |||
There was a problem hiding this comment.
This test is not all that it could be...
- It just tests counts. It'd be better if it ToString'ed the filenames that are in each set (where possible) so that when it fails you have some idea of what changed.
- It calls 'ParseDiffFile' rather than 'PerformDiff'. This misled me into thinking that 'PerformDiff' really didn't have any tests. 'ParseDiffFile', as it turns out, is just called by tests. It'd be better (and not hard to do) to make it call 'PerformDiff'.
Wouldn't take much to twist my arm to do that.
There was a problem hiding this comment.
I wouldn't worry too much about that. I imagine it would be difficult to modify the code in a way that is incorrect and still gets these numbers correct.
| @@ -1,4 +1,4 @@ | |||
| @ECHO OFF | |||
| @if "%_echo%"=="" (echo off) else (echo on) | |||
| SETLOCAL | |||
There was a problem hiding this comment.
I hope you don't mind a drive-by. I had some struggles getting these to run. We use this technique a bit around here to allow conditionally turning echo on to help diagnose the random "Error: wheel fell off" messages you get when running batch scripts in slightly incorrect environments"
There was a problem hiding this comment.
Sounds good to me. As long as the CI builds still pass with these messages, then I'm happy with the change.
| @@ -1,4 +1,4 @@ | |||
| @ECHO OFF | |||
| @if "%_echo%"=="" (echo off) else (echo on) | |||
| SETLOCAL | |||
There was a problem hiding this comment.
Sounds good to me. As long as the CI builds still pass with these messages, then I'm happy with the change.
| } | ||
|
|
||
| deletedPaths.Add(filePath); |
There was a problem hiding this comment.
I like that this has a performance improvement as well.
| @@ -90,10 +90,10 @@ public void CanParseBackwardsDiff() | |||
| diffBackwards.RequiredBlobs.Count.ShouldEqual(10); | |||
There was a problem hiding this comment.
I wouldn't worry too much about that. I imagine it would be difficult to modify the code in a way that is incorrect and still gets these numbers correct.
Major Updates ---------------- * Comes with `microsoft/git` v2.32.0.vfs.0.4. * This release will be available via `winget`. * The `gvfs upgrade` verb is deprecated. Any future updates will be taken via `winget`. * This version of Git has an upgrade mechanism that can advance independently of VFS for Git. Run `git update-microsoft-git` to update your Git version. * The build system has been updated to use a more recent version of .NET. * All macOS and POSIX code has been removed to simplify the build system. * A FastFetch bug around deleting files has been fixed. * A prefetch bug around downloading a small number of files has been fixed. Special thanks to contributors @ldennington, @tyrielv, @50Wliu, and @SteveBenz. Pull Requests --------------- * #1738: Bumping version of update-winget action * #1747: UpgradeVerb: write deprecation notice * #1745: do not exit early if no blobs found for a period when prefetching * #1740: Skip launching UI if running unattended * #1737: Update Git to v2.32.0 * #1746: Delete custom upgrader * #1741: Fix winget tag specification * #1744: Overhaul build and project systems and drop Mac/POSIX code * #1736: Update Git to v2.31.1.vfs.0.3 * #1735: Bumping winget action version * #1734: Adding winget workflow * #1733: Update Git to include 2.31.1 * #1730: Fix an issue with FastFetch when deleting files * #1732: Use one NuGet feed * #1726: Config: commitGraph.generationVersion=1
This fixes #1729 and also takes care of a performance problem in the same method.