AdamsLair / duality

a 2D Game Development Framework
https://adamslair.github.io/duality
MIT License
1.41k stars 290 forks source link

Move from the legacy nightlybuilder tool to more modern tooling #718

Closed Barsonax closed 4 years ago

Barsonax commented 5 years ago

Summary

Currently duality uses the legacy nightlybuilder. Consider moving towards more modern tooling. At work iam using NUKE build where you define your build in C# and can separate the build steps in targets that depend on each other. It offers full intelisense, debugging capabilities, allows you to install tools locally etc.

Example where I use it for my build: https://github.com/Barsonax/Singularity/blob/master/_build/Build.cs

Barsonax commented 5 years ago

@ilexp

ilexp commented 5 years ago

Some notes on the issue:

Barsonax commented 5 years ago

The dotnet pack command can be used to generate nupkg files. However iam using the new csproj format in my project so not sure if it works with the old format which duality still uses.

EDIT: If this turns out to be true then it might be a option to also migrate to the new cleaner format.

ilexp commented 5 years ago

Isn't dotnet part of the modern .NET Core / .NET Standard tooling? We need to check whether this produces the expected results in our case, especially since no .csproj is involved in packing here, only the .nuspecs.

Alternatively, I would assume that AppVeyor has nuget in their PATH - that could be a very straightforward replacement, if easily possible to execute in the way we need.

Barsonax commented 5 years ago

Yes it is. It uses msbuild internally (which we could also use). Don't know how it handles the old csproj format duality uses. It works for me but iam using packagereference.

On a side note I have been using nuke to build singularity. Currently putting the more advanced stuff such as code coverage in it as well. Really liking it as its just C# and the provided libraries are nice: https://github.com/Barsonax/Singularity/blob/master/_build/Build.cs

ilexp commented 5 years ago

More info on dotnet and msbuild command lines for .nuspec packing: https://docs.microsoft.com/en-us/nuget/reference/msbuild-targets#packing-using-a-nuspec

Don't see a variant that doesn't expect a .csproj, but that might be fine, would need to check this.

Barsonax commented 5 years ago

Apparently there is tooling to migrate to the new package reference format: https://docs.microsoft.com/en-us/nuget/reference/migrate-packages-config-to-package-reference#migration-steps

Not sure how good it handles the migration but if its easy to do and it works then why not.

ilexp commented 5 years ago

Assemblies in the lib root are ignored when the package is installed after the migration

This could be an issue unless also touching all affected dependencies. Needs to be done anyway, but would extend this issue with a bit more work - so if there is a way to use newer / non-custom tooling without the conversion as a first step, that might be worth considering as well.

Related issue: #696

ilexp commented 5 years ago

Issue #696 has been resolved, all Duality packages and dependencies now use framework folders.

Barsonax commented 5 years ago

So with that out of the way we could now try the migration and see if this is a easy route to adopt the new cleaner csproj format (with packagereference so we can easily make packages). Might have time tomorrow to give it a shot.

Barsonax commented 5 years ago

So I tried out the migration in #720 but ran into a issue where it did not copy some references to the output folder.

ilexp commented 5 years ago

Great, I'll take a look at the PR.

Barsonax commented 5 years ago

Iam thinking about porting the nightlybuild to NUKE build. Been using that at work and so far quite like it. Some features:

Links: Site: https://www.nuke.build/ Example build I use in singularity: https://github.com/Barsonax/Singularity/blob/master/_build/Build.cs

ilexp commented 5 years ago

Sounds interesting, but introducing an entire build system sounds like quite a big overhead, and I'm not convinced there would be a reduction in complexity that would offset this enough for a net win. Could keep it in mind for later though, always good to know some options 👍

Barsonax commented 5 years ago

Isn't the current nightly basically a build system?

In the end we have to define the build somewhere. I rather write the build in NUKE than in powershell. Much more robust and easier to extend later on (with for instance a doc build or code coverage)

ilexp commented 5 years ago

Isn't the current nightly basically a build system?

It's a (C#) script, but not a system, and one that we own completely. It's not a third party dependency that can update, refactor, deprecate, or itself depend on even more stuff, and it doesn't require learning a DSL to do something with it.

I think ideally the outcome of this issue would be either the removal of the builder without replacement if simple enough to fit into a batch script or AppVeyor build step, or a simplification of the builder itself to the point where all non-required functionality is gone and the remaining is easier to maintain. Both cases take things away, as a cleanup of sorts. Introducing nuke would go into the opposite direction by replacing what we have with an even more complex system.

Not arguing nuke is bad, but I don't see we need it at this point.

Barsonax commented 5 years ago

Hmm if its that simple a powershell script might be enough.

I would not use a appveyor step to pack/publish the packages though. This makes it impossible to run it locally. Pretty sure it can be done with a few commands.

ilexp commented 5 years ago

Good point. Let's try to keep something that can run locally.

Hmm if its that simple a powershell script might be enough.

Could be an option, but should first find out how simple we can actually get right now. Maybe a good first step would be to actually remove all unused functionality from the builder tool, see what remains. Running NUnit Tests for example is completely superseded by AppVeyor on the CI side and by VS integration on the dev side. Not 100% sure what else is in there right now, but some of it is pretty old.

Best case: A short powershell script can do the job. Worst case: We have a cleaned up builder tool / script.

Barsonax commented 5 years ago

Good point. Let's try to keep something that can run locally.

Yup and as a added bonus if for whatever reason you want to switch to some other CI later on since most of your build is now CI agnostic this will be a easy process.

Could be an option, but should first find out how simple we can actually get right now. Maybe a good first step would be to actually remove all unused functionality from the builder tool, see what remains. Running NUnit Tests for example is completely superseded by AppVeyor on the CI side and by VS integration on the dev side. Not 100% sure what else is in there right now, but some of it is pretty old.

Pretty sure you can run the unit tests from a powershell script as well. Atleast with Xunit its easy to do.

Also running the tests using appveyor functionality will make the build tightly coupled to Appveyor and make it impossible to run it locally. Ideally you want to be able to run the build locally by simply firing off a command. This makes it way more maintainable because you get a very fast iteration cycle. This is how I set it up with Singularity (though currently its using NUKE but in the past it was just powershell).

Barsonax commented 4 years ago

closing this, see https://github.com/AdamsLair/duality/issues/810