Sort by Project References to avoid project ordering dependency issues. #1109

arikalish · 2021-05-12T20:17:33Z

✨ What kind of change does this PR introduce? (Bug fix, feature, docs update...)

Bug fix

⤵️ What is the current behavior?

Packages to update are sorted by the packages they reference but ignores project dependency ordering, leading to issues when projects at the root of the dependency graph are updated before those that depend on them that are more than one level in the tree apart from one another. In particular, this leads to ERROR NU1605: https://docs.microsoft.com/en-us/nuget/reference/errors-and-warnings/nu1605

🆕 What is the new behavior (if this is a feature change)?

Sorts by project reference before sorting by package dependencies.

💥 Does this PR introduce a breaking change?

Possibly, but this was tested on a fairly large solution (175+ projects) with a very complex dependency graph and it did not cause any issues.

🐛 Recommendations for testing

Project dependency graph like this:

A -> B -> C -> D

Where -> means "references".

A and D reference the same package. Update the version referenced by D. I had some trouble replicating this in a simple solution, but was able to replicate it consistently with our large solution.

📝 Links to relevant issues/docs

Similar to: #988

🤔 Checklist before submitting

All projects build
Relevant documentation was updated

stale · 2021-08-11T00:16:47Z

This issue has been automatically marked as stale because it has not had recent activity. It will be closed if no further activity occurs. Thank you for your contributions.

arikalish · 2021-08-16T01:09:09Z

Still relevant.

promontis · 2021-09-20T13:44:26Z

@arikalish this seems like good code to resolve NU1605 errors. Do you know why it is not building?

arikalish · 2021-09-20T14:13:03Z

@promontis Unfortunately not. A lot of PR builds from the time I submitted this PR (and some others) failed for mysterious reasons. Unfortunately, I think it would require a maintainer to resolve the issues as they seem to be with the build pipeline.

The tests that fail are kind of strange, honestly.

promontis · 2021-09-20T14:23:18Z

@krissetto @CrispyDrone @skolima @rajbos any help is appreciated 🙏

skolima · 2021-09-20T15:09:10Z

I've generated some automated updates... and yes, the pipeline is failing. I'll see if I can work out why.

msallin · 2021-09-20T15:14:50Z

microsoft/azure-pipelines-agent#3501

2021-09-20T15:07:27.1692150Z Could not load file or assembly 'Microsoft.VisualStudio.Services.Agent, Version=2.192.0.0, Culture=neutral, PublicKeyToken=null'. The system cannot find the file specified.
2021-09-20T15:07:27.1692473Z 
2021-09-20T15:07:27.1692787Z Could not load file or assembly 'Microsoft.VisualStudio.Services.Agent, Version=2.192.0.0, Culture=neutral, PublicKeyToken=null'. The system cannot find the file specified.
2021-09-20T15:07:27.1693354Z 
2021-09-20T15:07:27.1693680Z Could not load file or assembly 'Microsoft.VisualStudio.Services.Agent, Version=2.192.0.0, Culture=neutral, PublicKeyToken=null'. The system cannot find the file specified.
2021-09-20T15:07:27.1693987Z 
2021-09-20T15:07:27.1694192Z    at System.Reflection.RuntimeModule.GetTypes(RuntimeModule module)
2021-09-20T15:07:27.1694443Z    at System.Reflection.RuntimeModule.GetTypes()
2021-09-20T15:07:27.1694660Z    at System.Reflection.Assembly.GetTypes()
2021-09-20T15:07:27.1694983Z    at Agent.Plugins.Log.TestResultParser.Plugin.ParserFactory.<>c.<GetTestResultParsers>b__0_0(Assembly x)
2021-09-20T15:07:27.1695433Z    at System.Linq.Enumerable.SelectManySingleSelectorIterator`2.MoveNext()
2021-09-20T15:07:27.1695719Z    at System.Linq.Enumerable.WhereSelectEnumerableIterator`2.ToList()
2021-09-20T15:07:27.1695994Z    at System.Linq.Enumerable.ToList[TSource](IEnumerable`1 source)
2021-09-20T15:07:27.1696498Z    at Agent.Plugins.Log.TestResultParser.Plugin.LogParserGateway.<>c__DisplayClass0_0.<InitializeAsync>b__0()
2021-09-20T15:07:27.1696791Z    at System.Threading.Tasks.Task.InnerInvoke()
2021-09-20T15:07:27.1697032Z    at System.Threading.Tasks.Task.<>c.<.cctor>b__274_0(Object obj)
2021-09-20T15:07:27.1697449Z    at System.Threading.ExecutionContext.RunFromThreadPoolDispatchLoop(Thread threadPoolThread, ExecutionContext executionContext, ContextCallback callback, Object state)
2021-09-20T15:07:27.1697864Z --- End of stack trace from previous location where exception was thrown ---
2021-09-20T15:07:27.1698273Z    at System.Threading.ExecutionContext.RunFromThreadPoolDispatchLoop(Thread threadPoolThread, ExecutionContext executionContext, ContextCallback callback, Object state)
2021-09-20T15:07:27.1698742Z    at System.Threading.Tasks.Task.ExecuteWithThreadLocal(Task& currentTaskSlot, Thread threadPoolThread)
2021-09-20T15:07:27.1699068Z --- End of stack trace from previous location where exception was thrown ---
2021-09-20T15:07:27.1699912Z    at Agent.Plugins.Log.TestResultParser.Plugin.LogParserGateway.InitializeAsync(IClientFactory clientFactory, IPipelineConfig pipelineConfig, ITraceLogger traceLogger, ITelemetryDataCollector telemetry)
2021-09-20T15:07:27.1700445Z    at Agent.Plugins.Log.TestResultParser.Plugin.TestResultLogPlugin.InitializeAsync(IAgentLogPluginContext context)
2021-09-20T15:07:27.1701156Z System.IO.FileNotFoundException: Could not load file or assembly 'Microsoft.VisualStudio.Services.Agent, Version=2.192.0.0, Culture=neutral, PublicKeyToken=null'. The system cannot find the file specified.
2021-09-20T15:07:27.1701547Z 
2021-09-20T15:07:27.1701790Z File name: 'Microsoft.VisualStudio.Services.Agent, Version=2.192.0.0, Culture=neutral, PublicKeyToken=null'
2021-09-20T15:07:27.1702029Z 
2021-09-20T15:07:27.1702129Z 
2021-09-20T15:07:27.1703247Z System.IO.FileNotFoundException: Could not load file or assembly 'Microsoft.VisualStudio.Services.Agent, Version=2.192.0.0, Culture=neutral, PublicKeyToken=null'. The system cannot find the file specified.
2021-09-20T15:07:27.1703691Z 
2021-09-20T15:07:27.1703967Z File name: 'Microsoft.VisualStudio.Services.Agent, Version=2.192.0.0, Culture=neutral, PublicKeyToken=null'
2021-09-20T15:07:27.1704254Z 
2021-09-20T15:07:27.1704345Z 
2021-09-20T15:07:27.1704777Z System.IO.FileNotFoundException: Could not load file or assembly 'Microsoft.VisualStudio.Services.Agent, Version=2.192.0.0, Culture=neutral, PublicKeyToken=null'. The system cannot find the file specified.
2021-09-20T15:07:27.1705215Z 
2021-09-20T15:07:27.1705673Z File name: 'Microsoft.VisualStudio.Services.Agent, Version=2.192.0.0, Culture=neutral, PublicKeyToken=null'

skolima · 2021-09-20T15:25:26Z

@arikalish this looks IMO like it was a test failure and was solved by #1113 - could you please update from main branch again?

arikalish · 2021-09-20T15:36:53Z

@skolima sure. I'll update all my PRs today!

…nces

promontis · 2021-09-20T16:03:27Z

Cool! All is green 👍 Can this be merged @skolima?

msallin · 2021-09-20T17:56:24Z

I like this. However, there was kind of a "feature". Nukeeper usually failed when some references were unnecessary, due to this matter.

Do you think its useful to have an option to opt-in, to check for unnecessary package refs?

skolima · 2021-09-20T18:02:31Z

I'd like to dig into this a bit more before going "OK". @msallin I'm not fond of "accidental features" like this - there's dedicated tools to trim the dependencies tree.

msallin · 2021-09-20T18:04:18Z

I'd like to dig into this a bit more before going "OK". @msallin I'm not fond of "accidental features" like this - there's dedicated tools to trim the dependencies tree.

Yes. I agree. It should be accidental.
I wasn't aware of such a tool. Can you point to one?

skolima · 2021-09-20T18:08:25Z

I use ReSharper for this, but seems that VS also has this built-in now https://stackoverflow.com/a/67715612/3205 . I've also used dotnet-outdated for this purpose before https://github.com/dotnet-outdated/dotnet-outdated#reporting-on-transitive-dependencies

arikalish · 2021-09-20T18:16:14Z

@skolima FWIW: We've added some tests to our solution that pinpoint unused project references using Roslyn but they haven't solved this issue. Also, for folks (perhaps unwisely) using reflection it can be difficult to detect unused references correctly.

IIRC dotnet-outdated calls MSBuild to generate a dependency graph. That's probably the most-correct thing to do but would require significant changes to NuKeeper.

slang25 · 2021-09-20T20:58:40Z

Yes the way dotnet-outdated does it really nice: https://github.com/dotnet-outdated/dotnet-outdated/blob/846207a9f656f8504f13a4fc51abacb85796c072/src/DotNetOutdated.Core/Services/DependencyGraphService.cs#L28

It shells out to msbuild to create the project.assets.json file, and deserializes that into a model it can work with.

This could also be done in memory using msbuild directly, and picking the appropriate SDK owned msbuild using MSBuildLocator (I've been doing this with a bit of success in other metaprogramming projects).

There is a fair bit of logic that could be simplified in NuKeeper by using MSBuild directly, like evaluating project graphs and and getting PackageReferences "from the horses mouth", with metadata taking us to the source lines they were defined.

skolima · 2021-09-21T09:58:08Z

@slang25 the catch with this approach - and the reason we avoided it before - is that it doesn't work with .NET Framework Classic. And at least when we looked at this (khem, khem, 2017 I think) it would have been much more complicated, if not completely unsupported.

It might be time to revisit that approach, but keeping in mind that a significant feature of NuKeeper over other tools is that it works with the Classic Framework.

arikalish · 2021-09-21T12:46:32Z

@skolima dotnet-outdated works on our solution. We're targeting Framework but have updated all of our projects (including a legacy Web project) to the modern csproj format. NuKeeper's support for BitBucket is the big differentiator.

promontis · 2021-09-29T08:48:52Z

@skolima how is the code looking so far? Would be awesome if this can be merged 🎉

promontis · 2021-11-18T09:45:15Z

@skolima have you had time to look into the code? Hopefully, this can be merged soon.

Sort by Project References to avoid project ordering dependency issues.

ded54ad

stale bot added the wontfix label Aug 11, 2021

stale bot removed the wontfix label Aug 16, 2021

Merge remote-tracking branch 'origin/master' into SortByProjectRefere…

a3c63be

…nces

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Sort by Project References to avoid project ordering dependency issues. #1109

Sort by Project References to avoid project ordering dependency issues. #1109

arikalish commented May 12, 2021 •

edited

Loading

stale bot commented Aug 11, 2021

arikalish commented Aug 16, 2021

promontis commented Sep 20, 2021

arikalish commented Sep 20, 2021 •

edited

Loading

promontis commented Sep 20, 2021

skolima commented Sep 20, 2021

msallin commented Sep 20, 2021 •

edited

Loading

skolima commented Sep 20, 2021

arikalish commented Sep 20, 2021

promontis commented Sep 20, 2021

msallin commented Sep 20, 2021

skolima commented Sep 20, 2021

msallin commented Sep 20, 2021

skolima commented Sep 20, 2021

arikalish commented Sep 20, 2021

slang25 commented Sep 20, 2021

skolima commented Sep 21, 2021 •

edited

Loading

arikalish commented Sep 21, 2021

promontis commented Sep 29, 2021

promontis commented Nov 18, 2021

Sort by Project References to avoid project ordering dependency issues. #1109

Are you sure you want to change the base?

Sort by Project References to avoid project ordering dependency issues. #1109

Conversation

arikalish commented May 12, 2021 • edited Loading

✨ What kind of change does this PR introduce? (Bug fix, feature, docs update...)

⤵️ What is the current behavior?

🆕 What is the new behavior (if this is a feature change)?

💥 Does this PR introduce a breaking change?

🐛 Recommendations for testing

📝 Links to relevant issues/docs

🤔 Checklist before submitting

stale bot commented Aug 11, 2021

arikalish commented Aug 16, 2021

promontis commented Sep 20, 2021

arikalish commented Sep 20, 2021 • edited Loading

promontis commented Sep 20, 2021

skolima commented Sep 20, 2021

msallin commented Sep 20, 2021 • edited Loading

skolima commented Sep 20, 2021

arikalish commented Sep 20, 2021

promontis commented Sep 20, 2021

msallin commented Sep 20, 2021

skolima commented Sep 20, 2021

msallin commented Sep 20, 2021

skolima commented Sep 20, 2021

arikalish commented Sep 20, 2021

slang25 commented Sep 20, 2021

skolima commented Sep 21, 2021 • edited Loading

arikalish commented Sep 21, 2021

promontis commented Sep 29, 2021

promontis commented Nov 18, 2021

arikalish commented May 12, 2021 •

edited

Loading

arikalish commented Sep 20, 2021 •

edited

Loading

msallin commented Sep 20, 2021 •

edited

Loading

skolima commented Sep 21, 2021 •

edited

Loading