Clang 10 FIX: games/devilutionx

classic Classic list List threaded Threaded
2 messages Options
Reply | Threaded
Open this post in threaded view
|

Clang 10 FIX: games/devilutionx

Brian Callahan-6
Hi ports --

As reported by naddy@, games/devilutionx doesn't build with clang-10.
The problem is that our ar doesn't understand archives with LLVM thin
LTO objects in it (devilutionx turns -flto=thin on by default if you
have a newer toolchain).

The solution is to disable LTO, which makes things build and run fine.

While here, add a patch to silence a bunch of warnings about
redefining __va_list.

OK?

~Brian

devilutionx-clang10.diff (3K) Download Attachment
Reply | Threaded
Open this post in threaded view
|

Re: Clang 10 FIX: games/devilutionx

Bryan Steele-2
On Fri, Jul 31, 2020 at 02:09:53PM +0000, Brian Callahan wrote:

> Hi ports --
>
> As reported by naddy@, games/devilutionx doesn't build with clang-10.
> The problem is that our ar doesn't understand archives with LLVM thin
> LTO objects in it (devilutionx turns -flto=thin on by default if you
> have a newer toolchain).
>
> The solution is to disable LTO, which makes things build and run fine.
>
> While here, add a patch to silence a bunch of warnings about
> redefining __va_list.
>
> OK?

It probably shouldn't have been enabling LTO automatically in the
first place, so it seems like a good thing to fix regardless of
mysterious clang10 rumblings.

Comments inline.

> ~Brian

> Index: Makefile
> ===================================================================
> RCS file: /cvs/ports/games/devilutionx/Makefile,v
> retrieving revision 1.6
> diff -u -p -r1.6 Makefile
> --- Makefile 16 Mar 2020 23:50:28 -0000 1.6
> +++ Makefile 31 Jul 2020 14:05:16 -0000
> @@ -2,6 +2,7 @@
>  
>  COMMENT = open source engine recreation for Diablo 1 game
>  PKGNAME = ${DISTNAME:L}
> +REVISION = 0
>  CATEGORIES = games x11
>  
>  GH_ACCOUNT = diasurgical
> @@ -37,7 +38,8 @@ NO_TEST = Yes
>  
>  # Remove DOS line endings from patched files
>  post-extract:
> - @cd ${WRKSRC} && perl -i -pe 's/\r$$//' CMakeLists.txt
> + @cd ${WRKSRC} && perl -i -pe 's/\r$$//' CMakeLists.txt \
> + SourceS/macos_stdarg.h
>  
>  # No install target
>  do-install:
> Index: patches/patch-CMakeLists_txt
> ===================================================================
> RCS file: /cvs/ports/games/devilutionx/patches/patch-CMakeLists_txt,v
> retrieving revision 1.5
> diff -u -p -r1.5 patch-CMakeLists_txt
> --- patches/patch-CMakeLists_txt 16 Mar 2020 23:50:28 -0000 1.5
> +++ patches/patch-CMakeLists_txt 31 Jul 2020 14:05:16 -0000
> @@ -1,10 +1,20 @@
>  $OpenBSD: patch-CMakeLists_txt,v 1.5 2020/03/16 23:50:28 bcallah Exp $
>  
> +-DLTO=OFF on the command line doesn't catch.
>  Don't do git here.
>  
>  Index: CMakeLists.txt
>  --- CMakeLists.txt.orig
>  +++ CMakeLists.txt
> +@@ -25,7 +25,7 @@ if(BINARY_RELEASE)
> +   set(ASAN OFF)
> +   set(UBSAN OFF)
> +   set(DEBUG OFF)
> +-  set(LTO ON)
> ++  set(LTO Off)

Should this be "OFF" capitalized to match the rest? Also there's already
a section for OpenBSD overrides cmake options like ASAN/UBSAN. I suppose
it doesn't matter much.

> +   set(DIST ON)
> +   set(FASTER OFF)
> + endif()
>  @@ -40,14 +40,8 @@ if(NIGHTLY_BUILD)
>     set(FASTER OFF)
>   endif()
> Index: patches/patch-SourceS_macos_stdarg_h
> ===================================================================
> RCS file: patches/patch-SourceS_macos_stdarg_h
> diff -N patches/patch-SourceS_macos_stdarg_h
> --- /dev/null 1 Jan 1970 00:00:00 -0000
> +++ patches/patch-SourceS_macos_stdarg_h 31 Jul 2020 14:05:16 -0000
> @@ -0,0 +1,19 @@
> +$OpenBSD$
> +
> +We don't need this.
> +
> +Index: SourceS/macos_stdarg.h
> +--- SourceS/macos_stdarg.h.orig
> ++++ SourceS/macos_stdarg.h
> +@@ -1,9 +1,11 @@
> + #ifndef __STDARG_H
> + #define __STDARG_H
> +
> ++#if 0
> + typedef __builtin_va_list va_list;
> + #define _VA_LIST_T
> + #define va_start(ap, param) __builtin_va_start(ap, param)
> + #define va_end(ap) __builtin_va_end(ap)
> ++#endif
> +
> + #endif /* __STDARG_H */

If we don't need this anymore, then we should remove the check in
SourceS/miniwin.h instead, seems it was working around a bug.

    // work around https://reviews.llvm.org/D51265^M
    #if defined(__APPLE__) || defined(__FreeBSD__) || defined(__OpenBSD__)
    #include "macos_stdarg.h"
    #else
    #include <stdarg.h>
    #endif

It doesn't appear to be needed with clang-8.0.1 either, *shrug*.

ok brynet@ with those changes.

-Bryan.