zynaddsubfx / zyn-fusion-build

Build Scripts For Zyn-Fusion
Do What The F*ck You Want To Public License
122 stars 39 forks source link

Build system: Totally rewrite with Makefile (and documentations) #71

Closed AnClark closed 3 years ago

AnClark commented 3 years ago

NOTE: I may still do minor improvements (e.g. UI), and use git rebase to merge trivial adjustments into commit bddb8d4.

fundamental commented 3 years ago

Just FYI, this change is likely a good overall change. It's something that I have to carve out a larger block of time to review to verify all of the details. At a high level everything looked good last time I read through the change set.

fundamental commented 3 years ago

Goodness, working through the backlog can eat through time like crazy. Reviewing this in more detail I'm really liking the improvements. I'm still rubbish at non-trivial makefile features, so I plan on verifying the build by having a docker before and after build of the same target version (most likely the recent -rc1 release). Unless there's something I'm forgetting the builds should be 'reproducable'. I think the VERSION info has an associated date attached, but that can be modified to be the date the source was most recently modified.

Modifications I'm planning on doing:

No changes should be required on your end and I should hopefully hack away at the above points this week. (I'd be more than a bit much to demand any changes quickly given the length of time I've put off integration of this PR).

fundamental commented 3 years ago

Oh fun, liblo is erroring out in a new way. With the scripts and the docker setup the -Werror flag is somehow appearing in the compiler flag list. One semi-harmless warning is causing that phase of the build to fail. I'm hoping I have a workaround as I suspect this is just autotools being annoyingly broken with some package's cross compilation modes.

fundamental commented 3 years ago

liblo works, other deps migrated back to tarballs, I see the comment about mruby-process, and I think based upon some discussion on this repo it was resolved upstream. Of minor note, it doesn't look like PARALLEL is working correctly on my system, so I've wedged in a temporary -j20. I'll have to remember to clear that out before commiting anything. Right now it's on the patch stages which should be failing (I think), but the code isn't getting stuck there. Oddly enough it's failing at the linker stage for zynaddsubfx with some libz errors.

fundamental commented 3 years ago
   22 | #include "../Containers/MultiPseudoStack.h"
  +++ |+#include <thread>
   23 | using namespace std;

Existing header:

#include "test-suite.h"
#include <cmath>
#include <cstdlib>
#include <iostream>
#include <fstream>
#include <string>
#include <thread>
#include <rtosc/thread-link.h>
#include <unistd.h>
#include "../Containers/MultiPseudoStack.h"

Thanks mingw... Looks like that test is getting disabled on windows, though I swear that the other build scripts didn't encounter this exact issue.

fundamental commented 3 years ago

zynaddsubfx is compiling under mingw, looks like both the windows cross compile and native builds invoke setup_zest which isn't defined anywhere. Guess that got lost in some of the latter commits. 'wget' and 'pkg-config' appeared to be missing from deps, so those have been added.

fundamental commented 3 years ago

libuv is having 'fun' linker issues again, which I think is related to CC/LD/CCLD/... environmental flags. Autotools should work fine with those, but I recall having to use workarounds in the past...

fundamental commented 3 years ago

Confirmed to be an issue with environmental variables. Other docker builds masked the issue, but I'm trying to clear those variables just for the libuv build phase which IMO should (eventually) be moved up a layer to where other dependent libraries are getting built.

fundamental commented 3 years ago

windows build is working now (I think) and Linux build should work shortly, though some of the system dependencies are mis-specified. e.g. zlib is zlib1g for some dumb reason. Someone within debian/ubuntu packaging seems to have a habit of appending junk to some package names even when there's nothing to disambiguate. Either which way, my intent is to make sure that the linux build works on ubuntu, debian, arch, and alpine. Easy enough on docker, but this integration is taking a bit. I might break for a while and then resume hacking in the evening. It would be quite nice as this will clear my email inbox of zyn related TODOs.

fundamental commented 3 years ago

Rebuilding docker as arch linux containers no longer work even with the hacky workaround...

fundamental commented 3 years ago

Oh fun, updating docker and associated packages did not resolve the arch linux+docker bug.

fundamental commented 3 years ago

arch docker issues resolved and alpine docker seems to be roughly working as well. Still some additional work to do though to address the original points at the start of today.

fundamental commented 3 years ago

Today's goal is to get individual docker builders spitting out the compiled binaries. Then it's a matter of proposing the modifications I think?

fundamental commented 3 years ago

Looks like the .ttl files aren't in the right spots for the output tarball. Looking into that now.

fundamental commented 3 years ago

Copying a file out of the dockerized overlay on the build system seems to work, now to resolve tarball naming conventions.

fundamental commented 3 years ago

Something is up with the environmental variables, but it's pretty darn close now. An additional thing to check is if the folder within the tarball is named correctly. I think at the moment the tarred folder doesn't match the name of the tarball. It might be an issue with the prior scripts as well, but it should get fixed anyhow.

AnClark commented 3 years ago

OK, here I've also improved Install-deps.mk to follow mruby-zest-build's libuv path (76d3ba6). Now I can build your upstream's mruby-zest well on Windows (Msys2, not cross-compile).

fundamental commented 3 years ago

Thanks for the updates. Resuming where things were left off I can confirm that the folder in the tarball is a simplified name. 'zyn-fusion' rather than 'zyn-fusion-VERSION-MODE'. I think the build was already working for mingw, but I could be wrong. I'm optimistically thinking today this gets merged. Once it's merged there might be some followup discussion, but it's a lot easier to get through next steps once they're incremental changes.

fundamental commented 3 years ago

It doesn't look like github detected it, but the vast majority of this work was just merged into the master branch. There may be a few mistakes here and there with my adaption, but it appears to be working correctly.

Thanks a ton for the contribution :)

Side note - I do wish that the github UI didn't mark everything accepted and then closed in this fashion with a red "it doesn't look like you merged this even if you did" icon. Pretty misleading UI, but that's just a tangent.

AnClark commented 3 years ago

Thanks a ton for the contribution :)

My pleasure!


By the way, as you removed Arch Linux support, you may need to amend README.md as well as Makefile.linux.mk to remove any Arch Linux related contents.

fundamental commented 3 years ago

I'm fairly sure the arch specific packaging code was removed from the makefile, but thanks for the reminder since I forgot to update the readme.