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

Basic .NET 4.5 -> 4.8 update #3749

Closed
wants to merge 1 commit into from
Closed

Conversation

unquietwiki
Copy link

Hey there. I wanted to update the program to .NET 6/7, to see if it resolved recent issues pulling metadata. I managed to clean up the build process to wherein the .NET 4.5 stuff is now .NET 4.8; anything .NET 5 should be unimpacted. The .NET project upgrade tool still struggles on this, but the program does work for me (albeit not as a single EXE, unless I picked the wrong folder to test).

No major code changes happened; I think it just adjusted some whitespace trying to re-implement references to different assemblies.

@unquietwiki
Copy link
Author

Now if this is accepted, I'd hit it with some of this tooling I've used in the past for re-factoring.

@HebaruSan HebaruSan added Enhancement Pull request Build Issues affecting the build system labels Dec 23, 2022
@HebaruSan
Copy link
Member

recent issues pulling metadata

Are you referring to the recent issue of #3707, fixed in #3708?

I don't think we want to break things for users who have .NET 4.5 but not .NET 4.8 installed unless there's a good reason.

@unquietwiki
Copy link
Author

unquietwiki commented Dec 23, 2022 via email

@HebaruSan
Copy link
Member

I don't think we want to break things for users who have .NET 4.5 but not .NET 4.8 installed unless there's a good reason.

I just wanted to reiterate this to make sure it's clear: If we merged this, then some number of users would find that CKAN no longer works from them, and we would have to handle the support load of triaging their bug reports and telling them how to upgrade to .NET 4.8. I realize that you probably spent a lot of time and effort on this, and I don't want there to be hurt feelings if and when we close this without merging it.

@unquietwiki
Copy link
Author

@HebaruSan That's fine. (ponders) If there's something else to inform your decision-making, 4.5 went EOL years ago, and 4.5.2 went EOL earlier this year. Any Win 7 deployment I've worked on before that went EOL was supporting 4.7 (or even 4.8) via a Windows Update. I know Kerbal uses a bastardized Mono runtime that's roughly 4.7; so if you want to accept that as a baseline; otherwise, my admin experience suggests you should be fine.

https://learn.microsoft.com/en-us/lifecycle/products/microsoft-net-framework

@unquietwiki
Copy link
Author

unquietwiki commented Dec 24, 2022

@HebaruSan Hey, the .NET 4.8 build successfully pulled metadata today; it's re-downloading many mods as I write this. I also did set up a Hurricane Electric IPv6 tunnel on my router last night; so that could be a part of it too (maybe v4 isn't working on an endpoint, but v6 is).

Edit: yeah, SpaceDock is dual-stack.
CKAN_Spacedock_IPv6_IPv4

@HebaruSan
Copy link
Member

That doesn't mean much to me, since I don't know what network problem you were having or whether you've done an apples-to-apples comparison with the master branch.

@unquietwiki
Copy link
Author

@HebaruSan Well, I already did that before the tunneling; the new build didn't pull the metadata same day as the old one. I made that tunnel change last night, and then it worked. I agree this is an edge case, but one that probably comes up for folks that may not realize its an issue (most websites you use will be maybe 100ms latency; this is almost 300).

@unquietwiki
Copy link
Author

@HebaruSan I also realize this may not be something you can immediately solve; but rather can suggest a possible workaround, until folks come up with a better solution. You can't do too much about IPv4 MTU issues with the app; that's VPN/network adapter turf.

@HebaruSan
Copy link
Member

The thing is that I don't know what we'd be working around. What problem are you trying to solve with this pull request?

@unquietwiki
Copy link
Author

@HebaruSan this PR was just for code modernization so you could eventually migrate to .NET 7, do cross-OS support, and resolve some of the other bugs via updated libraries. The network issue in #3751 is something that would have to be fixed via additional mirrors, CDNs, and offering workarounds.

@HebaruSan
Copy link
Member

OK, let's leave this as it is for now, then. We already have cross-OS support, and the other suggested benefits are entirely hypothetical as far as I can tell, as opposed to the real support load of breaking things for users who haven't upgraded to .NET 4.8.

@HebaruSan HebaruSan closed this Dec 24, 2022
@unquietwiki
Copy link
Author

@HebaruSan Acknowledged. At some point in the near future, I plan to do a deep dive on the .NET situation; you're not the only one that's had to worry about making such a change. I'll offer that up for your review when I do have that ready, so you can plan accordingly.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Build Issues affecting the build system Enhancement Pull request
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants