Bug 235933 - audio/giada: fix build with GCC-based architectures, don't assume building on FreeBSD
Summary: audio/giada: fix build with GCC-based architectures, don't assume building on...
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: Yuri Victorovich
URL:
Keywords:
Depends on:
Blocks:
 
Reported: 2019-02-22 10:50 UTC by Piotr Kubaj
Modified: 2019-02-23 18:28 UTC (History)
2 users (show)

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

Attachments
patch (1.33 KB, patch)
2019-02-22 10:50 UTC, Piotr Kubaj 		no flags Details | Diff
View All Add an attachment (proposed patch, testcase, etc.)

Note You need to log in before you can comment on or make changes to this bug.
Description Piotr Kubaj freebsd_committer freebsd_triage 2019-02-22 10:50:09 UTC 
Created attachment 202258 [details]
patch

Building with GCC requires including atomic in src/core/init.cpp.

While here, also don't assume building on FreeBSD.

I know I sent a patch for this port to fix build on GCC before, but it was before the switch to GCC 8, so that's why it built previously.

Tested on powerpc64 and amd64.

Hardware sponsored by IntegriCloud.
Comment 1 Yuri Victorovich freebsd_committer freebsd_triage 2019-02-23 17:20:07 UTC 
Committed the Makefile part.
The patch is wrong, it introduces the third instance of #include <atomic> in their code.

Thanks for your submission!
Comment 2 commit-hook freebsd_committer freebsd_triage 2019-02-23 17:20:12 UTC 
A commit references this bug:

Author: yuri
Date: Sat Feb 23 17:19:11 UTC 2019
New revision: 493713
URL: https://svnweb.freebsd.org/changeset/ports/493713

Log:
  audio/giada: fix build with GCC-based architectures, don't assume building on FreeBSD

  PR:		235933
  Submitted by:	Piotr Kubaj <pkubaj@anongoth.pl>

Changes:
  head/audio/giada/Makefile
Comment 3 Piotr Kubaj freebsd_committer freebsd_triage 2019-02-23 17:51:45 UTC 
What do you mean?
root@talos:$/usr/ports/audio/giada$ grep atomic work/giada-0.15.3/src/core/init.cpp
#include <atomic>
extern std::atomic<bool> G_quit;

This is with my patch.
Comment 4 Yuri Victorovich freebsd_committer freebsd_triage 2019-02-23 18:09:57 UTC 
(In reply to Piotr Kubaj from comment #3)

There are 2 atomic includes there: https://github.com/monocasual/giada/blob/master/src/core/init.cpp#L31

I created the PR to remove one: https://github.com/monocasual/giada/pull/242
Comment 5 Yuri Victorovich freebsd_committer freebsd_triage 2019-02-23 18:11:09 UTC 
Also, you should not create port patches that belong in the upstream code.
Comment 6 Yuri Victorovich freebsd_committer freebsd_triage 2019-02-23 18:17:39 UTC 
(In reply to Piotr Kubaj from comment #3)

It looks like they modified the place that you did after that release.
Unfortunately, they didn't merge the FreeBSD compatibility pull request properly, and now the port can't be updated due to conflicts.
Comment 7 Piotr Kubaj freebsd_committer freebsd_triage 2019-02-23 18:21:18 UTC 
(In reply to Yuri Victorovich from comment #4)
You're right, the upstream code has duplicate includes, but the version in ports doesn't have any include. So I'm not introducing duplicate includes.
Comment 8 Yuri Victorovich freebsd_committer freebsd_triage 2019-02-23 18:24:41 UTC 
The upstream is uncooperative, and wouldn't properly merge the patch.
I'll probably just remove this port due to the excessive maintanence requirement.
Comment 9 commit-hook freebsd_committer freebsd_triage 2019-02-23 18:28:07 UTC 
A commit references this bug:

Author: yuri
Date: Sat Feb 23 18:27:57 UTC 2019
New revision: 493723
URL: https://svnweb.freebsd.org/changeset/ports/493723

Log:
  audio/giada: fix build with GCC-based architectures, don't assume building on FreeBSD

  PR:		235933
  Submitted by:	Piotr Kubaj <pkubaj@anongoth.pl>

Changes:
  head/audio/giada/files/
  head/audio/giada/files/patch-src_core_init.cpp
Comment 10 Yuri Victorovich freebsd_committer freebsd_triage 2019-02-23 18:28:16 UTC 
I added the patch, thanks!