DuplicateWrites: Add support for robocopy task detection (from Microsoft.Build.Artifacts)#592
Conversation
This isn't complete - needs translations and some eyes
|
I just added some help on how to re-run the ResourcesGenerator tool to regenerate Strings.json: Basically you specify a list of resource strings from MSBuild you'd like to use here: run the ResourcesGenerator tool and it will generate Strings.json, so you'll be able to use it in Strings.cs |
|
I'm not sure whether the Robocopy task is localized or not and how to retrieve the localized strings for it. If it's not localized you don't need to modify Strings.json at all and just hardcode the English string in Strings.cs. I'd be OK with just the English string for starters. |
|
Looks great! |
I'm not sure if we should include skipped here...it feels like we should because it's coming from one place then another depending on date, etc. - whatever the condition is failing `.ShouldCopy()`. In the end, it's 2 files aiming at the same place and that's still potentially non-deterministic.
| .Replace(@"\{0}", @"(?<From>[^\""]+)") | ||
| .Replace(@"\{1}", @"(?<To>[^\""]+)") | ||
| ); | ||
| .Replace(@"\{1}", @"(?<To>[^\""]+)"), RegexOptions.Compiled); |
There was a problem hiding this comment.
I noticed these weren't optimized (but run quite a bit on some builds) - happy to separate this optimization into another PR to keep things clean!
There was a problem hiding this comment.
This is fine, thanks.
I benchmarked the difference a while back and it does have benefits:
https://github.com/KirillOsenkov/Benchmarks/blob/29e6de831b11abd9485b16cc24f3eb310e989ed6/src/Tests/Regex.cs#L8-L29
|
Whelp, I learned a few things. The reason I couldn't find a localization, even internally is that task isn't actually using Robocopy. It's just a file loop - see the Microsoft.Build.Artifacts package source here: https://github.com/microsoft/MSBuildSdks/blob/main/src/Artifacts/Tasks/Robocopy.cs#L66-L70 I still think this analyzer is useful and I've added the skipped and failed cases because they're all ultimately duplicate writes. e.g. a skip may be a date mismatch and non-deterministic for a build and a fail could be a attempted simultaneous double write. In either case I think it's in the spirit of what we're trying to detect. Happy to adjust though if that doesn't sound right! cc @jaredpar who may be interested in this one. |
|
Haha, Robocopy isn't using Robocopy! TIL. |
|
Love it! |
Overall: adds RoboCopy task support for duplicate file copy detection. This is something very prevalent in some solutions I've been working on and would be a good win if we had it on the dupe detection front.
The task it's detecting is from the Microsoft.Build.Artifacts package which it turns out...isn't actually using Robocopy because...yeah, no clue. Here's the source of the task we're matching: https://github.com/microsoft/MSBuildSdks/blob/543e965191417dee65471ee57a6702289847b49b/src/Artifacts/Tasks/Robocopy.cs