Sdk resolver updates by dsplaisted · Pull Request #5269 · dotnet/msbuild

dsplaisted · 2020-04-11T01:20:42Z

Add more abilities to SDK resolvers

Return any number of paths (zero, one, or many) in a successful result
Return MSBuild items and properties to add to the evaluation result

Resolves #5239

wli3 · 2020-04-16T21:38:08Z

test?

dsplaisted · 2020-04-23T00:26:14Z

test?

It's still a draft :-)

dsplaisted · 2020-04-23T00:26:22Z

/azp run

azure-pipelines · 2020-04-23T00:26:27Z

Pull request contains merge conflicts.

ITranslator methods now take an ObjectTranslator delegate which can translate an object in both directions, as opposed to requiring that objects implement ITranslatable and that a factory be passed in. The TranslatorHelpers class includes extension methods which support the old APIs, so call sites that use ITranslatable objects and NodePacketValueFactory still work.

- Return any number of paths (zero, one, or many) in a successful result - Return MSBuild items and properties to add to the evaluation result Resolves dotnet#5239

This helps support local dev cycles consuming those packages

dsplaisted · 2020-05-07T01:43:47Z

@rainersigwald @Forgind @benvillalobos I think this is basically ready for review. I think I still need to update the XML documentation for the new and refactored APIs, but otherwise this should be ready.

rainersigwald

Haven't looked at tests in detail yet. This seems generally ok to me, though.

src/Framework/Sdk/SdkResultPathAndVersion.cs

src/Shared/ITranslator.cs

src/Framework/Sdk/SdkResultItem.cs

src/Build/BackEnd/Components/SdkResolution/SdkResult.cs

src/Build/Evaluation/Evaluator.cs

rainersigwald · 2020-05-18T18:50:20Z

src/Build/Evaluation/Evaluator.cs

Can you add a test for this where the resolver produces some horrible special-character nonsense?

Are there any existing tests for horrible special characters that I can get the nonsense from? :-)

src\Build.UnitTests\EscapingInProjects_Tests.cs has a bunch related to tasks.

Returning properties/items/metadata with %-followed-by-two-digits and validating it doesn't get unescaped is probably sufficient.

src/Build/Evaluation/Evaluator.cs

Forgind

I mostly looked at tests, although I didn't understand all of them. Looks good to me.

Directory.Build.targets

src/Build.UnitTests/BackEnd/SdkResolverService_Tests.cs

src/Build.UnitTests/BackEnd/SdkResultOutOfProc_Tests.cs

src/Build.UnitTests/Collections/OMcollections_tests.cs

src/Build.UnitTests/InternalEngineHelpers.cs

src/Build/BackEnd/Components/SdkResolution/SdkResolverService.cs

src/Build.UnitTests/BackEnd/SdkResolverService_Tests.cs

src/Build.UnitTests/BackEnd/SdkResultOutOfProc_Tests.cs

src/Build.UnitTests/BackEnd/SdkResolverService_Tests.cs

src/Framework/Sdk/SdkResultFactory.cs

src/Framework/Sdk/SdkReference.cs

src/Build/Evaluation/Evaluator.cs

rainersigwald · 2020-06-01T20:55:03Z

src/Build/Evaluation/Evaluator.cs

src\Build.UnitTests\EscapingInProjects_Tests.cs has a bunch related to tasks.

Returning properties/items/metadata with %-followed-by-two-digits and validating it doesn't get unescaped is probably sufficient.

… and items

Co-authored-by: Rainer Sigwald <raines@microsoft.com>

dsplaisted · 2020-06-05T20:03:47Z

@rainersigwald @Forgind I think I've addressed all the feedback and this is ready to merge. I'd appreciate a signoff. Thanks!

src/Build/Evaluation/Evaluator.cs

src/Build.UnitTests/Evaluation/SdkResultEvaluation_Tests.cs

src/Framework/Sdk/SdkResultFactory.cs

rainersigwald · 2020-06-10T15:53:52Z

@dsplaisted did this merge to 16.7 for a specific reason? Most of our development is in master.

dsplaisted requested a review from rainersigwald April 15, 2020 01:27

dsplaisted added 8 commits April 22, 2020 17:30

Fix nullable translation

74a6d04

Simplify TranslatorHelpers

1823cfe

Add more abilities to SDK resolvers

a02f36b

- Return any number of paths (zero, one, or many) in a successful result - Return MSBuild items and properties to add to the evaluation result Resolves dotnet#5239

Fix discarding data from SdkResult from a different node

32c1a72

Implement Equals and GetHashCode for SdkResult values

4c9d7cb

Use hash for SdkResult projects and store in project cache

c7e2b11

Delete produced packages from NuGet cache

1c0fba7

This helps support local dev cycles consuming those packages

dsplaisted force-pushed the sdk-resolver-updates branch from 48fe2e1 to 1c0fba7 Compare April 23, 2020 00:31

dsplaisted changed the base branch from master to vs16.7 April 23, 2020 00:31

dsplaisted mentioned this pull request Apr 24, 2020

Refactor translator APIs #5223

Closed

dsplaisted marked this pull request as ready for review May 7, 2020 01:42

rainersigwald reviewed May 18, 2020

View reviewed changes

dsplaisted force-pushed the sdk-resolver-updates branch from c9d0b11 to 86b98c7 Compare May 21, 2020 06:01

Forgind reviewed May 22, 2020

View reviewed changes

rainersigwald reviewed Jun 1, 2020

View reviewed changes

dsplaisted added 8 commits June 4, 2020 16:50

Add resolver service tests for multiple paths and properties/items

01070fb

Check additional path versions from SdkResult

d3a3483

Add test for preprocessor output of SDK Resolver returning properties…

947d326

… and items

Add SdkResult evaluation tests

4e00970

Add out of process tests for MSBuild SDK resolvers

a2cd0c2

Fix test failing due to uninitialized property warning

ed24ddf

Update XML documentation for SDK Resolvers and Translator APIs

8e31d5c

Code review feedback

cb25f25

dsplaisted added 3 commits June 4, 2020 16:50

Get rid of SdkResultPathAndVersion

ca9de92

Make Sdk name and version case insensitive

c8c9366

Remove TODO comment

a1ca797

dsplaisted force-pushed the sdk-resolver-updates branch 2 times, most recently from 1b1c52c to 75b8f4f Compare June 5, 2020 00:07

dsplaisted and others added 3 commits June 5, 2020 11:03

Fix escaping for SDK Resolver results

86219c4

Apply code review feedback

9617293

Fix wording in XML doc

e804355

Co-authored-by: Rainer Sigwald <raines@microsoft.com>

dsplaisted force-pushed the sdk-resolver-updates branch from 75b8f4f to e804355 Compare June 5, 2020 18:04

rainersigwald approved these changes Jun 8, 2020

View reviewed changes

src/Build/Evaluation/Evaluator.cs Outdated Show resolved Hide resolved

src/Build.UnitTests/Evaluation/SdkResultEvaluation_Tests.cs Outdated Show resolved Hide resolved

src/Framework/Sdk/SdkResultFactory.cs Outdated Show resolved Hide resolved

Apply code review feedback

bce7be8

dsplaisted merged commit 392d4b6 into dotnet:vs16.7 Jun 10, 2020

rainersigwald mentioned this pull request Jun 10, 2020

[automated] Merge branch 'vs16.7' => 'master' #5417

Merged

dsplaisted mentioned this pull request Jun 11, 2020

[release/5.0.1xx-preview6] Update dependencies from microsoft/msbuild dotnet/sdk#12005

Merged

sfoslund mentioned this pull request Jun 15, 2020

[master] Update dependencies from microsoft/msbuild dotnet/sdk#12004

Merged

dsplaisted mentioned this pull request Jun 16, 2020

Fix API breaking change from adding SdkResolver IndicateSuccess overload #5435

Merged

KalleOlaviNiemitalo mentioned this pull request Aug 11, 2021

Unreliable equality comparisons in SdkResultItem #6750

Closed

rainersigwald mentioned this pull request Feb 25, 2025

Hide ephemeral ProjectRootElements from import enumerations #11474

Merged

Conversation

dsplaisted commented Apr 11, 2020

Uh oh!

wli3 commented Apr 16, 2020

Uh oh!

dsplaisted commented Apr 23, 2020

Uh oh!

dsplaisted commented Apr 23, 2020

Uh oh!

azure-pipelines bot commented Apr 23, 2020

Uh oh!

dsplaisted commented May 7, 2020

Uh oh!

rainersigwald left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

rainersigwald May 18, 2020

Choose a reason for hiding this comment

Uh oh!

dsplaisted May 20, 2020

Choose a reason for hiding this comment

Uh oh!

rainersigwald Jun 1, 2020

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Forgind left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

rainersigwald Jun 1, 2020

Choose a reason for hiding this comment

Uh oh!

dsplaisted commented Jun 5, 2020

Uh oh!

Uh oh!

Uh oh!

Uh oh!

rainersigwald commented Jun 10, 2020

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants