You signed in with another tab or window. Reload to refresh your session.You signed out in another tab or window. Reload to refresh your session.You switched accounts on another tab or window. Reload to refresh your session.Dismiss alert
In the Nerdbank.GitVersioning.Tasks.CompareFiles class, the IsContentOfFilesTheSame method calls Stream.Read and assumes that a return value smaller than requested means that it got to the end of the file. As mentioned in #797 (comment), such an assumption is generally a bug. Here though, the stream refers to a regular file, which makes the assumption more likely to hold.
// We should never get here since we compared file lengths, but
// this is a sanity check.
returnfalse;
}
The CompareFiles task is used in Nerdbank.GitVersioning.targets, for deciding whether to run the Copy task. The bug might cause the task to consider the files different when they are actually identical; the target would then needlessly copy the file and cause MSBuild to execute other targets, resulting in a slower incremental build but eventually the correct artefacts.
It might be possible to delete the CompareFiles task altogether and use the SkipUnchangedFiles parameter of the Copy task instead.
The task was faulty because it assumed `Stream.Read` would fill the buffer if at all possible. But we also don't need it, since the `Copy` task itself has similar functionality.
Fixes#825
Thanks for finding the bug. It turns out that Copy.SkipUnchangedFiles employs a cheap trick (probably for perf reasons) and will assume that the files are different when the timestamp is different, so it defeats the incremental build preservation that I'm attempting to do. I'll have to fix my task.
It's a cheap fix, in that it gives up streaming during the compare, but it's far more likely to be accurate than if I tried to maintain streaming while fixing the original bug of assuming that `Stream.Read` will always fill the buffer.
Fixes#825
In the Nerdbank.GitVersioning.Tasks.CompareFiles class, the IsContentOfFilesTheSame method calls Stream.Read and assumes that a return value smaller than requested means that it got to the end of the file. As mentioned in #797 (comment), such an assumption is generally a bug. Here though, the stream refers to a regular file, which makes the assumption more likely to hold.
Nerdbank.GitVersioning/src/Nerdbank.GitVersioning.Tasks/CompareFiles.cs
Lines 127 to 133 in 719b0d5
The CompareFiles task is used in Nerdbank.GitVersioning.targets, for deciding whether to run the Copy task. The bug might cause the task to consider the files different when they are actually identical; the target would then needlessly copy the file and cause MSBuild to execute other targets, resulting in a slower incremental build but eventually the correct artefacts.
It might be possible to delete the CompareFiles task altogether and use the SkipUnchangedFiles parameter of the Copy task instead.
Nerdbank.GitVersioning/src/Nerdbank.GitVersioning.Tasks/build/Nerdbank.GitVersioning.targets
Lines 144 to 149 in a908c6d
Nerdbank.GitVersioning/src/Nerdbank.GitVersioning.Tasks/build/Nerdbank.GitVersioning.targets
Lines 181 to 186 in a908c6d
The text was updated successfully, but these errors were encountered: