Builds on .NET 4.7.2 #3756
Builds on .NET 4.7.2 #3756
Conversation
|
Looking at the code with SonarLint, ModuleInstaller in Tests has a missing assert that matches the failed unpack test; so maybe that test was a false result? |
unquietwiki
force-pushed
the
net472
branch
2 times, most recently
from
8b3fa27
to
81fa5d4
Compare
|
ilrepack_successful_command.txt Okay, I dug into where ILRepack was having problems. This sequence works for me locally. |
|
Looks like this is the current build error. |
|
@HebaruSan taking a look, thanks for the heads up. I told it to pull in all the new commits, so I'll see how that impacts things, and will report back. |
|
@HebaruSan I found the missing NuGet include, and my latest build does run for me. Sneaky because it was building in VS just fine, though still missing that dependency. |
|
Now it says this: FYI, you can always see the latest errors on the Checks tab, or by clicking Details in the box at the bottom of the PR page. |
|
@HebaruSan I looked at the include again: the newer version of SharpZipLib was asking for .NET Standard 2.0; which from what I know in the Unity/Kerbal space, there's a choice between .NET 4.x and Standard 2.x for builds, and this build setup is meant for the former. It was probably working just fine for VS builds, but not for Mono builds. I've reverted the version of the library to try to avoid that dependency. |
|
@HebaruSan Okay, that's a better outcome. There's some Docker timeout on two of the builds, and the remaining two seem related to whatever you were using .Net 5 for (I don't fully understand how you were able to make that work; it seems only Core & Tests use it). The rest worked! |
|
Cool, yeah, that's progress. 🎉 We do need all the tests to pass before merging, though, and I don't have the spare cognitive bandwidth to dig into this. Maybe try reverting parts till you find out what breaks it? |
|
@HebaruSan I'll see what I can figure out. 👍 If you can tell me what you were trying to do with .NET 5, as you can remember, that'd be a good help. One other thing regarding that: .NET 6 is the current LTS, so whatever you're doing with 5 should probably move to that. |
|
I think you can find most of it in the git log for the core csproj. Originally @jbrot submitted .NET Standard 2.0 support in #2820 with plans to create a new Mac GUI that never materialized (unless he did it and just never told us), then later @DasSkelett updated that to .NET 5 in #3194 in anticipation of a future MAUI GUI, before Microsoft announced that would leave Linux and Mac users out in the cold. So it's not in use, but it creates opportunities to migrate to more modern platforms. |
|
Thanks for that; that's helpful! When I first started looking at this, I was trying to follow some of what's suggested by Microsoft, regarding migration to .NET 5-7. I can try to figure out how to at least fix the build issue; otherwise, a separate branch using the other migration tools might yield the desired upgrade. |
|
@HebaruSan I noticed in the logic there were calls to two different file managers, depending on .NET5 vs 4.x. I took care of trying to fix that reference, tried calling on .NET6, and disabled a 4.5-specific Hashset method (LINQ seems to provide it). Still don't have an easy way to test on my end: VS doesn't like it that most of the other projects don't target the newer framework, even with added references to try that. |
I think you can run this from the command line: |
|
And yes,
|
|
@HebaruSan that helped, thanks. I merged the last few commits that were coming up to the .NET5 build issue; and also gave up on using .NET6, since the builder on here is still on .NET5 (and the local compiler warned me plenty .NET5 is EOL). Regarding the incompatibility warnings I keep seeing... the messages I saw involved two of the libraries being potentially incompatible with .NET 5. One's CommandLineParser: I tried creating separate references between the current one and a .NET Standard version for .NET 5 to use; that just confused VS; going to version 2.x on that library requires a partial re-write of the code here. The other was iniparser: having the .NET Standard version alongside isn't throwing any immediate errors. Edit: what do you know, Mono did hate that extra iniparser reference. Removed! |
HebaruSan
added
GUI
HebaruSan
added
Pull request
Build
|
@unquietwiki, if you come back to this, please let us know and we can re-open it. Closing to reduce inactive stuff accumulating in the queue. |
|
@HebaruSan hey there, checking in; sorry for the delay. I'd like to finish this, but I guess I need some help from someone that worked on the original .NET 5 hooks & if they're even needed anymore. I also feel like since .NET 5 is EOL, the tests should be updated to .NET 6 LTS; that's a call you'd have to make. Let me know your thoughts. Thanks. |
|
I don't think you're going to get that help; as sometimes happens, you're the one that's interested in this, so whatever you can figure out is what will get done. Good luck! |
|
Thanks @HebaruSan. BTW, somewhat related to this: I saw a PR #3131 that updates CommandLineParser. That probably needs some love to ensure it still works, but that is a dependency issue I ran into on my PR. |
#3755 was a dead-end: between the LINQ IEnumerable calls, and the changes in SSL/TLS support in 4.7.2, re-targeting to 4.7.2 solves the following issues...
Tests passed: 852 of 860; one of failures was an unpack test, but I could still use the app successfully to uninstall & install mods. The previous 4.5-oriented build didn't even get halfway through the tests.
I hope this balances the compatibility concerns for the user base and the ability to develop further on the software.