search

KirillOsenkov / MSBuildStructuredLog

A logger for MSBuild that records a structured representation of executed targets, tasks, property and item values.
MIT License
1.42k stars 188 forks source link

DuplicateWrites: Add support for robocopy task detection (from Microsoft.Build.Artifacts) #592

Closed NickCraver closed 2 years ago

NickCraver commented 2 years ago

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

KirillOsenkov commented 2 years ago

I just added some help on how to re-run the ResourcesGenerator tool to regenerate Strings.json: https://github.com/KirillOsenkov/MSBuildStructuredLog/commit/afe5eead3fc3e542b88b5827d0540cb011fab4c7

Basically you specify a list of resource strings from MSBuild you'd like to use here: https://github.com/KirillOsenkov/MSBuildStructuredLog/blob/afe5eead3fc3e542b88b5827d0540cb011fab4c7/src/ResourcesGenerator/ResourceCreator.cs#L22

run the ResourcesGenerator tool and it will generate Strings.json, so you'll be able to use it in Strings.cs

KirillOsenkov commented 2 years ago

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.

KirillOsenkov commented 2 years ago

Looks great!

NickCraver commented 2 years ago

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.

KirillOsenkov commented 2 years ago

Haha, Robocopy isn't using Robocopy! TIL.

jaredpar commented 2 years ago

Love it!