Created attachment 193483 [details]
QA LOG for x11-wm/icewm

QA LOG to prove it tests, and builds for 12-CURRENT

Comment 2 Kirill Ponomarev 2018-05-23 14:03:59 UTC

I can't build it on -current:

#define null (*(class null_ref *)0)
              ^~~~~~~~~~~~~~~~~~~~
wmprog.cc:942:42: warning: binding dereferenced null pointer to reference has undefined behavior [-Wnull-dereference]
            addItem(_("_Logout..."), -2, null, actionLogout);
                                         ^~~~
./ref.h:22:15: note: expanded from macro 'null'
#define null (*(class null_ref *)0)
              ^~~~~~~~~~~~~~~~~~~~
33 warnings generated.

(In reply to Kirill Ponomarev from comment #2)
> I can't build it on -current:
> 
> #define null (*(class null_ref *)0)
>               ^~~~~~~~~~~~~~~~~~~~
> wmprog.cc:942:42: warning: binding dereferenced null pointer to reference
> has undefined behavior [-Wnull-dereference]
>             addItem(_("_Logout..."), -2, null, actionLogout);
>                                          ^~~~
> ./ref.h:22:15: note: expanded from macro 'null'
> #define null (*(class null_ref *)0)
>               ^~~~~~~~~~~~~~~~~~~~
> 33 warnings generated.

Odd. As you can see from my build log attached to this pr(1),
it built fine for me on 12.
Is there any chance you could provide me with:

uname -a

from the system you're building this on?
Is it also possible to attach a copy of the build log (transcript)

eg;

$ script ~/x11-wm_icewm.log
$ make

Thanks for the report, Kirill!

--Chris

(In reply to Chris Hutchinson from comment #3)
> (In reply to Kirill Ponomarev from comment #2)
> > I can't build it on -current:
> > 
> > #define null (*(class null_ref *)0)
> >               ^~~~~~~~~~~~~~~~~~~~
> > wmprog.cc:942:42: warning: binding dereferenced null pointer to reference
> > has undefined behavior [-Wnull-dereference]
> >             addItem(_("_Logout..."), -2, null, actionLogout);
> >                                          ^~~~
> > ./ref.h:22:15: note: expanded from macro 'null'
> > #define null (*(class null_ref *)0)
> >               ^~~~~~~~~~~~~~~~~~~~
> > 33 warnings generated.
> 
> Odd. As you can see from my build log attached to this pr(1),
> it built fine for me on 12.
> Is there any chance you could provide me with:
> 
> uname -a
> 
> from the system you're building this on?
> Is it also possible to attach a copy of the build log (transcript)
> 
> eg;
> 
> $ script ~/x11-wm_icewm.log
> $ make
> 
> Thanks for the report, Kirill!
> 
> --Chris

Hmm that last part should have read;

$ script ~/x11-wm_icewm.log

$ make

...

$ exit

Comment 5 Kirill Ponomarev 2018-05-24 10:29:16 UTC

Fails on stable as well:

https://krion.cc/data/head-amd64-default/2018-05-23_15h53m55s/logs/errors/icewm-1.3.8_4.log

https://krion.cc/data/11amd64-default/2018-05-23_15h54m50s/logs/errors/icewm-1.3.8_4.log

Please use poudriere for QA.

(In reply to Kirill Ponomarev from comment #5)
> Fails on stable as well:
> 
> https://krion.cc/data/head-amd64-default/2018-05-23_15h53m55s/logs/errors/
> icewm-1.3.8_4.log
> 
> https://krion.cc/data/11amd64-default/2018-05-23_15h54m50s/logs/errors/icewm-
> 1.3.8_4.log
> 

OK this is *bizarre*
I have a patch dated 02-26-2018 I'm *sure* I submitted in another
pr(1) against x11-wm/icewm that dealt with exactly the error
emitted in the log(s) you submitted.

I'll whip up another patch that incorporates that, and the one
I already have in this pr(1), and submit it shortly.

Thanks!

--Chris

> Please use poudriere for QA.

Created attachment 193659 [details]
updated svn diff for x11-wm/icewm

Version 2 of the previous patch

Changes
 o updates Makefile
 o properly name patches in files/ dir and updates
   patch-src_wmapp.cc

This patche addresses the above, and fixes the error(s)
generated. It does not address the many warnings, and I
have no intention of addressing them here. As I'm almost
done with icewm-v1.4. Which *does* address those warnings.
I should be able to land v1.4 in about 2 weeks.

Thanks!

--Chris

Created attachment 193660 [details]
update previous svn diff (now at v3)

Same as last patch, but bump PORTREVISION this time.
Sigh...

Thanks, and sorry.

--Chris

Comment 9 Kirill Ponomarev 2018-05-24 19:20:20 UTC

Still fails on both current and stable:

https://krion.cc/data/head-amd64-default/2018-05-24_21h18m28s/logs/errors/icewm-1.3.8_4.log
https://krion.cc/data/11amd64-default/2018-05-24_21h19m00s/logs/errors/icewm-1.3.8_4.log

Created attachment 193678 [details]
updated svn diff for x11-wm/icewm (ver 4)

adds files/patch-src_ybutton.cc whiuch addresses
the errors indicated in your log(s)

Thanks!

--Chris

Comment 11 Kirill Ponomarev 2018-05-25 06:15:23 UTC

Chris, sorry I dont't have time to build it every day and every day and encounter always the same errors. If you'd like to fix this, please test it carefully on your poudriere.

https://krion.cc/data/head-amd64-default/2018-05-25_08h12m35s/logs/errors/icewm-1.3.8_4.log

Created attachment 193703 [details]
updated svn diff for x11-wm/icewm (ver 5 && FINAL)

version 5, and FINAL patch

Changes
 o Makefile
 o files/ : renames patch files to correct format
 o restricts compiler to (llvm/clang) 50 or less

(In reply to Kirill Ponomarev from comment #11)
> Chris, sorry I dont't have time to build it every day and every day and
> encounter always the same errors. If you'd like to fix this, please test it
> carefully on your poudriere.
> 
> https://krion.cc/data/head-amd64-default/2018-05-25_08h12m35s/logs/errors/
> icewm-1.3.8_4.log

HUGE apology, Kirill
I found the discrepancy when running the following test case
against llvm/clang60

USES=		compiler:c++11-lang ...
USE_CXXSTD=	c++11

...

.include <bsd.port.pre.mk>

.if ${COMPILER_TYPE} == clang && ${COMPILER_VERSION} < 60
BUILD_DEPENDS+=	${LOCALBASE}/bin/clang60:devel/llvm60
CPP=		${LOCALBASE}/bin/clang-cpp60
CC=		${LOCALBASE}/bin/clang60
CXX=		${LOCALBASE}/bin/clang++60
.endif

...

	@${ECHO} "XXXX: ${COMPILER_VERSION}"


which immediately let me know the jail I had spun up, was
running llvm50!

So I spun up a jail with both 50 && 60, and performed some
further test cases. Which determined my current changes to this
(version) port won't build past (llvm/clang) 50.

I'm not (wasting) spending anymore time to bring this up to
a llvm/clang60 level. Because I'm trying to land version 1.4.22
of this port, which is *already* llvm/clang60 friendly.

This will at least quiet pkg-fallout, and allow people to build,
or install the package.

Thanks a million for all your time, and sorry for wasting so
much of it.

--Chris

Created attachment 194007 [details]
QA LOG against the most recent svn diff

This is the build transcript against my (most) current svn diff
for x11-wm/icewm. Which required the use of llvm/clang version 5.
It builds with warnings, but w/o error.

--Chris

Created attachment 194008 [details]
QA LOG for x11-wm/icewm

This is a build transcript for x11-wm/icewm built
against llvm/clang version 6. A build which fails
with error(s).

The (current) svn diff attached to this pr(1)
*requires* the use of llvm/clang version 5
These 2 QA LOGs prove that out. As do my most
recent remarks as to achieving this goal, and my
submitted solution.

I think I'm done here.

--Chris

Comment 16 commit-hook 2018-06-16 02:49:53 UTC

A commit references this bug:

Author: tobik
Date: Sat Jun 16 02:49:07 UTC 2018
New revision: 472514
URL: https://svnweb.freebsd.org/changeset/ports/472514

Log:
  x11-wm/icewm: Fix build with Clang 6

  While here

  - Fix license
  - Add missing dependencies

  PR:		228317
  Submitted by:	Chris Hutchinson <portmaster@bsdforge.com> (based on)
  Kudos to:	krion (for patiently test building many iterations of it)

Changes:
  head/x11-wm/icewm/Makefile
  head/x11-wm/icewm/files/patch-src__wmapp.cc

Comment 17 Tobias Kortkamp 2018-06-16 02:53:05 UTC

Committed an alternative patch.  The build can be fixed by applying
the src/wmapp.cc patch and setting USE_CXXSTD=c++98 so it's not
necessary to go backwards to Clang 5.

(In reply to Tobias Kortkamp from comment #17)
> Committed an alternative patch.  The build can be fixed by applying
> the src/wmapp.cc patch and setting USE_CXXSTD=c++98 so it's not
> necessary to go backwards to Clang 5.

Ugh. I can't *believe* I didn't think of that at the time.
Thanks Tobias!

--Chris