#220 VC++ Win32 patch for libspectrum

open
nobody
None
5
2013-01-01
2010-02-20
Simon Owen
No

I've revisited building libspectrum (current SVN revision 4116) under Win32. I'm using Cygwin for the initial autoconf setup and header generation, then switching to a native Visual Studio project with a hand-tuned config.h

I'm using VC6 to reduce Cygwin runtime dependencies and so I can link with the common DLL version of the CRT (MSVCRT.DLL). I've also been building with VS2005, as the compilers included with VS2003-2008 have a similar feature set (usually behind gcc4 though). I've limited my suggested changes to what's needed for the later VC++ versions.

I've attached a patch and included details of the changes below.

Executable scripts with CRLF line endings choke Cygwin, and configure.in also has issues with CRs in the multi-line AC_OUTPUT when it's split into separate file names. Properties of the following need changing from native to LF in SubVersion:

autogen.sh
configure.in
generate.pl.in
tape_accessors.pl
tape_set.pl

That's the bare minimum I needed to get it to work, though additional files would benefit from the same change. Currently, libspectrum.h will be generated with a mix of LF and CRLF endings, since other parts of it still have native endings.

dll.c has switch(ul_reason_for_call) but the parameter name is just 'reason'

WIN32_DLL [__declspec(dllexport)] should be before the return type in declarations, rather than between return type and function name. The files affected are:

generate.pl.in
libspectrum.h.in
make-perl.c

myglib/ghash.c:257 uses uint32_t in g_str_hash() on line 257, even if HAVE_STDINT_H isn't defined. I had to change it to libspectrum_dword instead.

warajevo_read.c:588 defines 'last_block' mid-scope, which I think might be valid in newer C standards (and certainly works with gcc4), but doesn't work with either version of VC++ that I tried.

M_LN2 (used by tzx_read.c) isn't defined in math.h under VC6, and requires _USE_MATH_DEFINES to be defined before including math.h to be present under VC2003 and later. I've added a conditional define to internals.h, but it could be done more thoroughly from configure.in if preferred.

VS6 doesn't support long long so __int64 is needed instead. I've not included the following change, which would need to be incorporated into the typedef section of make-perl.c.

#if defined(_MSC_VER) && _MSC_VER <= 1200
typedef unsigned __int64 libspectrum_qword;
typedef signed __int64 libspectrum_signed_qword;
#else
typedef unsigned long long libspectrum_qword;
typedef signed long long libspectrum_signed_qword;
#endif

Discussion

  • Simon Owen

    Simon Owen - 2010-02-20

    Patch file

     
  • Fredrick Meunier

    Thanks, all applied bar the line ending changes.

    > I've revisited building libspectrum (current SVN revision 4116) under
    > Win32. I'm using Cygwin for the initial autoconf setup and header
    > generation, then switching to a native Visual Studio project with a
    > hand-tuned config.h
    [...]
    > Executable scripts with CRLF line endings choke Cygwin, and configure.in
    > also has issues with CRs in the multi-line AC_OUTPUT when it's split into
    > separate file names.

    It's not quite clear to me if you are bootstrapping with the same compiler as you are subsequently using to compile - this is a fundamental expectation of the build system as-is.

    Having said that, changing the line endings in just those files seems a bit hacky for the base system, my preferred options would be:
    1) New build system taking into account the extra details we need these days (in addition to your requirement, easier use of cross-compilation toolchains where you can't execute the compiled make-perl executable for example)
    2) A script to do the required conversion on the target platform, and a how to build doc detailing what steps to follow for that configuration
    3) The mooted line-feed hard coding in the source code repository

    > VS6 doesn't support long long so __int64 is needed instead. I've not
    > included the following change, which would need to be incorporated into the
    > typedef section of make-perl.c.
    >
    > #if defined(_MSC_VER) && _MSC_VER <= 1200
    > typedef unsigned __int64 libspectrum_qword;
    > typedef signed __int64 libspectrum_signed_qword;
    > #else
    > typedef unsigned long long libspectrum_qword;
    > typedef signed long long libspectrum_signed_qword;
    > #endif

    I think this suggestion implies that you used gcc to bootstrap, before using VC to compile as make-perl.c has the long long emitting code wrapped up in an if ( sizeof(long long) == 8 ) test which would fail to compile on VC?

    I think you need to use VC to bootstrap with if possible, then a patch like the below would do the trick?
    Index: make-perl.c
    ===================================================================
    --- make-perl.c (revision 4120)
    +++ make-perl.c (working copy)
    @@ -89,12 +89,18 @@
    if( sizeof( long ) == 8 ) {
    printf( "typedef unsigned long libspectrum_qword;\n" );
    printf( "typedef signed long libspectrum_signed_qword;\n" );
    +#if defined(_MSC_VER) && _MSC_VER <= 1200
    + } else {
    + printf( "typedef unsigned __int64 libspectrum_qword;\n" );
    + printf( "typedef signed __int64 libspectrum_signed_qword;\n" );
    +#else
    } else if( sizeof( long long ) == 8 ) {
    printf( "typedef unsigned long long libspectrum_qword;\n" );
    printf( "typedef signed long long libspectrum_signed_qword;\n" );
    } else {
    fprintf( stderr, "No plausible 64 bit types found\n" );
    return 1;
    +#endif
    }

    #endif /* #ifdef HAVE_STDINT_H */

     
  • Simon Owen

    Simon Owen - 2010-02-21

    > Thanks, all applied bar the line ending changes.

    Thanks Fred!

    > It's not quite clear to me if you are bootstrapping with the same compiler
    > as you are subsequently using to compile - this is a fundamental
    > expectation of the build system as-is.

    It wasn't the main build process that caused the trouble, but that I checked out the project files using the Windows version of SubVersion. If I use the Cygwin version of svn I get Unix line endings, everything works fine.

    With native Windows line endings it falls at the first hurdle, even if I use Cygwin+gcc for the rest of the build process:

    $ ./autogen.sh
    ./autogen.sh: line 2: $'\r': command not found
    ./autogen.sh: line 5: $'\r': command not found
    etc.

    > Having said that, changing the line endings in just those files seems a
    > bit hacky for the base system

    I understand your concern, particularly since the affected files are common to all platforms. Though ultimately the problem files will never work if checked out with DOS line endings. I believe forced LF should work on all known systems, but if you think it's risky I'm happy to drop my request. I know about the issue now so I can either use Cygwin svn or just flag the problem files for LF SVN checkout in my local files.

    > I think this suggestion implies that you used gcc to bootstrap, before
    > using VC to compile

    I was indeed switching between bootstrapping with gcc and building with VC++, but I was replacing config.h with a hand-tuned VC++ version between the ./configure and make steps. That meant make-perl.c gets built with knowledge of VC++, so all dynamically generated files are suitable for it.

    I have since checked that bootstrapping does work correctly with both VC6 and VS2005 build environments, and the build process for each does successfully build a .libs/libspectrum-7.dll. Curiously the compiler warned about -g being an unrecognised flag, despite the configure test reporting it as being supported (non-cached result too).

    The post-build test building fails due to lack of unistd.h (included from test/test.c), and even wrapping that in a HAVE_UNISTD_H test and replacing ssize_t with size_t ran into a linking failure later on. I can look into that further if you're interested.

    > make-perl.c has the long long emitting code wrapped up in an
    > if ( sizeof(long long) == 8 ) test which would fail to compile on VC?

    The 'sizeof(long long) == 8' test is valid for later versions of VC++, but won't work with VC6.

    > I think you need to use VC to bootstrap with if possible, then a patch
    > like the below would do the trick?
    <snip>

    Yes, that patch works fine for both VC6 and VS2005, so please include it if you could :)

     
  • Fredrick Meunier

    > I understand your concern, particularly since the affected files are
    > common to all platforms. Though ultimately the problem files will never
    > work if checked out with DOS line endings. I believe forced LF should work
    > on all known systems, but if you think it's risky I'm happy to drop my
    > request. I know about the issue now so I can either use Cygwin svn or just
    > flag the problem files for LF SVN checkout in my local files.

    Not so much risky as fragile and prone to be stuffed up again in the future. I'd be happy to have a "HOWTO" doc for how to build in this config added with the instructions above.

    > The post-build test building fails due to lack of unistd.h (included from
    > test/test.c), and even wrapping that in a HAVE_UNISTD_H test and replacing
    > ssize_t with size_t ran into a linking failure later on. I can look into
    > that further if you're interested.

    Happy to accept patches for this if you are so inclined.

     
  • Stuart Brady

    Stuart Brady - 2011-04-06

    So, which files should we run 'svn propset svn:eol-style LF' on, apart from autogen.sh?

    While I'd agree that this may well get stuffed up at some future point, it seems to make sense to fix it...

    Maybe if Fuse built with VC++, we'd get a few more Win32 developers contributing? :-)

     

Log in to post a comment.