[PATCH] Fix and improve Windows/MinGW/MSYS build

[Uwe Hermann]

See patch.

Uwe.

[Carl-Daniel Hailfinger]

Am 03.07.2011 00:51 schrieb Uwe Hermann:
> Fix and improve Windows/MinGW/MSYS build.
>
> Makefile: Use $(OS_ARCH) to add some MinGW-specific workarounds and
> settings, so that a simple "make" is sufficient on MinGW (instead of
> manual Makefile hacking).
>
> Explicitly set CC=gcc in the Makefile, otherwise you get an error like
> "cc: command not found" on MinGW.

Mh. Would CC?=gcc work as well?
> MinGW doesn't have ffs(), use gcc's __builtin_ffs() instead.

Neat hack...
> Add /usr/local/include and /usr/local/lib to CPPFLAGS/LDFLAGS, that's
> where libusb-win32 and libftdi stuff is usually placed on MinGW/MSYS.
>
> Disable serprog (no sockets) and all PCI-based programmers (no libpci)
> for now. That leaves dummy, ft2232_spi, and buspirate_spi enabled on
> MinGW per default.
>
> serial.c: Add missing casts to avoid compile errors due to -Werror.

Those casts seem to use a Windows-specific cast.
> Signed-off-by: Uwe Hermann
> Index: Makefile
>
> --- Makefile	(Revision 1362)
> +++ Makefile	(Arbeitskopie)
> @@ -84,6 +84,88 @@
>  endif
>  endif
>
> +ifeq ($(OS_ARCH), MINGW32_NT-5.1)
> +# Explicitly set CC=gcc on MinGW, otherwise: "cc: command not found".
> +CC=gcc
> +# MinGW doesn't have the ffs() function, but we can use gcc's __builtin_ffs().
> +CFLAGS += -Dffs=__builtin_ffs
> +# libusb-win32/libftdi stuff is usually installed in /usr/local.
> +CPPFLAGS += -I/usr/local/include
> +LDFLAGS += -L/usr/local/lib
> +# Serprog is not supported under Windows/MinGW (missing sockets support).
> +ifeq ($(CONFIG_SERPROG), yes)
> +UNSUPPORTED_FEATURES += CONFIG_SERPROG=yes
> +else
> +override CONFIG_SERPROG = no
> +endif
> +# For now we disable all PCI-based programmers on Windows/MinGW (no libpci).

We really need a HAVE_PCI or CONFIG_PCI variable which takes care of this.
> +ifeq ($(CONFIG_INTERNAL), yes)
> +UNSUPPORTED_FEATURES += CONFIG_INTERNAL=yes
> +else
> +override CONFIG_INTERNAL = no
> +endif
> +ifeq ($(CONFIG_RAYER_SPI), yes)
> +UNSUPPORTED_FEATURES += CONFIG_RAYER_SPI=yes
> +else
> +override CONFIG_RAYER_SPI = no
> +endif
> +ifeq ($(CONFIG_NIC3COM), yes)
> +UNSUPPORTED_FEATURES += CONFIG_NIC3COM=yes
> +else
> +override CONFIG_NIC3COM = no
> +endif
> +ifeq ($(CONFIG_GFXNVIDIA), yes)
> +UNSUPPORTED_FEATURES += CONFIG_GFXNVIDIA=yes
> +else
> +override CONFIG_GFXNVIDIA = no
> +endif
> +ifeq ($(CONFIG_SATASII), yes)
> +UNSUPPORTED_FEATURES += CONFIG_SATASII=yes
> +else
> +override CONFIG_SATASII = no
> +endif
> +ifeq ($(CONFIG_ATAHPT), yes)
> +UNSUPPORTED_FEATURES += CONFIG_ATAHPT=yes
> +else
> +override CONFIG_ATAHPT = no
> +endif
> +ifeq ($(CONFIG_DRKAISER), yes)
> +UNSUPPORTED_FEATURES += CONFIG_DRKAISER=yes
> +else
> +override CONFIG_DRKAISER = no
> +endif
> +ifeq ($(CONFIG_NICREALTEK), yes)
> +UNSUPPORTED_FEATURES += CONFIG_NICREALTEK=yes
> +else
> +override CONFIG_NICREALTEK = no
> +endif
> +ifeq ($(CONFIG_NICNATSEMI), yes)
> +UNSUPPORTED_FEATURES += CONFIG_NICNATSEMI=yes
> +else
> +override CONFIG_NICNATSEMI = no
> +endif
> +ifeq ($(CONFIG_NICINTEL), yes)
> +UNSUPPORTED_FEATURES += CONFIG_NICINTEL=yes
> +else
> +override CONFIG_NICINTEL = no
> +endif
> +ifeq ($(CONFIG_NICINTEL_SPI), yes)
> +UNSUPPORTED_FEATURES += CONFIG_NICINTEL_SPI=yes
> +else
> +override CONFIG_NICINTEL_SPI = no
> +endif
> +ifeq ($(CONFIG_OGP_SPI), yes)
> +UNSUPPORTED_FEATURES += CONFIG_OGP_SPI=yes
> +else
> +override CONFIG_OGP_SPI = no
> +endif
> +ifeq ($(CONFIG_SATAMV), yes)
> +UNSUPPORTED_FEATURES += CONFIG_SATAMV=yes
> +else
> +override CONFIG_SATAMV = no
> +endif
> +endif
>
> ifeq ($(OS_ARCH), libpayload)
>  CC:=CC=i386-elf-gcc lpgcc
>  AR:=i386-elf-ar
> @@ -271,7 +353,12 @@
>  endif
>
> ifeq ($(CONFIG_FT2232_SPI), yes)
> +ifeq ($(OS_ARCH), MINGW32_NT-5.1)
> +# No pkg-config files for libftdi on Windows/MinGW, just assume it's installed.
> +FTDILIBS := -lftdi -lusb

Umm... if pkg-config fails, the fallback below will do exactly what you
did manually above. Does the fallback fail for you?
> +else
>  FTDILIBS := $(shell pkg-config --libs libftdi 2>/dev/null || printf "%s" "-lftdi -lusb")
> +endif
>  # This is a totally ugly hack.
>  FEATURE_CFLAGS += $(shell LC_ALL=C grep -q "FTDISUPPORT := yes" .features && printf "%s" "-D'CONFIG_FT2232_SPI=1'")
>  FEATURE_LIBS += $(shell LC_ALL=C grep -q "FTDISUPPORT := yes" .features && printf "%s" "$(FTDILIBS)")
> Index: serial.c
> ===================================================================
> --- serial.c	(Revision 1362)
> +++ serial.c	(Arbeitskopie)
> @@ -196,7 +196,7 @@
>
> while (writecnt > 0) {
>  #ifdef _WIN32
> WriteFile(sp_fd, buf, writecnt, &tmp, NULL);
> WriteFile(sp_fd, buf, writecnt, (PDWORD)&tmp, NULL);

Use (uint32_t *) instead of (PDWORD). That said, this might mess up
aliasing handling in the compiler, and we should fix the type of tmp
instead.
> #else
>    	tmp = write(sp_fd, buf, writecnt);
>  #endif
> @@ -219,7 +219,7 @@
>
> while (readcnt > 0) {
>  #ifdef _WIN32
> ReadFile(sp_fd, buf, readcnt, &tmp, NULL);
> ReadFile(sp_fd, buf, readcnt, (PDWORD)&tmp, NULL);

Same here.
> #else
>    	tmp = read(sp_fd, buf, readcnt);
>  #endif

Regards,
Carl-Daniel

[Uwe Hermann]

On Sun, Jul 03, 2011 at 01:11:51AM +0200, Carl-Daniel Hailfinger wrote:
> > Explicitly set CC=gcc in the Makefile, otherwise you get an error like
> > "cc: command not found" on MinGW.
>
> Mh. Would CC?=gcc work as well?

Doesn't work unfortunately.
> > serial.c: Add missing casts to avoid compile errors due to -Werror.
>
> Those casts seem to use a Windows-specific cast.

Yes (they're in the _WIN32 ifdef though).
> > ifeq ($(CONFIG_FT2232_SPI), yes)
> > +ifeq ($(OS_ARCH), MINGW32_NT-5.1)
> > +# No pkg-config files for libftdi on Windows/MinGW, just assume it's installed.
> > +FTDILIBS := -lftdi -lusb
>
> Umm... if pkg-config fails, the fallback below will do exactly what you
> did manually above. Does the fallback fail for you?

Oops, true, I removed that hunk.
> > WriteFile(sp_fd, buf, writecnt, &tmp, NULL);
> > WriteFile(sp_fd, buf, writecnt, (PDWORD)&tmp, NULL);
>
> Use (uint32_t *) instead of (PDWORD). That said, this might mess up
> aliasing handling in the compiler, and we should fix the type of tmp
> instead.

WriteFile(), a Windows API call, expects an 'LPDWORD', i.e. a pointer to
a DWORD, I think it makes sense to use that (in the _WIN32 ifdef part).

http://msdn.microsoft.com/en-us/library/aa365747%28VS.85%29.aspx
http://msdn.microsoft.com/en-us/library/cc230318%28PROT.10%29.aspx

We do similar things with 'fdtype' on MinGW:

programmer.h:typedef HANDLE fdtype;
  programmer.h:typedef int fdtype;
> > #else
> >    	tmp = write(sp_fd, buf, writecnt);

This write() here, however, returns an ssize_t, so I changed "long tmp"
to "ssize_t tmp" here, too.

Updated patch attached.

Uwe.

[Carl-Daniel Hailfinger]

Am 03.07.2011 16:42 schrieb Uwe Hermann:
> On Sun, Jul 03, 2011 at 01:11:51AM +0200, Carl-Daniel Hailfinger wrote:
> > > Explicitly set CC=gcc in the Makefile, otherwise you get an error like
> > > "cc: command not found" on MinGW.
> >
> > Mh. Would CC?=gcc work as well?
>
> Doesn't work unfortunately.
> > > serial.c: Add missing casts to avoid compile errors due to -Werror.
> >
> > Those casts seem to use a Windows-specific cast.
>
> Yes (they're in the _WIN32 ifdef though).
> > > ifeq ($(CONFIG_FT2232_SPI), yes)
> > > +ifeq ($(OS_ARCH), MINGW32_NT-5.1)
> > > +# No pkg-config files for libftdi on Windows/MinGW, just assume it's installed.
> > > +FTDILIBS := -lftdi -lusb
> >
> > Umm... if pkg-config fails, the fallback below will do exactly what you
> > did manually above. Does the fallback fail for you?
>
> Oops, true, I removed that hunk.
> > > WriteFile(sp_fd, buf, writecnt, &tmp, NULL);
> > > WriteFile(sp_fd, buf, writecnt, (PDWORD)&tmp, NULL);
> >
> > Use (uint32_t *) instead of (PDWORD). That said, this might mess up
> > aliasing handling in the compiler, and we should fix the type of tmp
> > instead.
>
> WriteFile(), a Windows API call, expects an 'LPDWORD', i.e. a pointer to
> a DWORD, I think it makes sense to use that (in the _WIN32 ifdef part).
>
> http://msdn.microsoft.com/en-us/library/aa365747%28VS.85%29.aspx
> http://msdn.microsoft.com/en-us/library/cc230318%28PROT.10%29.aspx
>
> We do similar things with 'fdtype' on MinGW:
>
> programmer.h:typedef HANDLE fdtype;
>   programmer.h:typedef int fdtype;
> > > #else
> > >    	tmp = write(sp_fd, buf, writecnt);
>
> This write() here, however, returns an ssize_t, so I changed "long tmp"
> to "ssize_t tmp" here, too.
>
> Updated patch attached.
>
> Fix and improve Windows/MinGW/MSYS build.
>
> Makefile: Use $(OS_ARCH) to add some MinGW-specific workarounds and
> settings, so that a simple "make" is sufficient on MinGW (instead of
> manual Makefile hacking).
>
> Explicitly set CC=gcc in the Makefile, otherwise you get an error like
> "cc: command not found" on MinGW.
>
> MinGW doesn't have ffs(), use gcc's __builtin_ffs() instead.
>
> Add /usr/local/include and /usr/local/lib to CPPFLAGS/LDFLAGS, that's
> where libusb-win32 and libftdi stuff is usually placed on MinGW/MSYS.
>
> Disable serprog (no sockets) and all PCI-based programmers (no libpci)
> for now. That leaves dummy, ft2232_spi, and buspirate_spi enabled on
> MinGW per default.

Is it really impossible to enable serprog? IIRC it works with Cygwin.
> serial.c: Use correct type for 'tmp', both on Windows/MinGW (DWORD)
> and POSIX (ssize_t).
>
> Signed-off-by: Uwe Hermann

Acked-by: Carl-Daniel Hailfinger

Regards,
Carl-Daniel

[Uwe Hermann]

On Sun, Jul 03, 2011 at 09:16:06PM +0200, Carl-Daniel Hailfinger wrote:
> Acked-by: Carl-Daniel Hailfinger

Thanks, r1363.

Uwe.