Bug 200028 - graphics/lensfun: fails to build on armv6
Summary: graphics/lensfun: fails to build on armv6
Status: Closed FIXED
Alias: None
Product: Ports & Packages
Classification: Unclassified
Component: Individual Port(s) (show other bugs)
Version: Latest
Hardware: arm Any
: --- Affects Only Me
Assignee: Alexey Dokuchaev
URL: http://chips.ysv.freebsd.org/data/11a...
Keywords:
Depends on: 205006
Blocks:
  Show dependency treegraph
 
Reported: 2015-05-07 16:05 UTC by mikael.urankar
Modified: 2015-12-04 05:04 UTC (History)
4 users (show)

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


Attachments
fix arm build (373 bytes, patch)
2015-05-07 16:05 UTC, mikael.urankar
no flags Details | Diff
fix arm build (366 bytes, patch)
2015-05-08 14:07 UTC, mikael.urankar
no flags Details | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description mikael.urankar 2015-05-07 16:05:16 UTC
Created attachment 156470 [details]
fix arm build

Hi,

There are no sse/sse2 instructions on arm.

build log:
http://mikael.urankar.free.fr/FreeBSD/arm/build_logs/lensfun-0.3.0.log
Comment 1 Sean Bruno freebsd_committer 2015-05-07 16:10:55 UTC
I think the proper check is to do something along the lines of:

.if empty(MACHINE_CPU:Msse2)
CMAKE_ARGS+=	-DBUILD_FOR_SSE2:BOOL=OFF -DBUILD_FOR_SSE:BOOL=OFF
.endif


Does this work for you?
Comment 3 Sean Bruno freebsd_committer 2015-05-08 14:09:26 UTC
Looks good to me.  I'll commit if danfe has no objections.
Comment 4 Alexey Dokuchaev freebsd_committer 2015-05-13 01:28:00 UTC
LGTM.  I would've probably changed "empty(...)" into "! ${...}" but that's up to you.  Just make sure there are no whitespace bugs when committing, thanks!
Comment 5 commit-hook freebsd_committer 2015-11-28 02:51:40 UTC
A commit references this bug:

Author: mmoll
Date: Sat Nov 28 02:50:41 UTC 2015
New revision: 402504
URL: https://svnweb.freebsd.org/changeset/ports/402504

Log:
  graphics/lensfun: fix build on arm

  PR:		200028
  Submitted by:	Mikael Urankar <mikael.urankar@gmail.com>
  Approved by:	danfe (maintainer)

Changes:
  head/graphics/lensfun/Makefile
Comment 6 Michael Moll freebsd_committer 2015-11-28 02:52:11 UTC
committed, thanks!
Comment 7 Jan Beich freebsd_committer 2015-11-28 17:40:05 UTC
Comment on attachment 156510 [details]
fix arm build

>+.if empty(MACHINE_CPU:Msse2)
>+CMAKE_ARGS+=	-DBUILD_FOR_SSE2:BOOL=OFF

Don't forget to bump PORTREVISION as you're pessimizing i386 build by default (no CPUTYPE -> no SSE). I've sent upstream better version:

  IF(CMAKE_SYSTEM_PROCESSOR MATCHES "[XxIi][0-9]?86|[Aa][Mm][Dd]64")
    SET(X86_ON ON)
  else()
    SET(X86_ON OFF)
  ENDIF()
  ...
  OPTION(BUILD_FOR_SSE "Build with support for SSE" ${X86_ON})
  OPTION(BUILD_FOR_SSE2 "Build with support for SSE2" ${X86_ON})

http://sourceforge.net/p/lensfun/mailman/message/34654903/
Comment 8 commit-hook freebsd_committer 2015-11-28 17:53:48 UTC
A commit references this bug:

Author: mmoll
Date: Sat Nov 28 17:53:18 UTC 2015
New revision: 402552
URL: https://svnweb.freebsd.org/changeset/ports/402552

Log:
  graphics/lensfun: bump PORTREVISION

  r402504 did effectively change the resulting binary for default i386 builds

  PR:		200028
  Submitted by:	jbeich

Changes:
  head/graphics/lensfun/Makefile
Comment 9 Michael Moll freebsd_committer 2015-11-28 17:54:24 UTC
thanks Jan!
Comment 10 Alexey Dokuchaev freebsd_committer 2015-11-29 03:09:16 UTC
> Don't forget to bump PORTREVISION as you're pessimizing i386 build by default (no CPUTYPE -> no SSE).

PORTREVISION bump was pointless, and there's no pessimization; users expecting any SSE code in their binaries should have set CPUTYPE in their /etc/make.conf long type ago.  We (correctly) produce 32-bit packages for i486 by default.

> I've sent upstream better version: [...]

I don't see much value in this code.  Even if it will be accepted, checking CPUTYPE would have to stay since not all i386 CPUs support SSE.
Comment 11 Jan Beich freebsd_committer 2015-12-04 05:04:23 UTC
(In reply to Alexey Dokuchaev from comment #10)
> users expecting any SSE code in their binaries should have set CPUTYPE in their /etc/make.conf long type ago.

Did you check the source? lensfun uses CPUID, or was until it was broken by cmake conversion which forced -msse for all files, not just those using SSE intrinsics.

Don't forget package-only users often cannot rebuild ports either because they have old hardware (that may still support SSE), bandwidth restrictions, broken environment, etc. In such cases runtime CPU detection often helps to satisfy wider range of users.

> We (correctly) produce 32-bit packages for i486 by default.

Why do you assume I disagree? ;)

> Even if it will be accepted, checking CPUTYPE would have to stay since not all i386 CPUs support SSE.

That wasn't the only fix I've sent upstream. While CPU may not support SSE, the toolchain must.

Let's move to bug 205006.