Bug 234216 - net/bredbandskollen: fix build with GCC-based architectures
Summary: net/bredbandskollen: fix build with GCC-based architectures
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: Mark Linimon
URL:
Keywords:
Depends on:
Blocks:
 
Reported: 2018-12-20 14:45 UTC by Piotr Kubaj
Modified: 2019-07-09 19:05 UTC (History)
3 users (show)

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


Attachments
patch (2.36 KB, patch)
2018-12-20 14:45 UTC, Piotr Kubaj
no flags Details | Diff
v2 (2.71 KB, patch)
2018-12-27 12:34 UTC, Piotr Kubaj
no flags Details | Diff
v3 (2.67 KB, patch)
2019-02-21 18:58 UTC, Piotr Kubaj
no flags Details | Diff
bredbandskollen update (1.08 KB, patch)
2019-07-05 17:03 UTC, Niclas Zeising
no flags Details | Diff
Update bredbandskollen and fix build with gcc (2.71 KB, patch)
2019-07-05 19:13 UTC, Niclas Zeising
no flags Details | Diff
Update bredbandskollen and fix build with gcc (340.48 KB, patch)
2019-07-06 21:33 UTC, Niclas Zeising
no flags Details | Diff
Update bredbandskollen and fix build with gcc (2.79 KB, patch)
2019-07-06 21:35 UTC, Niclas Zeising
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 2018-12-20 14:45:55 UTC
Created attachment 200304 [details]
patch

This port needs USES=compiler:c++11-lang to build on GCC architectures. It also specifies directly CXX with =, replace that with ?=.

Add missing includes (sys/types.h and sys/select.h) that GCC doesn't include by default.

Tested on powerpc64 and amd64.

Hardware sponsored by IntegriCloud.
Comment 1 Niclas Zeising freebsd_committer 2018-12-20 20:04:48 UTC
Any reason not to do the CXX= -> CXX?= for all architectures?
Comment 2 Piotr Kubaj freebsd_committer 2018-12-27 12:34:41 UTC
Created attachment 200558 [details]
v2

Also fix double https in pkg-descr.
Comment 3 Niclas Zeising freebsd_committer 2019-02-21 18:32:15 UTC
Hi!
What is the reason for the extra includes?
Comment 4 Piotr Kubaj freebsd_committer 2019-02-21 18:58:57 UTC
Created attachment 202235 [details]
v3

Sorry for the lag, I just forgot about this PR.
CXX can be applied everywhere.

Extra includes are needed because GCC doesn't include them by default, which results in build failure.
Comment 5 Niclas Zeising freebsd_committer 2019-02-21 19:14:11 UTC
This feels a little strange.  I'm surprised if clang automatically includes them.  Could be differences in libc++ and GCC's counterpart perhaps?
Comment 6 Piotr Kubaj freebsd_committer 2019-02-21 19:24:08 UTC
(In reply to Niclas Zeising from comment #5)
It's probable, I'm not really sure (I'm neither LLVM nor GCC developer).

But I noticed it happened also with other ports. Quite many ports need including sys/types.h when compiling with GCC.
Comment 7 Niclas Zeising freebsd_committer 2019-02-21 19:34:55 UTC
(In reply to Piotr Kubaj from comment #6)

Are they using the C or C++ compiler?
Comment 8 Piotr Kubaj freebsd_committer 2019-02-21 19:52:32 UTC
(In reply to Niclas Zeising from comment #7)
I don't really remember, I didn't pay attention to that.
Comment 9 commit-hook freebsd_committer 2019-05-29 07:36:39 UTC
A commit references this bug:

Author: pkubaj
Date: Wed May 29 07:35:45 UTC 2019
New revision: 502934
URL: https://svnweb.freebsd.org/changeset/ports/502934

Log:
  net/bredbandskollen: fix build with GCC-based architectures, fix pkg-descr

  This port needs USES=compiler:c++11-lang to build on GCC architectures. It also specifies directly CXX with =, replace that with ?=.

  Add missing includes (sys/types.h and sys/select.h) that GCC doesn't include by default.

  Also fix double https in pkg-descr.

  PR:		234216
  Approved by:	tcberner (mentor), zeising (maintainer timeout)
  Differential Revision:	https://reviews.freebsd.org/D20442

Changes:
  head/net/bredbandskollen/Makefile
  head/net/bredbandskollen/files/
  head/net/bredbandskollen/files/patch-src_framework_engine.h
  head/net/bredbandskollen/files/patch-src_framework_socketreceiver.h
  head/net/bredbandskollen/pkg-descr
Comment 10 Niclas Zeising freebsd_committer 2019-05-29 08:24:31 UTC
Hi!
This was not maintainer timeout.
I asked questions from the sumbitter, for which I did not get an answer. Please revert it.

The patch is wrong, since bredbandkskollen does not use C++, which I asked about, but you never replied.
Comment 11 Piotr Kubaj freebsd_committer 2019-05-29 08:37:22 UTC
(In reply to Niclas Zeising from comment #10)
You asked about other ports and what programming language they use, which I answered.

I don't really make any statistics about which ports that fail use which programming language, why would I?

If the port doesn't use C++, what does it use $CXX variable for?

Why do I see in failed build logs lines like:
c++ -MM -O2 -pipe -fno-strict-aliasing   -O2 -W -Wall -I.. -std=c++11 -DTASKRUNNER_LOGERR -DTASKRUNNER_LOGWARN -DTASKRUNNER_LOGINFO -MT ../http/websocketbridge.o ../http/websocketbridge.cpp > ../http/websocketbridge.d
c++ -MM -O2 -pipe -fno-strict-aliasing   -O2 -W -Wall -I.. -std=c++11 -DTASKRUNNER_LOGERR -DTASKRUNNER_LOGWARN -DTASKRUNNER_LOGINFO -MT ../http/httprequestengine.o ../http/httprequestengine.cpp > ../http/httprequestengine.d
c++ -MM -O2 -pipe -fno-strict-aliasing   -O2 -W -Wall -I.. -std=c++11 -DTASKRUNNER_LOGERR -DTASKRUNNER_LOGWARN -DTASKRUNNER_LOGINFO -MT ../http/httpserverconnection.o ../http/httpserverconnection.cpp > ../http/httpserverconnection.d
cc1plus: error: unrecognized command line option "-std=c++11"cc1plus: error: unrecognized command line option "-std=c++11"

cc1plus: error: unrecognized command line option "-std=c++11"
cc1plus: error: unrecognized command line option "-std=c++11"

As you can see, it does use C++.

Here's a build log from pylon: http://pylon.nyi.freebsd.org/data/head-powerpc64-default/p502184_s348025/logs/errors/bredbandskollen-0.20181210.log

It fails because of no C++11 as well.

And as it happens, my patch also fixes the build issues.
Comment 12 Niclas Zeising freebsd_committer 2019-05-29 10:47:13 UTC
Hi!
You sent the patch, you claimed build errors (without showing a log, or even snippet of a log), so it must be on you to answer questions and defend the patch.  The only reply I got from you was a "I don't remember", in comment #8.

There is definitely something weird going on here, especially sine bredbandskollen is presumably compiled with gcc on other platforms.  I ask of you to revert this, since it was committed  in error, and the patch might not be correct.
Comment 13 Piotr Kubaj freebsd_committer 2019-05-29 11:04:49 UTC
(In reply to Niclas Zeising from comment #12)
I replied "I don't know" because why would I remember about what programming language ports that fail to build are written in? This is simply not relevant.
Other than that, I answered all your questions according to my knowledge, but you still didn't answer.

Also, it's not only that GCC architectures use c++.

Clang architectures do it as well (poudriere testport with my patch):
===>  Building for bredbandskollen-0.20181210
gmake[1]: Entering directory '/wrkdirs/usr/ports/net/bredbandskollen/work/bbk-39b47a1/src/cli'
c++ -MM -O2 -pipe -fstack-protector-all -fno-strict-aliasing   -O2 -W -Wall -I.. -std=c++11 -DTASKRUNNER_LOGERR -DTASKRUNNER_LOGWARN -DTASKRUNNER_LOGINFO -MT ../framework/loadbalancer.o ../framework/loadbalancer.cpp > ../framework/loadbalancer.d
c++ -MM -O2 -pipe -fstack-protector-all -fno-strict-aliasing   -O2 -W -Wall -I.. -std=c++11 -DTASKRUNNER_LOGERR -DTASKRUNNER_LOGWARN -DTASKRUNNER_LOGINFO -MT ../framework/synchronousbridge.o ../framework/synchronousbridge.cpp > ../framework/synchronousbridge.d
c++ -MM -O2 -pipe -fstack-protector-all -fno-strict-aliasing   -O2 -W -Wall -I.. -std=c++11 -DTASKRUNNER_LOGERR -DTASKRUNNER_LOGWARN -DTASKRUNNER_LOGINFO -MT ../framework/bridgetask.o ../framework/bridgetask.cpp > ../framework/bridgetask.d
c++ -MM -O2 -pipe -fstack-protector-all -fno-strict-aliasing   -O2 -W -Wall -I.. -std=c++11 -DTASKRUNNER_LOGERR -DTASKRUNNER_LOGWARN -DTASKRUNNER_LOGINFO -MT ../framework/logger.o ../framework/logger.cpp > ../framework/logger.d
c++ -MM -O2 -pipe -fstack-protector-all -fno-strict-aliasing   -O2 -W -Wall -I.. -std=c++11 -DTASKRUNNER_LOGERR -DTASKRUNNER_LOGWARN -DTASKRUNNER_LOGINFO -MT ../framework/socketreceiver.o ../framework/socketreceiver.cpp > ../framework/socketreceiver.d
c++ -MM -O2 -pipe -fstack-protector-all -fno-strict-aliasing   -O2 -W -Wall -I.. -std=c++11 -DTASKRUNNER_LOGERR -DTASKRUNNER_LOGWARN -DTASKRUNNER_LOGINFO -MT ../framework/serversocket.o ../framework/serversocket.cpp > ../framework/serversocket.d
c++ -MM -O2 -pipe -fstack-protector-all -fno-strict-aliasing   -O2 -W -Wall -I.. -std=c++11 -DTASKRUNNER_LOGERR -DTASKRUNNER_LOGWARN -DTASKRUNNER_LOGINFO -MT ../framework/socketconnection.o ../framework/socketconnection.cpp > ../framework/socketconnection.d
c++ -MM -O2 -pipe -fstack-protector-all -fno-strict-aliasing   -O2 -W -Wall -I.. -std=c++11 -DTASKRUNNER_LOGERR -DTASKRUNNER_LOGWARN -DTASKRUNNER_LOGINFO -MT ../framework/socket.o ../framework/socket.cpp > ../framework/socket.d

Since the patch is ok and you didn't replied in 3 months, I fail to see why I would revert it.
Comment 14 Mark Linimon freebsd_committer freebsd_triage 2019-05-29 12:20:00 UTC
(In reply to Piotr Kubaj from comment #13)

You have to do the revert -- because the maintainer said so.

There is obviously some kind of communication problem here.  Let's take this conversation offline, please.
Comment 15 commit-hook freebsd_committer 2019-05-29 12:38:01 UTC
A commit references this bug:

Author: pkubaj
Date: Wed May 29 12:37:15 UTC 2019
New revision: 502955
URL: https://svnweb.freebsd.org/changeset/ports/502955

Log:
  net/bredbandskollen: revert 502934

  PR:		234216
  Submitted by:	zeising

Changes:
  head/net/bredbandskollen/Makefile
  head/net/bredbandskollen/files/
  head/net/bredbandskollen/pkg-descr
Comment 16 Niclas Zeising freebsd_committer 2019-05-30 08:17:45 UTC
After looking some more at this, I have more comments.

I did some tests with gcc8 on amd64, and I can see the need for sys/select.h in framework/engine.h, however, I can see no need to include sys/types.h in framework/socketreceiver.h.  Bredbandskollen builds fine without that patch, and I can't see anything in that file that warrants that inclusion.

Can you give me a snip of a log for that error?
Thanks!
Comment 17 Niclas Zeising freebsd_committer 2019-05-30 08:24:44 UTC
Just to be clear, since there's no flag in bugzilla for it, I'm waiting for feedback from submitter.
Comment 18 Piotr Kubaj freebsd_committer 2019-05-31 12:43:23 UTC
(In reply to Niclas Zeising from comment #17)
g++8 -O2 -pipe  -fstack-protector-strong -Wl,-rpath=/usr/local/lib/gcc8  -Wl,-rpath=/usr/local/lib/gcc8  -O2 -W -Wall -I.. -std=c++11 -DTASKRUNNER_LOGERR -DTASKRUNNER_LOGWARN -DTASKRUNNER_LOGINFO -c ../framework/socketreceiver.cpp -o ../framework/socketreceiver.o
In file included from /usr/include/sys/socket.h:41,
                 from ../framework/socketreceiver.h:6,
                 from ../framework/socketreceiver.cpp:6:
../framework/socketreceiver.h:36:18: error: 'register_t' was not declared in this scope
     char cmsgbuf[CMSG_SPACE(sizeof(int))];
                  ^~~~~~~~~~
../framework/socketreceiver.h:36:18: note: suggested alternative: 'register'
../framework/socketreceiver.h:36:18: error: 'register_t' was not declared in this scope
     char cmsgbuf[CMSG_SPACE(sizeof(int))];
                  ^~~~~~~~~~
../framework/socketreceiver.h:36:18: note: suggested alternative: 'register'
../framework/socketreceiver.h:36:18: error: 'register_t' was not declared in this scope
     char cmsgbuf[CMSG_SPACE(sizeof(int))];
                  ^~~~~~~~~~
../framework/socketreceiver.h:36:18: note: suggested alternative: 'register'
../framework/socketreceiver.h:36:18: error: 'register_t' was not declared in this scope
     char cmsgbuf[CMSG_SPACE(sizeof(int))];
                  ^~~~~~~~~~
../framework/socketreceiver.h:36:18: note: suggested alternative: 'register'
../framework/socketreceiver.cpp: In constructor 'SocketReceiver::SocketReceiver(Task*, int, pid_t)':
../framework/socketreceiver.cpp:19:30: error: 'cmsgbuf' was not declared in this scope
     fdpass_msg.msg_control = cmsgbuf;
                              ^~~~~~~
../framework/socketreceiver.cpp:19:30: note: suggested alternative: 'cmsg'
     fdpass_msg.msg_control = cmsgbuf;
                              ^~~~~~~
                              cmsg
In file included from /usr/include/sys/socket.h:41,
                 from ../framework/socketreceiver.h:6,
                 from ../framework/socketreceiver.cpp:6:
../framework/socketreceiver.cpp:24:22: error: 'register_t' was not declared in this scope
     cmsg->cmsg_len = CMSG_LEN(sizeof(int));
                      ^~~~~~~~
../framework/socketreceiver.cpp:24:22: note: suggested alternative: 'register'
../framework/socketreceiver.cpp: In member function 'virtual SocketConnection* SocketReceiver::incoming()':
../framework/socketreceiver.cpp:37:29: error: 'cmsgbuf' was not declared in this scope
     child_msg.msg_control = cmsgbuf;
                             ^~~~~~~
../framework/socketreceiver.cpp:37:29: note: suggested alternative: 'cmsg'
     child_msg.msg_control = cmsgbuf;
                             ^~~~~~~
                             cmsg
In file included from /usr/include/sys/socket.h:41,
                 from ../framework/socketreceiver.h:6,
                 from ../framework/socketreceiver.cpp:6:
../framework/socketreceiver.cpp:69:20: error: 'register_t' was not declared in this scope
     memcpy(&newfd, CMSG_DATA(cmsg), sizeof(newfd));
                    ^~~~~~~~~
../framework/socketreceiver.cpp:69:20: note: suggested alternative: 'register'
../framework/socketreceiver.cpp: In member function 'bool SocketReceiver::passSocketToPeer(int)':
../framework/socketreceiver.cpp:80:12: error: 'register_t' was not declared in this scope
     memcpy(CMSG_DATA(cmsg), &fd, sizeof(fd));
            ^~~~~~~~~
../framework/socketreceiver.cpp:80:12: note: suggested alternative: 'register'
gmake[2]: *** [../framework/mk.inc:105: ../framework/socketreceiver.o] Error 1
gmake[2]: Leaving directory '/usr/local/poudriere/ports/default/net/bredbandskollen/work/bbk-39b47a1/src/cli'
===> Compilation failed unexpectedly.
Comment 19 Piotr Kubaj freebsd_committer 2019-06-19 15:12:36 UTC
(In reply to Niclas Zeising from comment #17)
Just to be clear, are you waiting for more feedback?
Comment 20 Mark Linimon freebsd_committer freebsd_triage 2019-07-05 16:10:47 UTC
Take, as this PR seems to be stuck.  I will test with only the one patch.
Comment 21 Niclas Zeising freebsd_committer 2019-07-05 16:25:33 UTC
(In reply to Mark Linimon from comment #20)
> Take, as this PR seems to be stuck.  I will test with only the one patch.

It is not stuck, however, I can't reproduce all the issues, and I've had more important things going on in xorg land, so I have had to prioritize.
Comment 22 Niclas Zeising freebsd_committer 2019-07-05 16:26:41 UTC
(In reply to Niclas Zeising from comment #21)
> (In reply to Mark Linimon from comment #20)
> > Take, as this PR seems to be stuck.  I will test with only the one patch.
> 
> It is not stuck, however, I can't reproduce all the issues, and I've had
> more important things going on in xorg land, so I have had to prioritize.

And I want a solution that is upstreamable, it is not a sustainable solution to keep local patches for everything.
Comment 23 Niclas Zeising freebsd_committer 2019-07-05 17:03:06 UTC
Created attachment 205533 [details]
bredbandskollen update

I have this patch, can you please use that as a basis instead?
Comment 24 Niclas Zeising freebsd_committer 2019-07-05 18:46:11 UTC
Hi!
The CMSG_ macros are from our own sys/socket.h.  What I can't understand is where the register_t comes from.  The bredbandskollen code never uses it.

Looking at the preprocessor output on amd64, with gcc8 (which doesn't have this issue), 

char cmsgbuf[CMSG_SPACE(sizeof(int))];

expands to

char cmsgbuf[
# 36 "../framework/socketreceiver.h" 3 4
                ((((__uintptr_t)(sizeof(struct cmsghdr)) + (sizeof(__register_t) - 1)) & ~(sizeof(__register_t) - 1)) + (((__uintptr_t)(
# 36 "../framework/socketreceiver.h"
                sizeof(int)
# 36 "../framework/socketreceiver.h" 3 4
                ) + (sizeof(__register_t) - 1)) & ~(sizeof(__register_t) - 1)))
# 36 "../framework/socketreceiver.h"
                                       ];


The macro CMSG_SPACE macro looks like this (from sys/sys/socket.h)

#define CMSG_SPACE(l)           (_ALIGN(sizeof(struct cmsghdr)) + _ALIGN(l))

I wonder if _ALIGN expands to it, but I can't find that define.

On my amd64 system, through a series of includes, it looks like sys/socket.h includes x86/_types.h, which has a typedef for __register_t.
Comment 25 Niclas Zeising freebsd_committer 2019-07-05 18:57:46 UTC
So.  On powerpc, in sys/powerpc/include/_align.h, _ALIGNBYTES is defined, and it uses register_t.  _ALIGN uses _ALIGNBYTES.

On x86 (i386 and amd64 share the same file), in sys/x86/include/_align.h, _ALIGNBYTES uses __register_t.  This is why the issue doesn't show up, I would guess, assuming that powerpc includes a similar _types.h as x86, where __register_t but not register_t has its typedef.

I believe the includepath is sys/socket.h includes sys/_types.h, which includes _machine/_types.h.  _machine/_types.h is from sys/amd64/include/_types.h which includes sys/x86/include/_types.h.  I would assume that _machine/_types.h is from sys/powerpc/include/_types.h, that file defines __register_t, but not register_t.
Comment 26 Niclas Zeising freebsd_committer 2019-07-05 19:13:20 UTC
Created attachment 205534 [details]
Update bredbandskollen and fix build with gcc

Patch updates bredbandskollen to the latest version, and tries to fix the build with gcc, and specifically powerpc.
Comment 27 Piotr Kubaj freebsd_committer 2019-07-06 12:54:46 UTC
(In reply to Niclas Zeising from comment #26)
make: "/usr/local/poudriere/ports/default/net/bredbandskollen/Makefile" line 29: Malformed conditional (${ARCH:Mpowerpc*})

You need .include <bsd.port.pre.mk> before .if ${ARCH:Mpowerpc*}.
Comment 28 Niclas Zeising freebsd_committer 2019-07-06 21:33:38 UTC
Created attachment 205554 [details]
Update bredbandskollen and fix build with gcc

Yeah, my bad.  Here is an updated patch.
Comment 29 Niclas Zeising freebsd_committer 2019-07-06 21:35:25 UTC
Created attachment 205555 [details]
Update bredbandskollen and fix build with gcc

And here is really really the new patch.
Comment 30 Piotr Kubaj freebsd_committer 2019-07-09 10:22:08 UTC
(In reply to Niclas Zeising from comment #29)
bredbandskollen builds with this patch on powerpc64.
Comment 31 commit-hook freebsd_committer 2019-07-09 19:04:00 UTC
A commit references this bug:

Author: zeising
Date: Tue Jul  9 19:03:50 UTC 2019
New revision: 506304
URL: https://svnweb.freebsd.org/changeset/ports/506304

Log:
  net/bredbandskollen: Update snapshot

  Update net/bredbandskollen to the latest snapshot.
  Fix build on GCC-based architectures, such as powerpc64 [1]

  PR:		234216 [1]
  Submitted by:	pkubaj [1]

Changes:
  head/net/bredbandskollen/Makefile
  head/net/bredbandskollen/distinfo
  head/net/bredbandskollen/files/
  head/net/bredbandskollen/files/extra-src_framework_socketreceiver.h
  head/net/bredbandskollen/files/patch-src_framework_engine.h
Comment 32 Niclas Zeising freebsd_committer 2019-07-09 19:05:03 UTC
Committed