2.0.7.1 fails to build on Windows 7 / Ruby 1.9.3p392 (windef.h: duplicate 'unsigned')

[davidfstr]

This is a supported configuration, as given by the support matrix.

Repro steps:

• Fresh Windows 7 SP1 VM.
• Install Ruby 1.9.3p392 with the binary installer from RubyInstaller.
• Install DevKit-tdm-32-4.5.2 from the same site. (This is the only supported DevKit for Ruby 1.9.3, as per the instructions on the page.)
• gem install rdiscount (resolves to RDiscount 2.0.7.1)

Build output: https://gist.github.com/davidfstr/5210654

[davidfstr]

I'm trying to get a sane Windows development environment up and running. Here are my notes in case anybody wants to follow along.

Environment:

• Windows 7 SP1, 32 bit
• Ruby 1.9.3p392 (RubyInstaller)
• DevKit-tdm-32-4.5.2
  • (The install instructions really should be on the download page: Save [don't run]. Run magic script.)
  • Verified with successful run of gem install json --platform=ruby

I have cloned RDiscount to ~/dev/rdiscount

$ cd ~/dev/rdiscount  # use Windows equivalent
$ cd ext
$ C:\Ruby193\DevKit\devkitvars.bat  # DevKit folks really should document this prominently
$ ruby extconf.rb  # generate Makefile
$ make > make.log 2<&1

Alright it fails with a similar error as the original gem install failure: https://gist.github.com/davidfstr/5219172

[davidfstr]

Okay these errors from make do not make sense to me:

windef.h:229:23: error: duplicate 'unsigned'
windef.h:238:23: error: duplicate 'unsigned'
windef.h:238:23: error: two or more data types in declaration specifiers
windef.h:241:24: error: duplicate 'unsigned'

On the referenced lines:

229: typedef unsigned long DWORD;
238: typedef unsigned char BYTE;
241: typedef unsigned short WORD;

It's almost like the compiler is treating unsigned and long as two separate datatypes instead of as a combined unsigned long type.

In case this is a compiler-specific issue, it is: gcc (tdm-1) 4.5.2

[davidfstr]

Damn C macros. Apparently ext/Makefile has the following C flags:

CPPFLAGS = -DHAVE_RAND -DHAVE_SRAND -DSIZEOF_UNSIGNED_LONG=4 -DSIZEOF_UNSIGNED_INT=4 -DDWORD='unsigned long' -DWORD='unsigned int' -DBYTE='unsigned char' -DVERSION=\"2.0.7\" -DFD_SETSIZE=2048 $(DEFS) $(cppflags)

Notice that it defines DWORD, WORD, and BYTE (among other things). Also note that it defines these values differently than what windef.h says. Ugh.

I altered the line to omit defining these flags. Hopefully RDiscount will pick up the windef.h variants of these flags from the environment. The new build error in make.log: https://gist.github.com/davidfstr/5219304

It looks like rdiscount.c compiled just fine. No such luck for resource.c:

In file included from resource.c:17:0:
markdown.h:85:5: error: expected specifier-qualifier-list before 'DWORD'

Great. That's a really cryptic error meaning that DWORD hasn't been defined. So it's not picking it up from windef.h (or the CPPFLAGS that I just removed the define from).

[davidfstr]

Let's try adding #include <windef.h> to the top of resource.c.

compiling resource.c
resource.c: In function '___mkd_freeLine':
resource.c:25:11: error: called object '65536l' is not a function
resource.c: In function '___mkd_freefootnote':
resource.c:63:11: error: called object '65536l' is not a function
resource.c:64:11: error: called object '65536l' is not a function
resource.c:65:11: error: called object '65536l' is not a function
resource.c: In function '___mkd_freefootnotes':
resource.c:79:8: error: called object '65536l' is not a function
resource.c: In function '___mkd_freemmiot':
resource.c:111:8: error: called object '65536l' is not a function
resource.c:112:8: error: called object '65536l' is not a function
resource.c:113:8: error: called object '65536l' is not a function
make: *** [resource.o] Error 1

What the hell?

25: DELETE(ptr->text);
63: DELETE(f->tag);
64: DELETE(f->link);
65: DELETE(f->title);
...

Damn macros... I wonder how windef.h broken DELETE.

Let's find DELETE. Apparently it's in cstring.h:

#define T(x)        (x).text
#define S(x)        (x).size
#define ALLOCATED(x)    (x).alloc

#define DELETE(x)   ALLOCATED(x) ? (free(T(x)), S(x) = (x).alloc = 0) \
                     : ( S(x) = 0 )

Wat...

[davidfstr]

So if I expand resource.c:25, DELETE(ptr->text);, I get:

if (ptr->text).alloc {
    free((ptr->text).text);
    (ptr->text).size = 0;
    (ptr->text).alloc = 0;
} else {
    (ptr->text).size = 0;
}

Not sure why windef.h would break that...

[davidfstr]

Figured it out... Apparently windef.h defines something called DELETE that clobbers the one in cstring.h.

So the true macro expansion that was causing the compile error was some kind of nonsense - hence the ridiculous errors.

[davidfstr]

Ugh. Okay. Let's be sure to add the #include <windef.h> before #include "cstring.h" so that cstring's version of DELETE clobbers windef's (and not the other way around).

It looks like markdown.c and resource.c compile fine if I do that.

[davidfstr]

And in the end it looks like I get a successful build of rdiscount.so if I add #include <windef.h> before #include "cstring.h" in the following files.

#       modified:   markdown.h
#       modified:   resource.c
#       modified:   toc.c

Cool. Now that the hacked fix is complete. Let's figure out how to do a proper fix.

[davidfstr]

Aside: It looks like this is exactly the same issue as #74. At least I'm familar with the issue now.

[davidfstr]

Suggested mechanisms for a proper fix.

• Have Discount use namespaced defines for WORD, DWORD, etc - by prefixing them with MKD_ or similar.
  • Given that these defines have very generic names, this is the approach I would prefer, since it avoids future issues with other system headers redefining these macros.
  • Orc is opposed to this approach for some unclear reason.
• Have Discount conditionally include windef.h to define the macros if on Windows. Probably in config.h.
  • Orc seems to prefer this approach.

[davidfstr]

I've confirmed that adding (a conditional) #include <windef.h> to the bottom of config.h (along with the CPPFLAGS changes in the Makefile made earlier) is a viable fix.

[jonforums]

I've found it's safer to #include <windows.h> and let it pull in windef.h. I've seen strange errors when I incorrectly cherrypicked headers. That said, it may not matter in this case.

I'll try to carve out time to clone the repo and play with 32-bit flavors of 1.9.3p393 + mingw.org 4.6.2 and 2.0.0p73 + mingw-w64 4.7.2. More when I know more.

@luislavena what is your advice on windows.h vs. windef.h?

[luislavena]

@jonforums windows.h should be the header to be included. windef.h and others have been historically there, but we can't warrantee that.

[davidfstr]

I've found an even better fix that doesn't involve touching Discount code at all.

RDiscount's build system is different than Discount's. (I'm not exactly sure why.) Both build systems generate a Makefile and config.h.

RDiscount's build system defines many macro variables using compile flags in the Makefile:

CPPFLAGS = -DHAVE_RAND
           -DHAVE_SRAND
           -DSIZEOF_UNSIGNED_LONG=4
           -DSIZEOF_UNSIGNED_INT=4
           -DDWORD='unsigned long'
           -DWORD='unsigned int'
           -DBYTE='unsigned char'
           -DVERSION=\"2.0.7\"
           -DFD_SETSIZE=2048
           $(DEFS) $(cppflags)

By contrast, Discount's build system defines most of these variables in config.h:

#define OS_MINGW32_NT 1
#define USE_DISCOUNT_DL 1
#define DWORD unsigned long
#define WORD unsigned short
#define BYTE unsigned char
#define HAVE_SRAND 1
#define INITRNG(x) srand((unsigned int)x)
#define HAVE_BZERO 1
#define HAVE_RAND 1
#define COINTOSS() (rand()&1)
#define HAVE_STRCASECMP 1
#define HAVE_STRNCASECMP 1
#define HAVE_GETCWD 1
#define TABSTOP 4
#define HAVE_MALLOC_H 1
#define PATH_FIND "/usr/bin/find"
#define PATH_SED "/usr/bin/sed"

It turns out that moving DWORD, WORD, and BYTE from compile time flags to config.h (as Discount's original build system does) eliminates the declaration errors that were seen before.

Naturally this requires me to update the RDiscount build system in extconf.rb. Where the main difficulty I expect is that the mkmf Ruby module it depends on does not appear to be documented.

[davidfstr]

As a completely separate issue, Discount's build system for 2.0.7 appears to work correctly out of the box on Windows 7, with 2 caveats:

(1) configure.sh will not detect autodetect gcc as a C compiler. It wants cc instead. Fix by explicitly specifying a value for CC before calling configure.sh.

$ cd PATH/TO/rdiscount
$ git submodule update --init
$ cd discount
$ sh
$$ which gcc
/usr/mingw/bin/gcc.exe
$$ CC=/usr/mingw/bin/gcc.exe ./configure.sh
$$ exit

(2) MinGW's make appears to be confused by the simultaneous presence of VERSION and version.c files. Fix by adding an explicit noop rule to the generated Makefile:

# Dummy rule for VERSION.
# Needed to keep mingw's make from getting confused.
VERSION: 
    echo -n ""

Now the subsequent call to make should succeed:

$ make

The proper fix to Discount here is to (1) verify that this build issue exists in the latest Discount and (2) update configure.sh such that the Makefile it generates has the extra aforementioned dummy rule.

[davidfstr]

Looks like shirosaki beat me to the punch on implementing the new fix. #89

[jonforums]

@shirosaki is clever and quick that way :smile_cat:

[davidfstr]

Closed as Fixed.

[davidfstr]

This fix is now available in the latest RDiscount 2.0.7.2.