-
Notifications
You must be signed in to change notification settings - Fork 1.4k
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Fix item update evaluation when path contains parenthesis and add test coverage #11293
Fix item update evaluation when path contains parenthesis and add test coverage #11293
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This looks like a promising start, but it isn't a complete fix. This only affects item update, and as I mentioned later in the issue, this bug also applies to include. I manually tested the include-only version with your changes just now, and it still doesn't work.
@Forgind interesting, I am checking your example and it seems to be fine now: |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'm confused by the fact that the lookup is now somethime searched by normalized and sometimes (removal) not-normalized path - if that can be cleared out as not concern - I'm ready to sign off.
If that's possibly a concern - it'd be great to see a number of executions (plus e.g. Stopwatch accumulated time for normalizations calls) of that line for e.g. OrchardCore - to see if we should just normalize and move on, or if we need to rething the datastructure
This fix is only on preview2 right? |
yes, it's correct. |
Fixes #11237
Context
Currently we put a non-normalized path in the dictionary
msbuild/src/Build/Evaluation/LazyItemEvaluator.cs
and later attempt to pull from it a normalized version
msbuild/src/Build/Evaluation/LazyItemEvaluator.cs
In runtime it looks like this when path contains parenthesis:
we put C:\msbuild\msbuild_yk\msbuild\artifacts\bin\bootstrap\core\CheckSignApk%28\Microsoft.NETCore.App
we pull C:\msbuild\msbuild_yk\msbuild\artifacts\bin\bootstrap\core\CheckSignApk(\Microsoft.NETCore.App
Changes Made
adjust operation of adding to the dictionary and pulling values from it.
Testing
UI + manual