Bug 204858 - audio/libkcompactdisc build fails, when ALSA-option is selected
Summary: audio/libkcompactdisc build fails, when ALSA-option is selected
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: freebsd-kde (Team)
URL:
Keywords:
Depends on:
Blocks:
 
Reported: 2015-11-28 04:41 UTC by Mikhail Teterin
Modified: 2015-11-29 19:28 UTC (History)
0 users

See Also:


Attachments
Build-log (3.12 KB, text/plain)
2015-11-28 04:41 UTC, Mikhail Teterin
no flags Details

Note You need to log in before you can comment on or make changes to this bug.
Description Mikhail Teterin freebsd_committer 2015-11-28 04:41:34 UTC
Created attachment 163602 [details]
Build-log

Somehow clang gets invoked with such options, that ``inline'' becomes an unrecognised verb (std89?). See attached log.
Comment 1 Raphael Kubo da Costa freebsd_committer 2015-11-28 13:05:33 UTC
Can you post your full, verbose build output? I tried reproducing it here but got different errors caused by alsa/asoundlib.h not being detected by CMake and then not being included in the source files.
Comment 2 Raphael Kubo da Costa freebsd_committer 2015-11-28 13:11:25 UTC
OK, building as root found asoundlib.h and I've managed to reproduce the issue. It's possible that this has been broken for a while and just went unnoticed because the ALSA option is not on by default.

libkcompactdisc's CMakeLists.txt has this excerpt:

if(CMAKE_COMPILER_IS_GNUCXX)
    set(CMAKE_C_FLAGS "-std=c99")   ## ALSA no longer compiles with -std=c90, see https://bugzilla.novell.com/show_bug.cgi?id=817077
endif()

which only works for GCC. The condition needs to be extended to clang or there needs to be a call to change the default flags (I need to look for one).
Comment 3 commit-hook freebsd_committer 2015-11-28 13:38:16 UTC
A commit references this bug:

Author: rakuco
Date: Sat Nov 28 13:37:47 UTC 2015
New revision: 402530
URL: https://svnweb.freebsd.org/changeset/ports/402530

Log:
  Add upstream patch to fix the build with clang + ALSA option.

  Extend the compiler check so that the code is built with -std=c99, which is
  required by the ALSA headers. Bump PORTREVISION because this also changes
  the build flags for GCC users.

  PR:		204858
  MFH:		2015Q4

Changes:
  head/audio/libkcompactdisc/Makefile
  head/audio/libkcompactdisc/files/patch-git_d6927712
Comment 4 Mikhail Teterin freebsd_committer 2015-11-28 15:26:18 UTC
(In reply to commit-hook from comment #3)
> Bump PORTREVISION because this also changes
> the build flags for GCC users.

GCC or not, the change should only have affected people with ALSA-option -- which is not on by default. I don't think, PORTREVISION bump was warranted -- millions of users, who used the port without ALSA, will now be rebuilding it for no good reason whatsoever...

Too late to undo, of course.
Comment 5 commit-hook freebsd_committer 2015-11-28 16:52:43 UTC
A commit references this bug:

Author: rakuco
Date: Sat Nov 28 16:51:41 UTC 2015
New revision: 402545
URL: https://svnweb.freebsd.org/changeset/ports/402545

Log:
  Fix the ALSA=OFF build after r402530.

  Both extrapatch-no_alsa and patch-git_d6927712 touch the same region of
  CMakeLists.txt, and the latter was failing to apply when the ALSA option was
  disabled.

  Fix it by keeping patch-git_d6927712 (which fixes the CFLAGS values
  regardless of the value of the ALSA option) and replacing extrapatch-no_alsa
  with some post-patch/post-configure changes that achieve the same end
  result.

  PR:		204858
  MFH:		2015Q4

Changes:
  head/audio/libkcompactdisc/Makefile
  head/audio/libkcompactdisc/files/extrapatch-no_alsa
Comment 6 Raphael Kubo da Costa freebsd_committer 2015-11-28 16:56:15 UTC
(In reply to Mikhail Teterin from comment #4)
> GCC or not, the change should only have affected people with ALSA-option

I disagree. Upstream was overriding CFLAGS wrongly, and this is independent of our ALSA option and patches. Changes in compiler flags warrant a PORTREVISION bump.
Comment 7 Mikhail Teterin freebsd_committer 2015-11-28 17:04:38 UTC
(In reply to Raphael Kubo da Costa from comment #6)
> Changes in compiler flags warrant a PORTREVISION bump.

The question to ask ourselves in such a situation is (or ought to be): would an existing user with the port already installed benefit from rebuilding it anew?
Comment 8 commit-hook freebsd_committer 2015-11-29 19:28:37 UTC
A commit references this bug:

Author: rakuco
Date: Sun Nov 29 19:27:44 UTC 2015
New revision: 402617
URL: https://svnweb.freebsd.org/changeset/ports/402617

Log:
  MFH: r402530 r402545

  Add upstream patch to fix the build with clang + ALSA option.

  Extend the compiler check so that the code is built with -std=c99, which is
  required by the ALSA headers. Bump PORTREVISION because this also changes
  the build flags for GCC users.

  PR:		204858

  Fix the ALSA=OFF build after r402530.

  Both extrapatch-no_alsa and patch-git_d6927712 touch the same region of
  CMakeLists.txt, and the latter was failing to apply when the ALSA option was
  disabled.

  Fix it by keeping patch-git_d6927712 (which fixes the CFLAGS values
  regardless of the value of the ALSA option) and replacing extrapatch-no_alsa
  with some post-patch/post-configure changes that achieve the same end
  result.

  PR:		204858

  Approved by:	ports-secteam (feld)

Changes:
_U  branches/2015Q4/
  branches/2015Q4/audio/libkcompactdisc/Makefile
  branches/2015Q4/audio/libkcompactdisc/files/extrapatch-no_alsa
  branches/2015Q4/audio/libkcompactdisc/files/patch-git_d6927712
Comment 9 commit-hook freebsd_committer 2015-11-29 19:28:38 UTC
A commit references this bug:

Author: rakuco
Date: Sun Nov 29 19:27:44 UTC 2015
New revision: 402617
URL: https://svnweb.freebsd.org/changeset/ports/402617

Log:
  MFH: r402530 r402545

  Add upstream patch to fix the build with clang + ALSA option.

  Extend the compiler check so that the code is built with -std=c99, which is
  required by the ALSA headers. Bump PORTREVISION because this also changes
  the build flags for GCC users.

  PR:		204858

  Fix the ALSA=OFF build after r402530.

  Both extrapatch-no_alsa and patch-git_d6927712 touch the same region of
  CMakeLists.txt, and the latter was failing to apply when the ALSA option was
  disabled.

  Fix it by keeping patch-git_d6927712 (which fixes the CFLAGS values
  regardless of the value of the ALSA option) and replacing extrapatch-no_alsa
  with some post-patch/post-configure changes that achieve the same end
  result.

  PR:		204858

  Approved by:	ports-secteam (feld)

Changes:
_U  branches/2015Q4/
  branches/2015Q4/audio/libkcompactdisc/Makefile
  branches/2015Q4/audio/libkcompactdisc/files/extrapatch-no_alsa
  branches/2015Q4/audio/libkcompactdisc/files/patch-git_d6927712