Bug 238021 - graphics/milton: fix build with GCC-based architectures, please portlint
Summary: graphics/milton: fix build with GCC-based architectures, please portlint
Status: Closed FIXED
Alias: None
Product: Ports & Packages
Classification: Unclassified
Component: Individual Port(s) (show other bugs)
Version: Latest
Hardware: Any Any
: --- Affects Only Me
Assignee: Alexey Dokuchaev
Depends on:
Reported: 2019-05-21 12:53 UTC by Piotr Kubaj
Modified: 2019-06-24 11:43 UTC (History)
2 users (show)

See Also:
bugzilla: maintainer-feedback? (danfe)

patch (2.42 KB, patch)
2019-05-21 12:53 UTC, Piotr Kubaj
pkubaj: maintainer-approval? (danfe)
Details | Diff
patch to graphics/milton/Makefile (901 bytes, patch)
2019-06-21 11:40 UTC, Mark Linimon
no flags Details | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Piotr Kubaj freebsd_committer 2019-05-21 12:53:10 UTC
Created attachment 204508 [details]

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.

/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;
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 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 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 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 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 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 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

  - 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

Comment 9 Alexey Dokuchaev freebsd_committer 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 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 2019-06-24 11:43:33 UTC
Thanks.  I've reported this upstream: https://github.com/serge-rgb/milton/issues/125.