Bug 250740 - mail/mailsync 12.2-RELEASE breaks package build
Summary: mail/mailsync 12.2-RELEASE breaks package build
Status: Closed FIXED
Alias: None
Product: Ports & Packages
Classification: Unclassified
Component: Individual Port(s) (show other bugs)
Version: Latest
Hardware: amd64 Any
: --- Affects Some People
Assignee: freebsd-ports-bugs (Nobody)
URL:
Keywords:
Depends on:
Blocks:
 
Reported: 2020-10-30 14:39 UTC by Colin
Modified: 2020-11-12 11:23 UTC (History)
3 users (show)

See Also:
fernape: merge-quarterly+


Attachments
Poudriere build log under 12.2-RELEASE/LLVM10 (29.75 KB, text/plain)
2020-10-30 14:39 UTC, Colin
no flags Details
Patch to the ports tree (660 bytes, patch)
2020-11-11 14:16 UTC, Fernando Apesteguía
no flags Details | Diff
Poudriere build log under 12.2-RELEASE/LLVM10 with patch applied (29.75 KB, text/plain)
2020-11-11 16:39 UTC, Colin
no flags Details
Work around the #define hacks in c-client.h (1.68 KB, patch)
2020-11-11 17:54 UTC, Dimitry Andric
no flags Details | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Colin 2020-10-30 14:39:06 UTC
Created attachment 219221 [details]
Poudriere build log under 12.2-RELEASE/LLVM10

After updating to 12.2-RELEASE, mailsync 5.2.1.4 build fails during configure. This appears to be the result of the update of LLVM in base. The build log now ends with:-

checking if simple c-client program compiles without pam support... yes
checking if c-client works without -fno-operator-names in c++... no
checking if adding -fno-operator-names helps... no
configure: error: a working c-client installation is required for building mailsync
===>  Script "configure" failed unexpectedly.


Previously, under 12.1, (LLVM 8.01) this test yielded:-

checking if simple c-client program compiles without pam support... yes
checking if c-client works without -fno-operator-names in c++... no
checking if adding -fno-operator-names helps... yes
checking if c-client includes md5 support... yes
checking that generated files are newer than configure... done
configure: creating ./config.status

Full log attached.
Comment 1 Fernando Apesteguía freebsd_committer 2020-11-11 14:16:38 UTC
Created attachment 219554 [details]
Patch to the ports tree

Hi Colin,

Thanks for the report. I see mail/mailsync does not build in 11.4 or 12.2 but it does build in 12.1 and 13-current.

Would you mind trying this patch? It basically builds with GCC in those cases. I will investigate a bit more. The culprit seems to be this in mail/cclient:

configure:5854: checking if adding -fno-operator-names helps
configure:5864: c++ -c -O2 -pipe -fstack-protector-strong -fno-strict-aliasing   -fno-operator-names  -I/usr/local/include/c-client conftest.cpp >&5
In file included from conftest.cpp:32:
In file included from /usr/local/include/c-client/c-client.h:42:
In file included from /usr/local/include/c-client/osdep.h:29:
In file included from /usr/include/c++/v1/stdlib.h:100:
In file included from /usr/include/c++/v1/math.h:311:
/usr/include/c++/v1/type_traits:1364:1: error: unknown type name 'cclientPrivate'
private:
^
/usr/local/include/c-client/c-client.h:35:17: note: expanded from macro 'private'
#define private cclientPrivate  /* private to c-client */
                ^
In file included from conftest.cpp:32:
In file included from /usr/local/include/c-client/c-client.h:42:
In file included from /usr/local/include/c-client/osdep.h:29:
In file included from /usr/include/c++/v1/stdlib.h:100:
In file included from /usr/include/c++/v1/math.h:311:
/usr/include/c++/v1/type_traits:1365:5: error: expected expression
    typedef _LIBCPP_NODEBUG_TYPE typename remove_reference<_Tp>::type _Up;
    ^
/usr/include/c++/v1/type_traits:1367:51: error: use of undeclared identifier '_Up'
    typedef _LIBCPP_NODEBUG_TYPE typename __decay<_Up, __is_referenceable<_Up>::value>::type type;
                                                  ^
/usr/include/c++/v1/type_traits:1367:81: error: non-friend class member 'value' cannot have a qualified name
    typedef _LIBCPP_NODEBUG_TYPE typename __decay<_Up, __is_referenceable<_Up>::value>::type type;
                                                                              ~~^
/usr/include/c++/v1/type_traits:1367:81: error: typedef declarator cannot be qualified
    typedef _LIBCPP_NODEBUG_TYPE typename __decay<_Up, __is_referenceable<_Up>::value>::type type;
                                                                              ~~^
/usr/include/c++/v1/type_traits:1367:86: error: expected ';' at end of declaration list
    typedef _LIBCPP_NODEBUG_TYPE typename __decay<_Up, __is_referenceable<_Up>::value>::type type;
                                                                                     ^
/usr/include/c++/v1/type_traits:1721:1: error: unknown type name 'cclientPrivate'
private:
^

With the patch, it builds in {11.4,12.1}{amd64,i386} 12.2amd64 and 13-current amd64.
Comment 2 Fernando Apesteguía freebsd_committer 2020-11-11 14:30:36 UTC
^Triage: adding toolchain@ to see if they can shed some light here.

It seems we have a bug/limitation that shows up and hides depending of the version.

So without the patch:

11.4{amd64,i386}: Fails
12.1{amd64,i386}: OK
12.2amd64: Fails
13-current: OK (r366283)
Comment 3 Dimitry Andric freebsd_committer 2020-11-11 15:01:47 UTC
(In reply to Fernando Apesteguía from comment #1)
> /usr/include/c++/v1/type_traits:1364:1: error: unknown type name 'cclientPrivate'
> private:
> ^
> /usr/local/include/c-client/c-client.h:35:17: note: expanded from macro 'private'
> #define private cclientPrivate  /* private to c-client */
>                 ^

I don't know where this port got the idea that it's OK to redefine C++ keywords, and expect anything to keep working? Remove that nonsense and it might start working. :-)
Comment 4 Fernando Apesteguía freebsd_committer 2020-11-11 15:16:35 UTC
(In reply to Dimitry Andric from comment #3)
Hi Dimitry!

Yes, it does something similar with "and", "or"... and workarounds the problem by using -fno-name-operators.

Having a look at the code it looks like the author thought that flag would protect them:

extern "C" {
  /* If you use gcc, you may also have to use -fno-operator-names */
#define private cclientPrivate  /* private to c-client */
#define and cclientAnd          /* C99 doesn't realize that ISO 646 is dead */
#define or cclientOr
#define not cclientNot
#endif

Now, my question is why does it work in different versions of clang, including the most recent one but seems to fail from time to time across versions?
Comment 5 Colin 2020-11-11 16:36:56 UTC
(In reply to Fernando Apesteguía from comment #1)
I confirm that, with that patch applied, it then builds successfully in my 12.2R environment.
Comment 6 Colin 2020-11-11 16:39:38 UTC
Created attachment 219563 [details]
Poudriere build log under 12.2-RELEASE/LLVM10 with patch applied
Comment 7 Dimitry Andric freebsd_committer 2020-11-11 16:40:01 UTC
(In reply to Fernando Apesteguía from comment #4)

First of all, the c-client software was clearly written before C++ existed, since it uses C++ keywords as struct member names, so that is why they have to hack around it with #defining those keywords to something non-conflicting.

However, those defines should *not* be active when including any system headers, and that is clearly violated by the c-client.h file. It is simply totally broken, and you were lucky that it worked at all.

In any case, it is likely not about clang or clang++, but about the used libc++ headers. If you use C++, libc++ overrides certain standard C headers such as stdio.h, string.h, etc. Since the c-client headers are including multiple system headers with 'private' redefined, this wreaks havoc when it eventually goes into libc++'s own headers. I guess g++ doesn't work that way, but I haven't investigated.

In my opinion, c-client is obsolete software which is clearly incompatible with C++, and should either be patched up to relatively non-ancient standards, or ditched altogether.

Note that mailsync itself does not look much better. :)
Comment 8 commit-hook freebsd_committer 2020-11-11 17:26:04 UTC
A commit references this bug:

Author: fernape
Date: Wed Nov 11 17:25:22 UTC 2020
New revision: 554904
URL: https://svnweb.freebsd.org/changeset/ports/554904

Log:
  mail/mailsync: Unbreak in 11.4 and 12.2

  This port is broken in 12.2 and 11.4, but not in 12.1 or 13-current. The problem
  is the old mail/cclient doing things like redefining C++ keywords.

  Workaround this by building with GCC in those releases where it is broken.

  PR:	250740
  Submitted by:	colin@fbug.ksac.uk

Changes:
  head/mail/mailsync/Makefile
Comment 9 Fernando Apesteguía freebsd_committer 2020-11-11 17:26:21 UTC
(In reply to Dimitry Andric from comment #7)
Thanks for the exaplanation :)
Comment 10 Fernando Apesteguía freebsd_committer 2020-11-11 17:27:09 UTC
Committed,

We should probably expire mail/cclient and consequently mail/mailsync when the former breaks again.

Thanks!
Comment 11 Colin 2020-11-11 17:30:50 UTC
(In reply to Fernando Apesteguía from comment #9)
Thank you both.
Comment 12 Dimitry Andric freebsd_committer 2020-11-11 17:54:00 UTC
Created attachment 219566 [details]
Work around the #define hacks in c-client.h

Here's an alternative patch, which should also make mailsync compile with clang++, on 12.2-R.

Basically, it undefines all the keywords again before including osdep.h (which is the root cause of the original errors), then redefines them again. It's ugly but it works. :)
Comment 13 Colin 2020-11-11 19:14:34 UTC
(In reply to Dimitry Andric from comment #12)
Thanks Dmitry. Just to confirm cclient and mailsync also build cleanly on my system after applying your patch. This may be a more general approach, if there are other ports still dependent on cclient.
Comment 14 commit-hook freebsd_committer 2020-11-12 11:19:01 UTC
A commit references this bug:

Author: fernape
Date: Thu Nov 12 11:18:35 UTC 2020
New revision: 554949
URL: https://svnweb.freebsd.org/changeset/ports/554949

Log:
  mail/cclient: Unbreak in 11.4 and 12.2

  Dimitry sent a more generic patch that workarounds the problem in mail/cclient
  instead of its consumers.

  Related to r554904.

  PR:	250740
  Submitted by:	dim@FreeBSD.org
  Reported by:	colin@fbug.ksac.uk
  MFH:	2020Q4 (blanket, build fix)

Changes:
  head/mail/cclient/Makefile
  head/mail/cclient/files/patch-src_c-client_c-client.h
Comment 15 commit-hook freebsd_committer 2020-11-12 11:23:04 UTC
A commit references this bug:

Author: fernape
Date: Thu Nov 12 11:22:09 UTC 2020
New revision: 554951
URL: https://svnweb.freebsd.org/changeset/ports/554951

Log:
  MFH: r554949

  mail/cclient: Unbreak in 11.4 and 12.2

  Dimitry sent a more generic patch that workarounds the problem in mail/cclient
  instead of its consumers.

  Related to r554904.

  PR:	250740
  Submitted by:	dim@FreeBSD.org
  Reported by:	colin@fbug.ksac.uk

  Approved by:	ports-secteam (blanket, build fix)

Changes:
_U  branches/2020Q4/
  branches/2020Q4/mail/cclient/Makefile
  branches/2020Q4/mail/cclient/files/patch-src_c-client_c-client.h
Comment 16 Fernando Apesteguía freebsd_committer 2020-11-12 11:23:50 UTC
Done.

Thanks for looking at this!