Adding accepted SDK name match pattern to SDK manifests.#7597
Conversation
5855d66 to
330e3c6
Compare
9a51caf to
0f3fa49
Compare
c26cde0 to
1f894c1
Compare
rainersigwald
left a comment
There was a problem hiding this comment.
Overall looking very nice. This is mostly style/perf nits.
Co-authored-by: Rainer Sigwald <raines@microsoft.com>
| } | ||
| catch (RegexMatchTimeoutException ex) | ||
| { | ||
| ErrorUtilities.ThrowInternalError("Regular expression parsing exceeds timeout for manifest {0}. Error message: {1}", manifest.Name, ex.Message); |
There was a problem hiding this comment.
This doesn't feel like an internal error to me.
There was a problem hiding this comment.
I was also torn but the only real alternative is InvalidProjectFileException and since this is a problem with the SDK resolver I think InternalError is a better choice.
There was a problem hiding this comment.
I agree Internal > InvalidProjectFile, but why are those the only options? Internal makes it sound like it's a bug in MSBuild. It might be a bug in one of our SDK resolvers, but it might not be, and in fact, since we've tested ours, it presumably isn't. It's a fix-your-own-build problem.
There was a problem hiding this comment.
Well, it is not fix-your-own-build, rather fix-your-own-msbuild-cause-you-broke-it. I do not know a way to do that out of the project files. There is an option to specify additional folder for sdk resolvers and where customer's resolvers could be in theory located, but it is my understanding that it is a non-documented env variables.
| } | ||
| catch (RegexMatchTimeoutException ex) | ||
| { | ||
| ErrorUtilities.ThrowInternalError("Regular expression parsing exceeds timeout for manifest {0}. Error message: {1}", manifest.Name, ex.Message); |
There was a problem hiding this comment.
I was also torn but the only real alternative is InvalidProjectFileException and since this is a problem with the SDK resolver I think InternalError is a better choice.
benvillalobos
left a comment
There was a problem hiding this comment.
First pass, everything's looking good 👍
Going to give it another pass, but don't block on it if things move smoothly.
Fixes #7293
Context
Previously all SDK resolvers were loaded and then ordered by priority. The resolvers are tried one after one until one of them succeeds. In order to decrease the number of assemblies to be load we change the behavior since ChangeWave 17.4. Now the resolvers might specify the name pattern for sdks they intend to resolve. If sdk name does not match, we would not unnecessary load the resolver.
Changes Made
This change of algorithm might lead to a change in sdk resolution in the following situation:
Then before the change the project is resolved by resolver A and after with resolver B.
Testing