Added sub-dependencies of AspNetWebApi to package #6227
Conversation
This may fix dnnsoftware#6222 but I ran out of time to test, a clean install did work fine and that's all I had time to test for now. I think there are a couple other dlls in the issue we could handle here or elsewhere and we need to test upgrade scenarios from 9.13.4,5,6 to be sure.
|
@bdukes do you think this is the right way or we should have one package per dll? For context those DLLs are all pulled from sub-dependecies of bumping this one, so it looks to me like this way makes sense. |
|
And @iJungleboy does this also make sense to you, can you test the build artifact here and see if that works out https://dev.azure.com/dotnet/DNN/_build/results?buildId=111961&view=artifacts&pathAsName=false&type=publishedArtifacts |
|
@tvatavuk could you test please? |
|
Thank you very much, @valadas! 👍 This fix is a significant achievement, and we truly appreciate that it was possible to resolve this issue in this way. It's especially commendable that the solution includes aligning other dependencies as part of the library extension under DNN versioning, complete with proper bindings. Testing ResultsI performed extensive testing using the fixed build of DNN (DNN_Platform_9.13.6-pr6227-0013_Install.zip or DNN_Platform_9.13.6-pr6227-0013_Upgrade.zip), and the results were as expected. There were no observed issues with DNN itself, so my primary focus was on assessing 2sxc's behavior across various sequences of DNN installation and upgrades. Test Case 1
Test Case 2
Test Case 3
Test Case 4
Test Case 5
Summary of FindingsThe fix in PR #6227 ensures that future DNN installations or upgrades no longer depend on Request for Next StepsGiven that existing upgrade packages for |
| </assembly> | ||
| <assembly> | ||
| <path>bin</path> | ||
| <name>Newtonsoft.Json.dll</name> |
There was a problem hiding this comment.
Is it correct that we include this here, as it is already included in another package (
)I assume this is still ok as this is how third-party modules would behave, but I think this is the first time we have had this question come up.
There was a problem hiding this comment.
It won't hurt anything to include it twice, though I would recommend removing it since it does have its own package.
There was a problem hiding this comment.
I guess the right thing would be to add a dependency on the Newtonsoft package, rather than the DLL. But, if we're going to a quick fix here, what's in the PR doesn't break anything, so I'm fine leaving it as is.
There was a problem hiding this comment.
Hmm, good point on the package dependency, but if this package should try and install first, would it automatically reorder the package install order, anyone has experience with that scenario ? I would see a great benefit in using a package dependency as if the sub-dependency has sub-dependencies of its own, it solves that too.
This may fix #6222 but I ran out of time to test, a clean install did work fine and that's all I had time to test for now.
I think there are a couple other dlls in the issue we could handle here or elsewhere and we need to test upgrade scenarios from 9.13.4,5,6 to be sure.
Summary