Backward-compatibility with KeyValuePair<string, string> metadata items [#8864] - #8870
#8870Backward-compatibility with KeyValuePair<string, string> metadata items [#8864]#8870
Conversation
|
@dotnet-policy-service agree |
rainersigwald
left a comment
There was a problem hiding this comment.
Now that I understand the problem, I'm not sure this fix is sufficient; there could theoretically be an ITaskItem implementation that returned a totally custom type with non-KVP non-DictionaryEntry backing for its IEnumerable implementation.
Taking this would certainly cover the likely implementations, but I'm starting to think we should go back to the foreach for this fallback code path.
|
(oh and also . . . thanks for the PR!) |
7b58e87 to
a4e179d
Compare
|
@donJoseLuis @rainersigwald I have updated the PR based on the suggestions to use P.S. Do we care that we are assuming all DictionaryEntry's will be <string, string>? |
No, that's ok--it's the only thing that would ever have worked AFAIK. |
|
Updated to loop DictionaryEntry's in the foreach. |
JanKrivanek
left a comment
There was a problem hiding this comment.
Thank you for diving into this!
If the call to SetMetadata is intentionall - please explain the reasoning in the description.
Without that - it looks good to go
rainersigwald
left a comment
There was a problem hiding this comment.
One more small nit (we can avoid reallocations by specifying the size of the list up front). Thanks for working through this, it's looking great!
@JaynieBai, could you please write a test for this case? We'll need a task that returns a custom ITaskItem implementation that has a custom IDictionary type returned from CloneCustomMetadata().
Co-authored-by: Rainer Sigwald <raines@microsoft.com>
Yeah, I will do that |
|
@rainersigwald - all expressed concerns seem to be addressed - do you want to revise? |
|
/backport to vs17.7 |
|
Started backporting to vs17.7: https://github.com/dotnet/msbuild/actions/runs/5415050762 |
Fixes #8864
Context
TaskExecutionHost.cs
Changes Made
Fallback to the prior behavior of expecting metadata items of type KeyValuePair<string, string>
Testing
This has not been tested