Skip to content
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 invalid binding redirects when upgrading to DNN 9.4.0 #2980

Merged
merged 5 commits into from Sep 18, 2019

Conversation

@bdukes
Copy link
Contributor

bdukes commented Sep 6, 2019

Upon upgrading, this PR will find invalid binding redirects (i.e. <dependentAssembly xmlns="">), and remove them from the web.config (since they weren't doing anything anyway). If a valid dependentAssembly element also exists, then that's all that's done, but if it doesn't, and new dependentAssembly is added with a binding redirect for the version in the bin folder.

I'm still in process of testing this, but thought I'd push it as a PR in case someone wants to pick it up and run with it this weekend.

Not sure if we want to try to get this in 9.4.0 or hold for 9.4.1, but this'll need adjustments if we hold it.

Fixes #2971.

Copy link
Contributor

mitchelsellers left a comment

If we include this, do we need the other PR merged as well to ensure that we don't break something?

@bdukes

This comment has been minimized.

Copy link
Contributor Author

bdukes commented Sep 9, 2019

No, the bug that caused an issue in the 8.0.0 upgrade was using collision="overwrite", and that bug has already been fixed, the other fix is for collision="save".

For #2971
@bdukes bdukes force-pushed the feature/upgrade-xmlns-9.4.0 branch from 336c202 to 7f6090b Sep 9, 2019
@bdukes

This comment has been minimized.

Copy link
Contributor Author

bdukes commented Sep 9, 2019

I've run through an initial test, (force) pushed a couple missing changes, it seems to work. That said, if we're pulling this in for 9.4.0, we're going to need to grab a wider test bed of sites to upgrade.

@valadas

This comment has been minimized.

Copy link
Contributor

valadas commented Sep 10, 2019

Since this is not a regression in 9.4.0, I am assigning it to 9.4.1 for now, let me know if you want it different.

@valadas valadas added the Type: Bug label Sep 10, 2019
@valadas valadas added this to the 9.4.1 milestone Sep 10, 2019
Copy link
Contributor

mitchelsellers left a comment

This solution looks great to me

Copy link
Contributor

valadas left a comment

Looks good to me

mitchelsellers and others added 2 commits Sep 12, 2019
... since it did not get merged in 9.4.0
@valadas valadas dismissed stale reviews from mitchelsellers and themself via f542e4f Sep 17, 2019
@donker
donker approved these changes Sep 17, 2019
Copy link
Contributor

donker left a comment

Looks good to me

@valadas valadas merged commit 067c3cd into release/9.4.x Sep 18, 2019
2 checks passed
2 checks passed
Dnn.Platform [CI] Build #9.4.1-pr2980.16.21509 succeeded
Details
license/cla All CLA requirements met.
Details
@valadas valadas deleted the feature/upgrade-xmlns-9.4.0 branch Dec 31, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
5 participants
You can’t perform that action at this time.