Bug 238021

Summary: graphics/milton: fix build with GCC-based architectures, please portlint
Product: Ports & Packages Reporter: Piotr Kubaj <pkubaj>
Component: Individual Port(s)Assignee: Alexey Dokuchaev <danfe>
Status: Closed FIXED    
Severity: Affects Only Me CC: danfe, linimon
Priority: --- Flags: bugzilla: maintainer-feedback? (danfe)
Version: Latest   
Hardware: Any   
OS: Any   
Attachments:
Description Flags
patch
pkubaj: maintainer-approval? (danfe)
patch to graphics/milton/Makefile none

Description Piotr Kubaj freebsd_committer freebsd_triage 2019-05-21 12:53:10 UTC
Created attachment 204508 [details]
patch

Move USE_GITHUB and INSTALLS_ICONS to please portlint.

Add -DNO_WARN_X86_INTRINSICS -maltivec -mvsx to CXXFLAGS on powerpc64 to use the new Intel SIMD software translation mode in GCC.

Add ifdefs for a couple of E* macros to files/patch-src_shadergen.cc.

Errors:
/wrkdirs/usr/ports/graphics/milton/work/milton-1.6.0/src/shadergen.cc:364:13: error: 'ENODATA' was not declared in this scope
        case ENODATA:         str = "No message is available on the STREAM head read queue (POSIX.1)"; break;
             ^~~~~~~
/wrkdirs/usr/ports/graphics/milton/work/milton-1.6.0/src/shadergen.cc:364:13: note: suggested alternative: 'ENOTTY'
        case ENODATA:         str = "No message is available on the STREAM head read queue (POSIX.1)"; break;
             ^~~~~~~
             ENOTTY
Comment 1 Mark Linimon freebsd_committer freebsd_triage 2019-05-29 21:27:52 UTC
This builds for me, so once the maintainer approves, consider this mentor approval.
Comment 2 Alexey Dokuchaev freebsd_committer freebsd_triage 2019-06-04 15:53:59 UTC
Thanks for the patch.  Let me see if I can reproduce the errors on my Mac mini G4 here (I'm particularly curious about those EFOOBAR #ifdef's).

> Move USE_GITHUB and INSTALLS_ICONS to please portlint.
Actually, portlint(1) warning is bogus here: GitHub-related (that is, fetch-related) knobs are deliberately grouped together.
Comment 3 Piotr Kubaj freebsd_committer freebsd_triage 2019-06-04 15:59:52 UTC
(In reply to Alexey Dokuchaev from comment #2)
Just note that you won't be able to run the binary that is compiled with this patch.

VSX was introduced in POWER8. However there's currently no way to set IGNORE depending on the POWER CPU generation.
Comment 4 Mark Linimon freebsd_committer freebsd_triage 2019-06-21 11:40:05 UTC
Created attachment 205260 [details]
patch to graphics/milton/Makefile

Here's the slightly different patch that I tested.
Comment 5 Alexey Dokuchaev freebsd_committer freebsd_triage 2019-06-21 11:44:19 UTC
I'm a bit reluctant to pull heavy stuff like LLVM, which can be a pain on low-end hardware like my Mac mini G4, I need to think more about this.
Comment 6 Piotr Kubaj freebsd_committer freebsd_triage 2019-06-21 11:50:03 UTC
(In reply to Mark Linimon from comment #4)
Why does using LLVM here help?
Comment 7 Alexey Dokuchaev freebsd_committer freebsd_triage 2019-06-24 09:33:37 UTC
I suspect because of unguarded inclusion of x86-specific headers in src/system_includes.h:

> #include <xmmintrin.h>
> #include <emmintrin.h>
... and pulling LLVM/Clang allows to handle them on !x86.  This, of course, is very bogus solution.

No more patches are needed at this point, I'm on it with my Mac mini G4. :-)
Comment 8 commit-hook freebsd_committer freebsd_triage 2019-06-24 10:59:16 UTC
A commit references this bug:

Author: danfe
Date: Mon Jun 24 10:58:22 UTC 2019
New revision: 505022
URL: https://svnweb.freebsd.org/changeset/ports/505022

Log:
  - Update `graphics/milton' to version 1.6.2
  - It's written in C++11, so pull the right compiler
  - Don't try to decode errnos which are never returned on FreeBSD
    and thus might not be defined with some compilers
  - Fix the build on !x86 by removing unused intrinsic headers
  - Add some missing USE_* components reported by stage Q/A

  PR:	238021

Changes:
  head/graphics/milton/Makefile
  head/graphics/milton/distinfo
  head/graphics/milton/files/patch-src_system__includes.h
Comment 9 Alexey Dokuchaev freebsd_committer freebsd_triage 2019-06-24 11:08:54 UTC
OK, now lets talk more about AltiVec, VSX, and POWER8.  Piotr, I have a few questions:

1) What is -DNO_WARN_X86_INTRINSICS for and why do we need it?  Is it still needed after ports r505022?

2) Unfortunately, the program segfaults for me on powerpc32 even without -maltivec -mvsx, so I cannot test it.  Did you observe some real, measurable benefit when building it with these options?

3) You've said that VSX requires POWER8.  But AltiVec doesn't, right?  Also, AltiVec is available on PPC32.  This suggests the following:

> CXXFLAGS_powerpc=	-maltivec
> CXXFLAGS_powerpc64=	-maltivec -mvsx
To make sure that generated packages are working on pre-POWER8 64-bit machines (like Apple's G5), these can be controlled by the option which is off by default.  Unfortunately, as I'm mentioned, I cannot do proper run-time tests, so I might need some help here.
Comment 10 Piotr Kubaj freebsd_committer freebsd_triage 2019-06-24 11:16:30 UTC
(In reply to Alexey Dokuchaev from comment #9)
It looks like removing includes fixed it and this patch is not necessary now.
Comment 11 Alexey Dokuchaev freebsd_committer freebsd_triage 2019-06-24 11:43:33 UTC
Thanks.  I've reported this upstream: https://github.com/serge-rgb/milton/issues/125.