No MinGW build option - Githubissues

GoogleCodeExporter commented 9 years ago


 - Operating System: Windows
 - Machine type (32-bit or 64-bit): both
 - Mupen64Plus version: 1.99.5

It would be nice to have a MinGW choice in the 'projects' of core source.

I'm baffled as to why some many projects use visual studio projects, when it 
seems that it would be much more fluid to use the MinGW build system under 
windows instead.

Original issue reported on code.google.com by topherwh...@gmail.com on 15 Nov 2012 at 2:39

GoogleCodeExporter commented 9 years ago

Original comment by s...@narfation.org on 16 Nov 2012 at 2:11

Added labels: OpSys-Win-32, Priority-Low, Type-Enhancement

GoogleCodeExporter commented 9 years ago

Here is the archive: m64p_msys-mingw_dec2012.zip

This contains (most) all the changes needed to properly compile under 
MinGW/MSYS, though its still a little experimental.

I altered a recent pull from merc repo. It can be unpacked in the "source" 
folder, just above the individual package sources, or you could unzip to its 
own folder and diff against source tree; I retained the folder structure in the 
zip.

Original comment by topherwh...@gmail.com on 4 Dec 2012 at 12:58

Attachments:

GoogleCodeExporter commented 9 years ago

And we should replace the newest changes with the version you prepared? Please 
create a pull request or send actual patches. Otherwise we might end up 
removing fixes by using your stuff

Original comment by s...@narfation.org on 4 Dec 2012 at 7:30

GoogleCodeExporter commented 9 years ago

For example, this would introduce the build failure on x86 for rice again.

Original comment by s...@narfation.org on 4 Dec 2012 at 7:32

GoogleCodeExporter commented 9 years ago

 # list of source files to compile
-SOURCE = \
+qSOURCE = \
        $(SRCDIR)/main.c \
-   $(SRCDIR)/volume.c \
+ $(SRCDIR)/volume.c
+ifeq ($(OS),MINGW)
+qSOURCE += \
+ $(SRCDIR)/osal_dynamiclib_win32.c
+else
+qSOURCE += \
        $(SRCDIR)/osal_dynamiclib_unix.c
+endif

Are you kidding me? What is qSOURCE and why do you want to break the compile?

Original comment by s...@narfation.org on 4 Dec 2012 at 7:43

GoogleCodeExporter commented 9 years ago

I'd love to do a patch, but i haven't created one in years.  If you could point 
me to some info on howto properly create one (the way you would like it), i'd 
be greatful.

Those changes were applied to a pull that is only a week old.  And the source 
snippet above shouldn't break anything.  I don't know who you are or where you 
came from, but your response to free code from a volunteer is coming across 
rude.  - did you even try to compile?  why don't you apply it to newest source 
on your very own PC and test before complaining?  (You could make a temp dir, 
pull source, apply .zip, and compile)  After all, it should be tested on other 
systems (especially systems that it's not intended for).

As for "qSOURCE", that was already in the code; maybe i accidentally hit the 
"Q" key.  All you have to do is change occurrences of "qSOURCE" to "SOURCE" and 
you're back; thanks for the 'bug report'

Thank you
veganaize

Original comment by topherwh...@gmail.com on 4 Dec 2012 at 10:47

GoogleCodeExporter commented 9 years ago

https://developer.mozilla.org/en-US/docs/Creating_a_patch

It broke stuff in a very obvious way. I tried to compile it and it didn't work 
because your changes broke stuff. And how can you it accidentally the q key 3 
times in different places?

And don't forget how started this type of friendly conversation. Do you 
remember your mail on the mailing list?

Original comment by s...@narfation.org on 5 Dec 2012 at 10:14

GoogleCodeExporter commented 9 years ago

haha... tru that!  If the case of the mysterious "Q" was my fault (which i'm 
not trying to dodge in any way); look at the changes that i've made; I must 
have copied the original "qSOURCE" (while possibly mis-striking the q key) and 
pasted the other two.  I did notice that it looked strange, but everything 
seemed to compile fine on my system and I didn't want to break anything if 
someone had put it in there for some reason.

When I *triple* checked *everything* I must have seen the changes in WinDiff, 
but not realized the change from "SOURCE" to "qSOURCE".

While I don't mind (and actually prefer) my mistakes pointed out, I felt a 
little attacked.  And if you think of the time & energy we've wasted on this, 
both of of probably could've prepared appropriate patches by now - speaking of, 
I'm serious about how I should prepare a patch - I made one for MSYS years ago 
(to keep it back compatible with win9x ;) and I was shot down in a rude way, 
even though I had prepared everything _exactly_ how they had asked.  This is my 
first contribution (that i can think of) to an open-source project since then.

Not to mention my programming skills rusty/lacking in some areas, but I intend 
to correct all that, and it's much of my motivation working on this; I want to 
be the best dang programmer on this square meter of earth.

Original comment by topherwh...@gmail.com on 5 Dec 2012 at 11:38

GoogleCodeExporter commented 9 years ago

sorry, i just noticed the mozilla patch link :]

Original comment by topherwh...@gmail.com on 5 Dec 2012 at 11:39

GoogleCodeExporter commented 9 years ago

m64p.msys.u8pwr.diff -- patch without whitespace changes
m64p.msys.u8pr.diff -- patch with whitespace changes

Both were created just above the individual module source trees, in the 
top-level source directory.  It primarily applies to the makefiles, with some 
slight changes to source as well.  I addressed two additional minor issues as 
well; 

In the (near) future I will start providing more specific patches.  Comments & 
criticism enjoyed in the meantime.

Original comment by topherwh...@gmail.com on 9 Dec 2012 at 12:27

Attachments:

GoogleCodeExporter commented 9 years ago

I wanted to try your patch, but it failed. Please see the attached log for more 
information. I can have a look at it later, but maybe it is better when you 
have a look.

mupen64plus-audio-sdl changeset:   86:95d0050869fc
mupen64plus-core changeset:   496:5edb58495c23
mupen64plus-input-sdl changeset:   153:d5741eb0687f
mupen64plus-rsp-hle changeset:   90:3a580cea470b
mupen64plus-ui-console changeset:   123:cddc82fa7a43
mupen64plus-video-rice changeset:   162:b1389c996f20

And maybe you can make an extra patch for whitespace changes as they aren't 
relevant for mingw support.

Original comment by s...@narfation.org on 10 Dec 2012 at 9:28

Attachments:

patch.log

GoogleCodeExporter commented 9 years ago

And I dont understand why printf-puts replacement is necessary for mingw32 
support. Maybe you can also separate this change and give some insight why it 
is better (performance doesn't really count here). It is also weird to have a 
commit message inside the source code.

Does this line generate an compiler warning in mingw32 or why was it changed? 
It is not really necessary in my eyes. But when you want to have it, please 
make a separate patch an use the style of the surrounding code (so instead of 
"if ( (..) || (..) )" use "if ((..) || (..))")

-  if (jump_vec > 127 || jump_vec < -128)
+  if ( (jump_vec > 127) || (jump_vec < -128) )

Another thing to the whitespace part. Please don't introduce tabs in places 
were spaces are used (see the big license header for example). Richard isn't a 
big fan of it in general and I personally think that the license header is the 
completely wrong place for tabs.

Original comment by s...@narfation.org on 10 Dec 2012 at 9:41

GoogleCodeExporter commented 9 years ago

Here my further review (m64p.msys.u8pr.diff):

* mupen64plus-audio-sdl applied (with minor differences) in 
https://bitbucket.org/richard42/mupen64plus-audio-sdl/commits/95d0050869fcfcd0b3
4791bbe430bd7f
* mupen64plus-core applied (with minor differences and without the "if" change) 
in 
https://bitbucket.org/richard42/mupen64plus-core/commits/7c9610d72a8e86a12ceb9e1
e3719cb90ea72a743
* mupen64plus-input-sdl applied (with minor differences) in 
https://bitbucket.org/richard42/mupen64plus-input-sdl/commits/2f55b8b13414ce40fb
3f591cc5c6ec2b235df186
* mupen64plus-rsp-hle applied (with minor differences) in 
https://bitbucket.org/richard42/mupen64plus-rsp-hle/commits/3a580cea470bdfe536b9
6b361eb7e56fb7d3aa25
* mupen64plus-ui-console applied (with minor differences and without the puts 
change) in 
https://bitbucket.org/richard42/mupen64plus-ui-console/commits/cddc82fa7a43b7115
f55b05b913910d560ba5d15
* mupen64plus-video-rice applied (with minor differences, no special -msse 
handling and with strncasecmp+strcasecmp enabled in MinGW because it seems to 
be defined in the MinGW32 headers when strict ansi is not enforced --- but no 
compile tested in Windows+MinGW; please recheck) in 
https://bitbucket.org/richard42/mupen64plus-video-rice/commits/b1389c996f209ad69
264562f7b57e0af292f63a5

Original comment by s...@narfation.org on 10 Dec 2012 at 6:39

GoogleCodeExporter commented 9 years ago

The puts() calls in the main.c for ui-console were just an "I'm here, might as 
well take care of..." situation.  When testing the command-line ui, I noticed 
that when my command line wraps because of lots of switches or long ROM path, 
it distorts the ASCII logo; I added one blank line above it to try and solve 
this.  While there, I noticed that although most of the lines didn't require 
variable substitution, they were using slow printf() calls; puts() is a far 
faster alternative that doesn't waste time parsing strings that should just be 
sent straight to stdout.  Speed should always be an issue, whether it's a 
string of text or a million polygons.

One of those patches has whitespace changes, the other doesn't.

I don't know anything about a commit message in the source code. Maybe i diff'd 
them wrong.  Unless your talking about one of my /* comments */. It's generally 
considered proper to insert small comments for questionable lines of code.  As 
I don't have "commit" access, that is my only channel (that I know of) to note 
some things.

The "if" statement was a situation where I'd originally thought the line was 
causing an issue, but later found it wasn't.  I prefer those types of 
conditional expressions inside of their own braces for clarity, and to ensure 
proper operator precedence; if you'd rather keep it the original way, i don't 
care, i just think it's a little sloppy.

Thank you for considering my contributions,
veganaize

Original comment by topherwh...@gmail.com on 10 Dec 2012 at 9:38

GoogleCodeExporter commented 9 years ago

The -msse flag for rice video was necessary for me - otherwise I got a bunch of 
xmm0, etc errors.

Original comment by topherwh...@gmail.com on 10 Dec 2012 at 9:48

GoogleCodeExporter commented 9 years ago

Please read  about the git or hg patch format to find more about other ways to 
write commit messages. You can also fork the repos on bitbucket, add commits 
and ask richard42 to pull the changes.

Please check the current version in hg and provide changes when necessary. 
Please also check ABI compatibility with components compiled with Visual Studio 
(calling convention, alignments, type differences, ...)

Original comment by s...@narfation.org on 11 Dec 2012 at 7:21

GoogleCodeExporter commented 9 years ago

Ok, I've just tried to build it and can say that it isn't working. I will 
propose necessary changes myself. There are maybe also runtime problems which I 
will not test or tackle right now.

Just the build environment used for completeness (running Debian Wheezy on 
amd64 and mxe 04fe9ffeef4c20fc2336bdce93de8cf75f8b0003)

aptitude install scons yasm build-essential git autoconf automake wget bison 
cmake flex intltool libtool pkg-config unzip mercurial
git clone -b stable http://github.com/mxe/mxe.git
cd mxe
make gcc sdl libpng freetype speex libsamplerate pthreads freeglut -j 4 JOBS=4

cat > usr/i686-pc-mingw32/lib/pkgconfig/gl.pc << EOF
Name: gl
Version: 0
Description: OpenGL
Libs: -lopengl32
EOF
cat > usr/i686-pc-mingw32/lib/pkgconfig/glu.pc << EOF
Name: gl
Version: 0
Description: OpenGL
Libs: -lglu32
EOF
echo 'Requires: zlib' >> usr/i686-pc-mingw32/lib/pkgconfig/libpng.pc
cd ..

export PATH="$(pwd)/mxe/usr/bin:$(pwd)/mxe/usr/i686-pc-mingw32/bin/:$PATH"
export PKG_CONFIG_PATH="$(pwd)/mxe/usr/i686-pc-mingw32/lib/pkgconfig/"
for i in core ui-console audio-sdl input-sdl rsp-hle video-rice; do hg clone 
https://bitbucket.org/richard42/mupen64plus-${i}; done
for i in core ui-console audio-sdl input-sdl rsp-hle video-rice; do  make -C 
mupen64plus-${i}/projects/unix/ CC=i686-pc-mingw32-gcc CXX=i686-pc-mingw32-g++ 
HOST_CPU=i686 UNAME=MINGW PIC=0 V=1 LTO=1 all ; done
mkdir result
for i in core ui-console audio-sdl input-sdl rsp-hle video-rice; do cp 
mupen64plus-${i}/projects/unix/*.* result; cp mupen64plus-${i}/data/* result; 
done

Original comment by s...@narfation.org on 12 Dec 2012 at 9:49

GoogleCodeExporter commented 9 years ago

Fixes are pushed into my repositories:

 * mupen64plus-core 498:0436f632e52c: Don't use unwritten time information for RTC read on MinGW
 * mupen64plus-core 497:589f99fc6e15: Don't add malformed/missing linker flag to LDFLAGS on MinGW
 * mupen64plus-ui-console 124:4bed1d4376fb: Really build mupen64plus.exe on MinGW

Original comment by s...@narfation.org on 12 Dec 2012 at 10:22

GoogleCodeExporter commented 9 years ago

I completely removed (and reinstalled msys) and some libs i had hanging in my 
PATH.

I re-wrote the README for msys to include the ./configure && make && make 
install commands for dependencies.  I did get everything to compile fine with 
sound and intermixed with visual c libraries, with no runtime errors.  However, 
I can't get SDL.dll to offer smooth operation with sound when I custom compile 
it; I use the pre-compiled binary copied into the 'test' dir for testing.

The OpenGL development files are provided with a typical MinGW/MSYS install.  I 
had some in another dir that were getting linked in previously. I'm preparing 
individual patches for this and a couple other issues, right now.

Original comment by topherwh...@gmail.com on 12 Dec 2012 at 11:32

GoogleCodeExporter commented 9 years ago

msys.opengl.u8pwr.diff patch has _no_ whitespace changes, the 
msys.opengl.u8pr.diff does.

This patch adjusts the 'core' and 'video' makefiles to use the installed opengl 
libraries when compiling with MINGW/MSYS.

Original comment by topherwh...@gmail.com on 13 Dec 2012 at 12:36

Attachments:

GoogleCodeExporter commented 9 years ago

Updated README.msys.txt

Attached msys makefile for lpng

Original comment by topherwh...@gmail.com on 13 Dec 2012 at 12:46

Attachments:

GoogleCodeExporter commented 9 years ago

The opengl/glu stuff sounds like an build environment problem. Maybe you can 
fix it in the environment instead of adding mingw quirks to the makefile.

Original comment by s...@narfation.org on 13 Dec 2012 at 7:31

GoogleCodeExporter commented 9 years ago

I think we need to slow down.  In the failed build you show above, it looks 
like you didn't install zlib dev pkg, and you have LTO enabled, which isn't 
compatible with win32.  MSYS support doesn't necessarily mean that it will 
cross-compile with random flags set.

1st: Build the entire mupen64plus suite as you normally would...
(run-time test, no problems?)
2nd: Build mupen64plus in MSYS exactly as specified in README.msys.txt
(run-time test, no problems?)
3rd: Knock yourself out with all the customization you want, but the intentions 
of my contribution are to get you as far as step two.

I had a typo in previous opengl patches, I've attached new ones.

BTW: OpenGL is as standard to MinGW as MSVCRT.DLL is to Windows; I wouldn't 
call that an environment problem, and I wouldn't call taking advantage of 
either one a "quirk".  The fact is that the current makefile tests for opengl 
w/ pkg-config - Most MSYS users will never satisfy that requirement, as they 
already have the opengl dev files pre-installed, so they will be less likely to 
install a third-party package and get a .pc file.  Please reconsider my opengl 
patch, everything compiles without error on my setup (winxp pro sp3 w/msys 
1.0.17 mingw gcc 4.6.2) with the dependencies installed into /usr/local - it 
runs perfect.

msys.opengl.u8pwr -> withOUT whitespace changes
msys.opengl.u8pr -> with whitespace changes

Original comment by topherwh...@gmail.com on 13 Dec 2012 at 8:17

Attachments:

GoogleCodeExporter commented 9 years ago

I've installed zlib with all necessary files to build against it. LTO is 
compatible enough to produce working versions of the binaries in the cross 
compiler setup (why should Win32 not be compatible with LTO? VC++ allows to use 
LTO since a long time). All changes for it are committed in my repositories.

Original comment by s...@narfation.org on 13 Dec 2012 at 9:05

GoogleCodeExporter commented 9 years ago

And btw. The "standard" mingw packages are all all without pkg-config files.

The nice README you wrote is building a lot of extra packages. That's why I 
would prefer that just two small text files are added to the build environment 
instead of adding some extra workarounds for some mingw based build 
environments (maybe even get them shipped by default in the w32api-dev 
package). Other packages would automatically benefit from it (in the m64p 
world: mupen64plus-video-arachnoid, mupen64plus-video-z64, 
mupen64plus-video-glide64 and mupen64plus-video-glide64-mk2).

Original comment by s...@narfation.org on 13 Dec 2012 at 9:26

GoogleCodeExporter commented 9 years ago

What extra packages are being built?  I'm gonna take a breather from all of 
this.  As a final note, should probably change __declspec's in "m64p_types.h" 
to __attribute__'s for MINGW/GCC... maybe change line 16 from:

#if defined(WIN32)

...to...

#if defined(WIN32) && !defined(__GNUC__)

...for consistency...  everything compiles/runs fine for me at this point (even 
without this change).

Original comment by topherwh...@gmail.com on 13 Dec 2012 at 9:40

GoogleCodeExporter commented 9 years ago

 * pkg-config
 * zlib
 * nasm
 * lpng
 * SDL
 * freetype

Ok, you've mostly only mention the configure call, but I thought the rest 
should also follow.

I will have a look on the recommended change later

Original comment by s...@narfation.org on 13 Dec 2012 at 9:55

GoogleCodeExporter commented 9 years ago

The commits were today pulled by Richard. I would say the bug can now be 
closed. Feel free to test it and resent some patches in case something was 
forgotten/doesn't work.

Original comment by s...@narfation.org on 31 Dec 2012 at 8:30

Changed state: Fixed

jwmanyon / mupen64plus

No MinGW build option #523