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

Updating the native build process for Windows. #91

Merged
merged 5 commits into from
Dec 28, 2021

Conversation

Gopall
Copy link
Contributor

@Gopall Gopall commented Nov 20, 2021

Hello. This pull request contains several fixes that are needed in order for native building on Windows to work.

I saw this issue ( #86 ) late yesterday, so I decided to take a look at it today. I was trying to fix that issue but once I fixed it I found a few other build errors. Some of the errors, like the first, happened because some necessary changes weren't applied to "Makefile.mingw64.mk", the native makefile. Some of these changes were applied in Makefile.windows.mk, but had not been applied yet here.

The changes needed weren't that big, but the issues were probably unrelated to eachother. So, I'm going to explain what I changed for clarity (and documentation I guess):

I'm fairly certain that warning message in that issue, was there when I last built zyn-fusion. But, I believe that the reason that it prevented compilation this time is because there was a compilation flag set to treat compilation warnings as errors. To fix this, in Makefile.mingw64.mk I changed a line from
./autogen.sh --prefix=$(PREFIX_PATH) --disable-shared --enable-static to ./configure --prefix=$(PREFIX_PATH) --disable-shared --enable-static. Autogen.sh generates some build & config files for liblo, and I think those were causing the issue. Fortunately the liblo download already comes with pre-generated build files that work fine.

This was the second error:
patch -N < /c/users/gopal/Desktop/zyn-fusion-build-master/mruby-dir-glob-no-process.patch /bin/sh: line 3: /c/users/gopal/Desktop/zyn-fusion-build-master/mruby-dir-glob-no-process.patch: No such file or directory make: *** [Makefile.mingw64.mk:95: apply_mruby_patches] Error 1
The patch file that it talks about is obsolete and was removed. So I removed that section from the makefile. I actually previously fixed this error in the previous obsolete build system.

This is the third error:
make -C /c/users/gopal/Desktop/zyn-fusion-build-master/src/mruby-zest-build --always-make builddepwin make[1]: Entering directory '/c/users/gopal/Desktop/zyn-fusion-build-master/src/mruby-zest-build' make[1]: *** No rule to make target 'builddepwin'. Stop. make[1]: Leaving directory '/c/users/gopal/Desktop/zyn-fusion-build-master/src/mruby-zest-build' make: *** [Makefile.mingw64.mk:110: setup_zest] Error 2
To fix this I removed the builddepwin target, which had been removed in other makefiles as well.

The fourth error was caused because of a namespace conflict between libuv (which is a library used by mruby-zest) and the mingw environment. I found these issue and pull requests in the github pages of those projects, and used them to solve the issue: libuv/libuv#3345 & msys2/MINGW-packages#9946
This has been fixed in the latest version of libuv, but mruby-zest is using an older version. I'm not sure if updating libuv would break mruby-zest, and it would be more complex. So, in the meantime, I created a patch from that commit, and added a portion to the makefile which would apply the patch.

The 5th error is that the $(ZYN_FUSION_OUT) variable was missing from the mingw makefile. The 6th error is that in the packaging and zipping part of the build proccess a file path wasn't updated. I think I copied both solutions from the other makefile.

TL;DR:
Ported some fixes from the crosscompilation makefile to the native makefile, and added a patch for libuv.

This patch fixes a conflict between the mingw environment and libuv.
There are variety of fixes I had to apply.
@Gopall
Copy link
Contributor Author

Gopall commented Nov 20, 2021

I built this on Windows 10 with the msys2 mingw64 shell. The program seemed to run fine.

Makefile.mingw64.mk Outdated Show resolved Hide resolved
Makefile.mingw64.mk Outdated Show resolved Hide resolved
cd $(ZEST_PATH) ; \
ruby rebuild-fcache.rb
# cd $(ZEST_PATH) ; \
# ruby rebuild-fcache.rb
Copy link
Member

Choose a reason for hiding this comment

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

I'm pretty sure that make windows in zest doesn't build the cache file, so I think this is still a required step.

Copy link
Contributor Author

@Gopall Gopall Dec 8, 2021

Choose a reason for hiding this comment

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

I tested it again without running the "ruby rebuild-fcache.rb", and it compiled fine.

I then tested it adding it back in and it compiled fine as well.

Makefile.mingw64.mk Outdated Show resolved Hide resolved
Makefile.mingw64.mk Outdated Show resolved Hide resolved
@fundamental
Copy link
Member

As per the libuv version there shouldn't be any version specific dependence, it was just the current version available when some of the initial coding was being done. AFAIK all of the API use is pretty basic and nothing should have been deprecated or behavior altered.

@Gopall
Copy link
Contributor Author

Gopall commented Dec 8, 2021

I might try compiling mruby-zest with a newer version of libuv, to check if it still works. In the meantime, should I keep the libuv patch I added?

@fundamental
Copy link
Member

Ideally we'd upgrade libuv, though I'd be fine with merging the current patch set leaving the libuv update for another patch. Opinions? IIRC this patch set was ready to merge before I hit a bunch of end-of-year crunch time which blocked most zyn PR reviews.

@Gopall
Copy link
Contributor Author

Gopall commented Dec 27, 2021

I'd say my opinion is that we should merge this patch for now and leave the libuv update for another patch.

@fundamental
Copy link
Member

That works for me. Thanks for the enhancement 👍

@fundamental fundamental merged commit d48bbcb into zynaddsubfx:master Dec 28, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants