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

Fixes build errors under mingw (3113) #3369

Merged
merged 18 commits into from Mar 5, 2017
Merged

Fixes build errors under mingw (3113) #3369

merged 18 commits into from Mar 5, 2017

Conversation

@DragonEagle
Copy link
Contributor

DragonEagle commented Feb 19, 2017

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

@tresf
Copy link
Member

tresf commented Feb 19, 2017

@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.

@DragonEagle
Copy link
Contributor Author

DragonEagle commented Feb 19, 2017

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

This comment has been minimized.

Copy link
@Umcaruje

Umcaruje Feb 20, 2017

Member

Will these changes affect linux builds though?

This comment has been minimized.

Copy link
@tresf

tresf Feb 20, 2017

Member

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)

This comment has been minimized.

Copy link
@Umcaruje

Umcaruje Feb 22, 2017

Member

Ah, ok, this might actually be the cause of #2951, I'll check if this change fixes that.

This comment has been minimized.

Copy link
@tresf

tresf Feb 22, 2017

Member

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.

This comment has been minimized.

Copy link
@DragonEagle

DragonEagle Feb 22, 2017

Author Contributor

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.

This comment has been minimized.

Copy link
@Umcaruje

Umcaruje Feb 22, 2017

Member

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.

This comment has been minimized.

Copy link
@Umcaruje

This comment has been minimized.

Copy link
@tresf

tresf Feb 22, 2017

Member

👍 @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.

This comment has been minimized.

Copy link
@Umcaruje

Umcaruje Feb 22, 2017

Member

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 👍

This comment has been minimized.

Copy link
@tresf

tresf Feb 22, 2017

Member

@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.

@zonkmachine zonkmachine added this to In Progress in Release 1.2.0 RC3 Feb 22, 2017
@Umcaruje
Copy link
Member

Umcaruje commented Mar 5, 2017

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 msys_helper.sh:

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 🎉

@Umcaruje Umcaruje merged commit f2fbf1e into LMMS:master Mar 5, 2017
1 check passed
1 check passed
continuous-integration/travis-ci/pr The Travis CI build passed
Details
@zonkmachine zonkmachine moved this from In Progress to Done in Release 1.2.0 RC3 Mar 5, 2017
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
Linked issues

Successfully merging this pull request may close these issues.

None yet

3 participants
You can’t perform that action at this time.