Orc / discount

My C implementation of John Gruber's Markdown markup language
http://www.pell.portland.or.us/~orc/Code/discount
Other
858 stars 156 forks source link

Autoconf macro @DWORD@ not replaced in mkdio.h on Windows #149

Closed meisme closed 8 years ago

meisme commented 8 years ago

Building discount 2.2.0 under cygwin or MSYS very nearly works out of the box, but in mkdio.h.in line 8 the @DWORD@ macro is passed through unmodified, so mkdio.h line 8 ends up looking like

typedef @DWORD@ mkd_flag_t;

Manually changing @DWORD@ to unsigned int allows it to compile. Once compiled it passes most tests, the output of make test is attached below (the tests that fail also fail for me when compiled on an up to date Debian 8 box so that's a separate issue).

discount-tests.txt

Orc commented 8 years ago

Can you show me

  1. how you run configure.sh (what options do you pass in on the command line)
  2. the output from running configure.sh
  3. your config.log
  4. what version of cygwin/msys you're running
  5. what version of windows you're running
meisme commented 8 years ago
$ ./configure.sh
Configuring for [markdown]
Looking for cpp (/lib/cpp) ok
looking for install (/usr/bin/install)
checking the C compiler (cc) oh ick, it looks like gcc
looking for ar (/usr/bin/ar)
looking for ranlib (/usr/bin/ranlib)
checking for "volatile" keyword (found)
checking for "const" keyword (found)
Checking for "inline" keyword (found)
defining WORD & DWORD scalar types (defined in WinDef.h)
looking for a reentrant basename (found)
looking for header libgen.h (found)
looking for header stdlib.h (found)
looking for the alloca function (found in alloca.h)
looking for header pwd.h (found)
looking for the getpwuid function (found)
looking for the srandom function (found)
looking for the bzero function (found)
looking for the random function (found)
looking for the strcasecmp function (found)
looking for the strncasecmp function (found)
looking for the fchdir function (found)
looking for header malloc.h (found)
looking for find (/usr/bin/find)
looking for "ln -s" (/usr/bin/ln)
looking for ar (/usr/bin/ar)
looking for ranlib (/usr/bin/ranlib)
looking for sed (/usr/bin/sed)
generating Makefile
generating version.c
generating mkdio.h

$ make
cc -Wno-return-type -Wno-implicit-int -I. -g -I. -c main.c
In file included from main.c:13:0:
./mkdio.h:8:9: error: stray ‘@’ in program
 typedef @DWORD@ mkd_flag_t;
         ^
./mkdio.h:8:15: error: stray ‘@’ in program
 typedef @DWORD@ mkd_flag_t;
               ^
./mkdio.h:8:10: error: unknown type name ‘DWORD’
 typedef @DWORD@ mkd_flag_t;
          ^
Makefile:97: recipe for target 'main.o' failed
make: *** [main.o] Error 1

This log is from Cygwin 64-bit running on Windows 7 (afaik cygwin itself isn't versioned, but I updated it just prior to running this test). GCC version 5.3.0, GNU Make version 4.1, autoconf version 2.69. sh is bash version 4.3.42(4)-release.

I have the same behaviour on Cygwin 64-bit running Windows 10, and on msys running on Windows 10. msys is also 64-bit and was also updated today. It has GCC 3.4.4 (msys special), GNU Make version 3.81, autoconf version 2.68 and sh is bash 3.1.23(1)-release

On msys:

$ ./configure.sh
Configuring for [markdown]
Looking for cpp (/bin/cpp) ok
looking for install (/bin/install)
checking the C compiler (no C compiler found)

$ CC=gcc ./configure.sh
Configuring for [markdown]
Looking for cpp (/bin/cpp) ok
looking for install (/bin/install)
checking the C compiler (gcc) oh ick, it looks like gcc
looking for ar (/bin/ar)
looking for ranlib (/bin/ranlib)
checking for "volatile" keyword (found)
checking for "const" keyword (found)
Checking for "inline" keyword (found)
defining WORD & DWORD scalar types (defined in WinDef.h)
looking for a reentrant basename (not found)
looking for header stdlib.h (found)
looking for the alloca function (found)
looking for header pwd.h (found)
looking for the getpwuid function (found)
looking for the srandom function (found)
looking for the bzero function (found)
looking for the random function (found)
looking for the strcasecmp function (found)
looking for the strncasecmp function (found)
looking for the fchdir function (found)
looking for header malloc.h (found)
looking for find (/bin/find)
looking for "ln -s" (/bin/ln exists, but -s does not seem to work)
looking for ln (/bin/ln)
looking for ar (/bin/ar)
looking for ranlib (/bin/ranlib)
looking for sed (/bin/sed)
generating Makefile
generating version.c
generating mkdio.h

$ make
gcc -Wno-return-type -Wno-implicit-int -I. -g -I. -c main.c
In file included from main.c:13:
./mkdio.h:8: error: stray '@' in program
./mkdio.h:8: error: stray '@' in program
./mkdio.h:8: error: parse error before "mkd_flag_t"
./mkdio.h:8: warning: data definition has no type or storage class
./mkdio.h:12: error: parse error before "mkd_flag_t"
./mkdio.h:13: error: parse error before "mkd_flag_t"
./mkdio.h:17: error: parse error before "mkd_flag_t"
./mkdio.h:18: error: parse error before "mkd_flag_t"
./mkdio.h:28: error: parse error before "mkd_flag_t"
./mkdio.h:34: error: parse error before "mkd_flag_t"
./mkdio.h:35: error: parse error before "mkd_flag_t"
./mkdio.h:60: error: parse error before "mkd_flag_t"
./mkdio.h:77: error: parse error before "mkd_flag_t"
In file included from main.c:20:
pgm_options.h:6: error: parse error before '*' token
main.c: In function `main':
main.c:68: error: parse error before "flags"
main.c:86: error: `flags' undeclared (first use in this function)
main.c:86: error: (Each undeclared identifier is reported only once
main.c:86: error: for each function it appears in.)
make: *** [main.o] Error 1

config.log.txt Log from the first cygwin run.

Orc commented 8 years ago

Does the WinDef.h on your machine define any of DWORD/WORD/BYTE?

meisme commented 8 years ago

Yeah, all of them appear to be defined. At least the below compiles and produces the expected output.

#include <WinDef.h>
#include <stdio.h>

int main(int argc, char** argv) {
    printf("DWORD: %d\nWORD:  %d\nBYTE:  %d\n", sizeof(DWORD), sizeof(WORD), sizeof(BYTE));
}
Orc commented 8 years ago

Okay, I think I figured it out. I'm a non-windows joint, so I didn't keep that part of the autoconfigure script uptodate. I pushed a change to configure.inc just now that hopefully defines DWORD, etc; can you pull from @HEAD and see if it works better?

meisme commented 8 years ago

It doesn't, as WinDef.h is only included in config.h and not mkdio.h (mkdio.h does not include config.h).

However this showed me where to look, and as far as I can tell there isn't really a need for special handling of the Windows platform at all. By removing the special handling (configure.inc lines 879 through 896) everything works fine. Eliminating special handling of Windows when possible seems like a good idea to me since you don't use it regularly.

(By fine I mean it configures, compiles and all tests complete successfully on cygwin and msys - though sh in msys has broken unicode support so for that I had to rename the tests/muñoz.t file to have it run)

I've also discovered that while including WinDef.h works fine with the MinGW Windows API implementation, it actually causes a compile error on the later versions of the Microsoft Windows API implementation. They want you to always use the windows.h header now, which would be massive overhead.

### configure.inc lines 879 - 896 --- the lines that can be removed ###
    if AC_QUIET AC_CHECK_HEADERS WinDef.h; then
    # windows machine; BYTE, WORD, DWORD already
    # defined

    for x in $@; do
        case "$x" in
        sub)AC_SUB DWORD DWORD
        AC_SUB WORD WORD
        AC_SUB BYTE BYTE
        ;;
        *)
        echo "#include <WinDef.h>" >> "$__cwd"/config.h
        ;;
        esac
    done
    TLOG " (defined in WinDef.h)"
    return 0
    fi
Orc commented 8 years ago

Alas, I did this whole elaborate WinDef.h detection nonsense because someone on Windows had the internally generated DWORD collide with WinDef.h when they tried to build the thing :-( I will have to puzzle over it for a bit more -- I can't trust that some combination of Windows+msys/mingw/cygnus unix emulation won't drag in the windows headers even if internally I only use standard Unix headers.

meisme commented 8 years ago

Have you changed the system since then? For instance by changing from using a typedef to a macro for DWORD? Because as far as I can tell there shouldn't be any trace left of a DWORD symbol once the preprocessor has run. DWORD is defined as a typedef in Windows (both MS and MinGW), so there shouldn't be any conflict.

Having removed the lines I indicated above, even if I insert #include <windows.h> directly below the header guards in mkdio.h and config.h, it still compiles on cygwin and msys and all tests complete successfully. I get a warning from the preprocessor about DELETE being redefined, but it still works.

Orc commented 8 years ago

It's possible. I've sanitized the definition so that there's only one public side where @DWORD@ is visible. I've pushed a version of configure.inc into the github mirror that doesn't look for WinDef.h; can you try it out and let me know how it works? (can you tell me what version of msys you're using, too; I'm going to try to set up a win xp virtual machine with msys so I can run the occasional test build on a long obsolete version of windows.)

-david parsons

On May 28, 2016, at 3:56 AM, meisme notifications@github.com wrote:

Have you changed the system since then? For instance by changing from using a typedef to a macro for DWORD? Because as far as I can tell there shouldn't be any trace left of a DWORD symbol once the preprocessor has run. DWORD is defined as a typedef in Windows (both MS and MinGW), so there shouldn't be any conflict.

Having removed the lines I indicated above, even if I insert #include directly below the header guards in mkdio.h and config.h, it still compiles on cygwin and msys and all tests complete successfully. I get a warning from the preprocessor about DELETE being redefined, but it still works.

— You are receiving this because you were assigned. Reply to this email directly, view it on GitHub https://github.com/Orc/discount/issues/149#issuecomment-222302634, or mute the thread https://github.com/notifications/unsubscribe/AABOgl4DRaE_ETjKJGU6dF9tXXZE6RGFks5qGB9TgaJpZM4IkBAy.

meisme commented 8 years ago

Yes, that works for me both on msys and cygwin. And it still works even if I make it include windows.h, so it shouldn't break even in a polluted environment.

msys hasn't been versioned as a whole since 1.0.11 which released sometime in 2007, instead it uses a package system where components are updated more individually. I've been using the mingw-get-setup.exe manager linked to from the getting started gude, and I just have the lastest packages installed.