Bug 200028

Summary: graphics/lensfun: fails to build on armv6
Product: Ports & Packages Reporter: Mikael Urankar <mikael>
Component: Individual Port(s)Assignee: Alexey Dokuchaev <danfe>
Status: Closed FIXED    
Severity: Affects Only Me CC: freebsd-arm, jbeich, mmoll, sbruno
Priority: --- Flags: bugzilla: maintainer-feedback? (danfe)
Version: Latest   
Hardware: arm   
OS: Any   
URL: http://chips.ysv.freebsd.org/data/11armv6-default/2015-05-04_04h43m53s/logs/errors/lensfun-0.3.0.log
Bug Depends on: 205006    
Bug Blocks:    
Attachments:
Description Flags
fix arm build
none
fix arm build none

Description Mikael Urankar freebsd_committer freebsd_triage 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 freebsd_triage 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 freebsd_triage 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 freebsd_triage 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 freebsd_triage 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 freebsd_triage 2015-11-28 02:52:11 UTC 
committed, thanks!
Comment 7 Jan Beich freebsd_committer freebsd_triage 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 freebsd_triage 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 freebsd_triage 2015-11-28 17:54:24 UTC 
thanks Jan!
Comment 10 Alexey Dokuchaev freebsd_committer freebsd_triage 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 freebsd_triage 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.