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

Cleanup project, update builds, fixes #3108

Merged
merged 1 commit into from
Jul 12, 2020

Conversation

Olympic1
Copy link
Member

@Olympic1 Olympic1 commented Jul 10, 2020

Problem

I wanted to prepare CKAN for .NET 5.0, and some other stuff I'm working on, but I coudn't open the project in VS.

Cause

Not sure what exactly, but I traced it back to CKAN.Core. Something was probably malformed in the .csproj file. I let VS recreate the file and now I can open it again.

Changes

  • Enabled deterministic builds (better security)
  • Upgraded Cake to 0.38.4
  • Upgraded NuGet to 5.6.0
  • Upgraded Cake tools and addins
  • Added vars for build versions (now we only change 2 values instead of 15)
  • Fixed .csproj files
  • Removed bootstrap packages from GUI and consoleUI (we don't use an installer)
  • Fixed error with ILRepack @HebaruSan (Issue) - (Culprit)
  • Migrated NuGet packages from packages.config to PackageReference (More info)
  • Fixed links for reporting issues
  • Removed duplicate images from zh-CN.resx file (400 kB saved)

NOTES:

  • The PackageReferences will create obj folders outside _build but currently you can't change their output path.
  • I upgraded netkan.exe to use CommandLineParser v2.8.0 but the CmdLine still uses the old one. I'm currently updating it but have to rewrite some stuff.

@HebaruSan
Copy link
Member

Huh, this builds fine in a mono:latest (6.8.0) image.

$ docker pull mono:latest
$ docker run -it --rm --entrypoint bash mono:latest
# apt update && apt install git
# git clone https://github.com/KSP-CKAN/CKAN.git
# cd CKAN/
# git remote add Olympic1 https://github.com/Olympic1/CKAN.git
# git fetch Olympic1
# git checkout -b update/builds Olympic1/update/builds
# ./build

No idea why the automated builds say they're missing reference assemblies.

@HebaruSan
Copy link
Member

Something in this created a bunch of obj folders outside of the _build directory:

$ ls -d */obj
Cmdline/obj  ConsoleUI/obj  Core/obj  GUI/obj  Netkan/obj  Tests/obj

This broke the compile when I switched to another branch. Should get those back into _build.

@Olympic1
Copy link
Member Author

Olympic1 commented Jul 10, 2020

No idea why the automated builds say they're missing reference assemblies.

.NET v4.8 is only supported since mono 6.6.0. .NET v4.7.2 is supported since mono 6.0.0
We need to keep using v4.5 to support mono 5.20 because their cross-platform support still doesn't work.

Something in this created a bunch of obj folders outside of the _build directory:

Those are the NuGet PackageReferences, I was looking into getting that into the build folder but that isn't possible atm

@Olympic1 Olympic1 force-pushed the update/builds branch 9 times, most recently from 020eec3 to b8e997b Compare July 10, 2020 14:37
@Olympic1 Olympic1 added Build Issues affecting the build system Cake Issues affecting Cake Infrastructure Issues affecting everything around CKAN (the GitHub repos, build process, CI, ...) Pull request labels Jul 10, 2020
Netkan/Program.cs Outdated Show resolved Hide resolved
Netkan/Program.cs Outdated Show resolved Hide resolved
Netkan/CmdLineOptions.cs Outdated Show resolved Hide resolved
Copy link
Member

@HebaruSan HebaruSan left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The copyright symbol thing isn't a big deal, but we can't make netkan error out without a file parameter.

Copy link
Member

@HebaruSan HebaruSan left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks!

@DasSkelett
Copy link
Member

Can you squash these commits before merge? Rebase commits à la "Merge branch 'master' into ..." are always a bit confusing in the commit history.

Copy link
Member

@DasSkelett DasSkelett left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Let's hope Mono gets its stuff together and restores cross-platform compatibility soon. Would be nice to update the rest too.
Anyways, thanks a lot for that!

@HebaruSan HebaruSan merged commit 5912af2 into KSP-CKAN:master Jul 12, 2020
@Olympic1 Olympic1 deleted the update/builds branch July 12, 2020 17:34
HebaruSan added a commit to HebaruSan/CKAN that referenced this pull request Jul 13, 2020
This reverts commit 5912af2, reversing
changes made to 7c6310b.
HebaruSan added a commit that referenced this pull request Jul 14, 2020
This reverts commit 5912af2, reversing
changes made to 7c6310b.
@HebaruSan HebaruSan mentioned this pull request Jul 14, 2020
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 Cake Issues affecting Cake Infrastructure Issues affecting everything around CKAN (the GitHub repos, build process, CI, ...)
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants