-
-
Notifications
You must be signed in to change notification settings - Fork 1k
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
Fixes build errors under mingw (3113) #3369
Conversation
built from source
it was the project lead
cross tools override the native tools
at that version for make package
@dragoneagle I'm intrigued you're using these msys2 helpers for mingw. Is this on a Linux machine or Windows? They were written as helpers for the Windows build environments, but we don't Travis-CI test Windows, so the only way to know the impact of these on Windows is to run the painstaking task of setting up a Windows build environment which I personally won't have time for for a while. |
This is on a windows machine. I meant msys. sorry. I know your pain. I've personally set up a windows build system about 15 times just for testing this. Each time I would make headway I would start over with a clean install to make sure everything was still working properly. |
@@ -8,7 +8,7 @@ else | |||
CMAKE_OPTS="$CMAKE_OPTS -DLMMS_BUILD_MSYS=1" | |||
fi | |||
|
|||
export PATH=$PATH:$MINGW/bin | |||
export PATH=$MINGW/bin:$PATH |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Will these changes affect linux builds though?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
export PATH=$MINGW/bin:$PATH
Will these changes affect linux builds though?
They should be benign and Travis-CI passes, which is a good sign.
What this change does is force the path to prefer mingw's BIN first to any system BIN. I'd expect this behavior as well and we use it in other areas. If I were to harbor a guess, I'd say this is just an oversight that's been copied and pasted since our first mingw builds (see d3516cd#diff-7e1f009e6b046eedd463b471771e7a05R2)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Ah, ok, this might actually be the cause of #2951, I'll check if this change fixes that.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
this might actually be the cause of #2951, I'll check if this change fixes that.
It shouldn't help that problem. This should only fix problems where the same library exists in both $PATH
as well as $MINGW/bin
which I wouldn't expect to solve #2951. This is just a change which fixes the order of resolution. #2951 is more likely related to something with pkg-config
or CMAKE_PREFIX_PATH
since the library isn't found at all.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actually might fix it. I made the change because the right pkg-config wasn't being found so it could find libsndfile. I got a successful build after that, and I wasn't aware of #2951.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I can confirm this change fixed #2951 for me. My machine and (@T0NIT0RMX's too) probably had portaudio dev files installed alongside the mingw ones, so it was picking them up for pkg-config and looking for alsa, which is probably required for linux portaudio.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Great job @dragoneagle :D
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
👍 @Umcaruje let's commit that change directly then (no PR). That will close out your issue and then @dragoneagle can rebase his PR as needed.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Well I was planning on actually setting a windows environment tommorow, and testing this PR, and possibly merging it, so if I encounter problems on my windows environment, I'll do the direct change 👍
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@Umcaruje sounds good. FYI to you @dragoneagle and anyone else, we may want to start (eventually) publishing to scoop.sh
which is Homebrew clone for Windows.
This works amazingly well @dragoneagle this is a terrific job you've done. The only issue I encountered is that the libgig download gives a 404, but that is a problem with the linuxsampler website, and will probably get fixed. I went around the issue by using a web archive link at line 209 in wget https://web.archive.org/web/20160330120039/http://download.linuxsampler.org/packages/libgig-$gigver.tar.bz2 -O $HOME/gig-source.tar.xz I'm merging this. Thank you for your contribution, this makes compiling on Windows a breeze 🎉 |
* Made fltk install to the write path * Supress downloading packages that are either installed with pacman or built from source * Build libogg from source * Cleanup after installing libogg * Build libvorbis from source * Build flac from source * Build libgig from source * Build STK from source * Fixed "already run" check on STK * Took credit for calling somoene at Stanford a yutz so they don't think it was the project lead * Fixed symlink to pkg-config for 32 bit systems * Enabled shared library production with fltk * Hopefully fixed install of STK * Backed off last change regarding STK. It broke things * Put the cross toolchins in front of path. It needs to be there so the cross tools override the native tools * Move libgig dlls into the bin directory so they can be found * let libjpeg and fltk be installed from the repository as they are needed at that version for make package
Modified build bash scripts for mingw to fix errors when building under mingw. Specifically when compiling gigplayer and malletstk. The build now compiles without error on mingw systems with a clean install. There are still some issues with make package and some dlls that can't be found, but the program compiles and runs. It is beyond my abilities with cmake to fix these.
-E. Allen Soard