2.0.7 fails to build on windows with mingw or mingw-w64 #74

Closed
jonforums opened this Issue Feb 5, 2013 · 26 comments

Comments

Projects
None yet
4 participants

On win7 32bit using either mingw gcc 4.6.2 or mingw-w64 gcc 4.7.2 I get gem build failures due to the use of the DWORD macro.

The DWORD, WORD, and BYTE macros are known windows data types. Their unfortunate re-use in discount/rdiscount causes problems with the mingw/mingw-w64 headers as in the following failure:

C:\ruby193\lib\ruby\gems\1.9.1\gems\rdiscount-2.0.7\ext>ruby extconf.rb
checking for random()... no
checking for srandom()... no
checking for rand()... yes
checking for srand()... yes
checking size of unsigned long... 4
checking size of unsigned int... 4
creating Makefile

C:\ruby193\lib\ruby\gems\1.9.1\gems\rdiscount-2.0.7\ext>make
compiling rdiscount.c
In file included from c:\devkit\mingw\bin\../lib/gcc/mingw32/4.6.2/../../../../include/windows.h:48:0,
                 from c:\devkit\mingw\bin\../lib/gcc/mingw32/4.6.2/../../../../include/winsock2.h:22,
                 from c:/ruby193/include/ruby-1.9.1/ruby/win32.h:40,
                 from c:/ruby193/include/ruby-1.9.1/ruby/defines.h:223,
                 from c:/ruby193/include/ruby-1.9.1/ruby/ruby.h:67,
                 from c:/ruby193/include/ruby-1.9.1/ruby.h:32,
                 from rdiscount.c:2:
c:\devkit\mingw\bin\../lib/gcc/mingw32/4.6.2/../../../../include/windef.h:229:23: error: duplicate 'unsigned'
c:\devkit\mingw\bin\../lib/gcc/mingw32/4.6.2/../../../../include/windef.h:238:23: error: duplicate 'unsigned'
c:\devkit\mingw\bin\../lib/gcc/mingw32/4.6.2/../../../../include/windef.h:238:23: error: two or more data types in declaration specifiers
c:\devkit\mingw\bin\../lib/gcc/mingw32/4.6.2/../../../../include/windef.h:241:24: error: duplicate 'unsigned'
rdiscount.c: In function 'rb_rdiscount_to_html':
rdiscount.c:18:5: warning: implicit declaration of function 'rb_rdiscount__get_flags' [-Wimplicit-function-declaration]
rdiscount.c:18:5: warning: ISO C90 forbids mixed declarations and code [-Wdeclaration-after-statement]
rdiscount.c: In function 'rb_rdiscount_toc_content':
rdiscount.c:55:5: warning: ISO C90 forbids mixed declarations and code [-Wdeclaration-after-statement]
make: *** [rdiscount.o] Error 1

One fix is to choose better macro names. When I replaced DWORD with MKD_DWORD in markdown.{c,h} and mkdio.c and updated extconf.rb, I was able to successfully run a ruby extconf.rb && make cycle and get a rdiscount.so. This type of fix should also re-enable gem installs for users using our rubyinstaller with the devkit (msys+mingw+autotools+goodies enabling native gem installs on windows from source).

Given that 1.6.8 installs used to work well for windows users, I'd love to see 2.0.7+ continue to work. We use rdiscount as our smoke test for users to determine if their devkit install looks OK.

Glad to see rdiscount re-supported...hopefully this info helps, and I'll try to find time to swing back around and do more testing.

I can confirm this issue.

It is related to DWORD being defined already by Windows headers:

http://msdn.microsoft.com/en-us/library/cc230318.aspx

Passing them as compilation flag is causing these errors.

The following patch makes DWORD, WORD and BYTE private by prepending MKD_ to them and use across the files and headers.

https://gist.github.com/luislavena/4719655

Let me know if patch is not enough so I can send a pull request (did this between dinner and dessert)

Thank you
❤️ ❤️ ❤️

Forgot to /cc @davidfstr , just in case 😉

Owner

davidfstr commented Feb 6, 2013

@Orc Any chance I could get you to integrate this improved support for MinGW into Discount 2.1.5?

(That way I'll just be able to pick it up for RDiscount 2.1.5.)

Orc commented Feb 6, 2013

Goddamn windows. I define WORD, DWORD, and the like in my configuration scripts, which I use in a bunch of contexts, so I can't just MKD_ prefix them.

But if the windows definitions are the same as my definitions, I won't even need to run the check function and define them. What header file defines WORD, DWORD, and BYTE? If I can pull that in and be certain that it's not lying, then I'll #include that header file in config.h instead of defining those three.

Owner

davidfstr commented Feb 6, 2013

What header file defines WORD, DWORD, and BYTE?

@luislavena I delegate this question to you since I am not familiar with MinGW.

Goddamn windows. I define WORD, DWORD, and the like in my configuration scripts, which I use in a bunch of contexts, so I can't just MKD_ prefix them.

Can I know why are you doing that?

What header file defines WORD, DWORD, and BYTE?

It doesn't matter what header, because is part of the platform (windows have definitions split into multiple header files, mostly winbase.h which is included from windows.h or any other Windows header... you get the idea)

If I can pull that in and be certain that it's not lying, then I'll #include that header file in config.h instead of defining those three.

Why it will be lying? What platform did you encounter that problem? Wouldn't be better only check for that?

Why can't you check if DWORD is defined? by default #include "ruby.h" will include Windows headers, which is already done by any thing that you try compilation with mkmf

Orc commented Feb 6, 2013

Discount is not a ruby program. #including ruby.h is not really an option, and I don't use mkmf either.

Discount is not a ruby program. #including ruby.h is not really an option, and I don't use mkmf either.

Can you answer the other questions?

  • Why it will be lying? What platform did you encounter that problem? Wouldn't be better only check for that?
  • Why can't you check if DWORD is defined?

I was not saying you use ruby.h or mkmf, I was suggesting that what mkmf is doing is overkill.

Also, I need to understand your reasoning behind defining those elements (which problems they solve) before I can propose a solution that you might not like (as you stated with prepend MKD_ for your private data type definitions.

Orc commented Feb 6, 2013

If you don't know what header file they come from you can just say it -- I certainly won't think any less of you if you have trouble digging down through the mess of interlocking #includes to find the appropriate sys/ file that has those definitions in it. But since I don't use windows I can't actually fabricate a configure.inc patch to work around windows predefinitions unless I know what header file to include, sorry :-(

If you don't know what header file they come from you can just say it

Again, that was not my question, and again, you will need to check that discount is running on Windows to include that header and that will defeat the defines you're running from mkmf or any external configure/build script.

Dunno how this will help you but the file is windef.h, at least in MinGW, dunno if Visual Studio still have/use it, but I think it should.

windef.h is included by winbase.h which is also included by windows.h

Please answer my previous questions to I can better understand where and how to propose a better solution than forcing defines.

Thank you.

Orc commented Feb 6, 2013

windef.h? Thanks. Do the C compilers on Windows spit out any standard #define that indicate that they're running in a windows environment?

#ifdef _WIN32 will handle both MSFT's VC++ compiler and the MinGW gcc's. There are other defines for 32 vs 64bit as well as other defines, but _WIN32 should be what you're looking for.

That said, if you can tell us specifically what you're trying to do, Luis or I can help more.

BTW, what's your build system?

@Orc as background, there are essentially two flavors of mingw typically used in windows dev: the mingw.org flavor which is currently only 32bits, and the mingw-w64 flavor which supports both 32 and 64bits.

While the headers are different between the two implementations, I hope the differences will not impact what you're trying to do. For example, regarding DWORD in windef.h here are the two current implementations:

mingw.org impl
mingw-w64 impl and scroll to line 117

If Luis and I are more clear on your changes to better support discount/rdiscount on windows, we can guide you through the landmines. Also, as most distros have one or both flavors of mingw cross compilers in their repos (mingw-w64 linux binaries also available for download), we could advise on how to quickly setup on your system so you can quickly test your mods.

update: arch -> pacman -Si mingw32-gcc, ubuntu -> aptitude search '^mingw'

Owner

davidfstr commented Feb 23, 2013

@Orc: Would it be helpful if I moved this issue to the Orc/discount project directly? I am not currently working on it myself.

Orc commented Feb 25, 2013

Yes, it would be.

Owner

davidfstr commented Mar 21, 2013

@jonforums: I might note that mingw64 is not a supported DevKit for Ruby 1.9.3, as per the RubyInstaller download page. Only the tdm DevKit is supported for Ruby 1.8.6 - 1.9.3.

Am I correct in assuming that your Ruby is installed via RubyInstaller and mingw64 is installed via the DevKit from the same project?

@davidfstr mingw-w64 is being used by RubyInstaller CI environment to build 1.9.3, 2.0.0 and trunk of Ruby.

You can download binary packages of Ruby built with it or use the official installer that uses tdm-32-4.5.2

Independently of the DevKit version used, the redefinition of WORD, DWORD and BYTE will be quite visible on any installation.

Owner

davidfstr commented Mar 21, 2013

Independently of the DevKit version used, the redefinition of WORD, DWORD and BYTE will be quite visible on any installation.

Probably true. I'm currently tracking a number of Windows-specific testing tasks at the moment (#83, #86, #88), so this issue will probably get fixed one way or another for RDiscount 2.0.7.2.

Now if only I could extract some more time from the universe to get these tasks processed faster...

@davidfstr let me know anything that I can do to help out on this.

The RubyInstaller CI is running a remote slave at home, perhaps I can setup rdiscount build there? That would help?

Now if only I could extract some more time from the universe to get these tasks processed faster...

Welcome to my world 😉

@davidfstr for support sanity reasons, we only officially support specific DevKit versions with specific RubyInstaller releases. This is primarily a constraint for the pre-built RubyInstaller versions that most people use.

However, if you're more adventurous (of the Arch rolling release mindset) and regularly build ruby from source, you have more flexibility. Even though we don't officially support it, I specifically added support for using multiple DevKit flavors into the RubyInstaller build recipes. For example, currently you can build using one of the following toolchains

C:\projects\rubyinstaller-git>rake devkit:ls
=== Available DevKit's ===
    mingw-32-3.4.5
    mingw-32-4.6.2
    mingw64-32-4.7.2
    mingw64-64-4.7.2
    mingwbuilds-32-4.7.2
 => tdm-32-4.5.2          [default]
    tdm-32-4.6.1
    tdm-32-4.7.1
    tdm-64-4.6.1
    tdm-64-4.7.1

As Luis mentioned, recent RubyInstaller releases (and CI builds) use the mingw-w64 toolchains so the redefinition issue is a real problem.

While Luis has offered to add rdiscount to his CI build slaves (a good thing since both 32 and 64bit is tested), if you also want some quick testing of edge rdiscount let me know. Due to other priorities, I've scaled back all of my other open-source interests, but will continue to help here if needed.

note: if you want to build your own DevKit flavor for testing native gem installs, it's usually as easy as cloning the RubyInstaller repo and doing a rake devkit sfx=1 dkver=mingw64-32-4.7.2 and "injecting" it into a RubyInstaller installation via a ruby dk.rb install. There's a bit more to it, but if you're interested, either Luis or I can help you get things set up.

Owner

davidfstr commented Mar 22, 2013

I've figured out a fix in the style that Orc mentioned. Now I just need to polish it for submission to the upstream Discount project.

Orc commented Mar 22, 2013

Make sure it doesn't conflict with the configuration change I've already applied to the baseline.

Owner

davidfstr commented Mar 23, 2013

Closed as Fixed.

@davidfstr davidfstr closed this Mar 23, 2013

Owner

davidfstr commented Apr 7, 2013

This fix is now available in the latest RDiscount 2.0.7.2.

Fantastic for both 1.9.3 (mingw.org gcc 4.6.2) and 2.0.0 (mingw-w64 gcc 4.7.2) on my Win7 32-bit box.

C:\>ruby --version
ruby 1.9.3p411 (2013-04-04 revision 40099) [i386-mingw32]
C:\>gem i rdiscount-2.0.7.2.gem
Temporarily enhancing PATH to include DevKit...
Building native extensions.  This could take a while...
Successfully installed rdiscount-2.0.7.2
1 gem installed
C:\>ruby -rrdiscount -ve "puts RDiscount::VERSION; puts RDiscount.new('**Yeh!**').to_html"
ruby 1.9.3p411 (2013-04-04 revision 40099) [i386-mingw32]
2.0.7
<p><strong>Yeh!</strong></p>
----
C:\>ruby --version
ruby 2.0.0p109 (2013-04-07 revision 40169) [i386-mingw32]
C:\>gem i rdiscount-2.0.7.2.gem
Temporarily enhancing PATH to include DevKit...
Building native extensions.  This could take a while...
Successfully installed rdiscount-2.0.7.2
1 gem installed
C:\>ruby -rrdiscount -ve "puts RDiscount::VERSION; puts RDiscount.new('**Yeh!**').to_html"
ruby 2.0.0p109 (2013-04-07 revision 40169) [i386-mingw32]
2.0.7
<p><strong>Yeh!</strong></p>

This fix is now available in the latest RDiscount 2.0.7.2.

Thank you so much! ❤️ ❤️ ❤️

@andig andig referenced this issue in flori/json Oct 17, 2014

Closed

Build fails with devkit on win32 #218

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment