add gcc 7 and 8 build to travis tests #2067

andy5995 · 2019-01-25T19:36:06Z

This works but I'm not sure yet how much the code can be reduced by using a build matrix. Comments welcome.

elextr · 2019-04-30T06:25:24Z

To record conversation from IRC, I don't think there is any need to matrix all the envs with the compiler types.

Travis for Geany is essentially a compile check (there are no real Geany tests to run, just a few ctags ones from upstream). So all we need is each of the env options to get compiled, and to look out for any compile issues with variations of compilers (and newly defaulted stupid warnings :)

andy5995 · 2019-04-30T06:29:39Z

Understood. All the checks have passed and this is ready for reviewed.

cc @b4n

elextr · 2019-04-30T07:21:26Z

Clang could also be added, but it does have a "-pthread option not used" warning on the link (on clang 6 which is my default here) which googling seems to imply is harmless. Googling also implies that libtool automatically adds this, so it might not be easy to fix.

andy5995 · 2019-04-30T07:44:47Z

Clang could also be added

May be worth a short discussion that if an osx (xcode) test were added, it would use clang. So the question would be, should the osx test be added? should it be added on a separate PR? And if so, would that render a clang test on Ubuntu as not needed?

cc @techee

andy5995 · 2019-04-30T07:49:56Z

I'm pretty sure if it's decided that adding osx is ultimately desired, it should be done in a separate PR, because most of the stuff in the 2nd half of .travis.yml would have to be put in conditional statements (if linux (run this) if osx (run this), and some brew lines would be needed to install packages if the test was for osx.

elextr · 2019-04-30T07:50:04Z

Well, OSX stuff is in a separate repository geany/geany-osx, so not sure if the code here compiles on OSX by itself.

andy5995 · 2019-04-30T07:58:55Z

Well, OSX stuff is in a separate repository geany/geany-osx, so not sure if the code here compiles on OSX by itself.

Ok, so in travis.yml there'd need to be instructions to clone that repo or cache it.

techee · 2019-04-30T08:14:26Z

Well, OSX stuff is in a separate repository geany/geany-osx, so not sure if the code here compiles on OSX by itself.

Not really, geany-osx contains just the bundling stuff but no code so it would make sense to have an osx test. I just think the osx builders tend to be overloaded on travis and take too much time to build. However, the command-line part of osx behaves like freebsd so if there's some BSD builder on travis, we could use that one.

elextr · 2019-04-30T09:33:39Z

No BSDs I'm afraid, maybe OSX should be left to another PR, lets see what @b4n says about this one for now.

b4n

This sounds good, but I'm not so sure how important it is to check with recent compilers -- old ones for compat, new ones… well, it certainly doesn't hurt :) However, we most probably don't need to run distcheck on all compilers, as it's mostly checking the build system itself rather than the compilation. Only all (obviously) and check are really useful to run everywhere for a sensible test, and that might give a comfortable speed up by not building everything twice. This can be changed at some later point tho, and can probably be fixed by e.g. setting TARGETS="all check distcheck" in the "normal" case and removing distcheck in the "compiler builds", and replacing the make -j2 && make -j2 check && make -j2 distcheck with just make -j2 $TARGETS.

Anyway, the change seems a little verbose, but there might not be a shorter syntax for this (I'm no YAML expert tho).

.travis.yml

b4n · 2019-04-30T13:32:26Z

.travis.yml

+          sources:
+            - llvm-toolchain-trusty-5.0
+          packages:
+            - clang-5.0


Ideally we'd test with clang 3.4 as it's supposed to be able to build Scintilla, which is our most bleeding edge.

replace 5.0 with 3.4, or add 3.4?

Sadly Geany C89 is the trailing edge, although I think a few C99s have crept in under the radar :)

But Clang 3.4 isn't officially C++11 either.

I don't see that the 3.4 toolchain is available anymore https://www.ubuntuupdates.org/package/xorg-edgers/trusty/main/base/llvm-toolchain-3.4

3.5 fails on trusty and xenial
5 passes on trusty
7 passes on xenial

b4n · 2019-05-01T17:39:54Z

.travis.yml

 before_install:
+  - eval "${MATRIX_EVAL}"


this should be in before_script below with the CFLAGS export, shouldn't it?

Also I'd rather see this done using export, and set e.g. MATRIX_EXPORT="CC=clang-5.0 CXX=clang++-5.0", which is effectively what we want done (and so it doesn't rely on CC and CXX being exported by something else).

As @elextr noted, what I've used is suggested by the travis docs. It doesn't appear from the logs that CC and CXX are getting exported by anything else later. But you're concern is that it's a possibility? But definitely let me know if you're sure you'd like me to change that anyway.

.travis.yml

elextr · 2019-05-01T22:46:26Z

@b4n

Anyway, the change seems a little verbose, but there might not be a shorter syntax for this (I'm no YAML expert tho).

The syntax is copied from the Travis help page link I posted on #2139 so blame it on Travis. This applies to some of your comments above too, so unless you know better than Travis themselves maybe it should stay as is.

andy5995 · 2019-05-02T03:20:02Z

However, we most probably don't need to run distcheck on all compilers, as it's mostly checking the build system itself rather than the compilation. Only all (obviously) and check are really useful to run everywhere for a sensible test, and that might give a comfortable speed up by not building everything twice. This can be changed at some later point tho, and can probably be fixed by e.g. setting TARGETS="all check distcheck" in the "normal" case and removing distcheck in the "compiler builds", and replacing the make -j2 && make -j2 check && make -j2 distcheck with just make -j2 $TARGETS.

I pushed a commit that addresses that. But I made the change before I reviewed your suggested approach. If you'd like it changed, please let me know.

Anyway, the change seems a little verbose, but there might not be a shorter syntax for this (I'm no YAML expert tho).

That's another copy/paste from the travis docs. afaik the only way to reduce the lines is to use a script that manually imports the toolchain repo and keys, and installs the appropriate gcc package.

However, using the the "addons/source" travis directive as it is now may be easier in the long-run, to maintain. What do you think?

andy5995 · 2019-05-06T04:29:42Z

Oh.. a lot of errors when building the documentation on xenial

https://travis-ci.org/geany/geany/jobs/528635477#L1635

compared to trusty

https://travis-ci.org/geany/geany/jobs/527884388#L1667

Should a build fail when that happens?

elextr · 2019-05-06T05:58:47Z

We shouldn't be allowing anything to fail, the problem with clang needs to be fixed.

Not sure why, but it seems to have the wrong library version, you can see its using ".../c++/4.8/..." with clang 5. Thats likely the problem.

elextr · 2019-05-06T08:21:14Z

Note: clang 5 doesn't work on my system, not just Travis, (even with -std=c++11) but clang 6 does. But removing the "noexcept"s from LineMarker (scintilla/src/linemaker.h) makes it work.

elextr · 2019-05-06T08:22:11Z

@b4n does Mitchell actually check with the compilers he claims to support?

andy5995 · 2019-05-06T08:33:27Z

Got some VERBOSE output from the latest Travis build https://travis-ci.org/geany/geany/jobs/528700712#L1734

andy5995 · 2019-05-06T08:40:35Z

Got some VERBOSE output from the latest Travis build https://travis-ci.org/geany/geany/jobs/528700712#L1734

I see now C++11 support for clang < 6 was already being added by the -std=gnu++11 flag

elextr · 2019-05-06T10:21:35Z

@andy5995 don't ever [Edit: deliberately] use the gnu++ flags, they can contain things that actually conflict with the standard, so its not portable.

andy5995 · 2019-05-06T10:26:01Z

@andy5995 don't ever use the gnu++ flags, they can contain things that actually conflict with the standard, so its not portable.

Ok, but I didn't put that there xD I don't know where it's coming from...

b4n · 2019-05-07T21:57:55Z

@b4n does Mitchell actually check with the compilers he claims to support?

I'm not sure. And if it's due to the linemarker thing and this PR got rebased on master, it makes sense it breaks, and it's a good thing we know it.

This said, for me clang 3.8 builds master.

Ok, but I didn't put that there xD I don't know where it's coming from...

It's coming from AX_CXX_COMPILE_STDCXX_11 which figured it's a way to get that compiler to support C++11 -- or at least a part of it that is checked for.

.travis.yml

b4n · 2019-05-07T22:07:10Z

.travis.yml

+        make -j2 check;
+      else
+        make -j2 distcheck DISTCHECK_CONFIGURE_FLAGS="$CONFIGURE_FLAGS";
+      fi;


I don't like that, because what you do is build one more time, and only that one do the distcheck. And given how distcheck works (make the tarball and try to build it), it could possibly be affected by the configuration options selecting buildig or not some code (even though it should not if we didn't mess up, but CI is to catch case where we did, isn't it 😄 ). So it is a good thing it's checked for all variations of the environment.

Only the same environment with different compilers is not useful, because building with another compiler shouldn't affect what we're building.

I didn't completely understand this but let me know if the change I made is what you want.

Now its failed on another noexcept, @andy5995 please don't trigger another build, its not possible to see old build logs, wait for others to see it.

Sure, I'll wait. But I do believe you can see old build logs at https://travis-ci.org/geany/geany/pull_requests

I didn't completely understand this but let me know if the change I made is what you want.

now DISTCHECK_ONLY is never set, is it?
but stop worrying about this part and we'll see once the build stuff is dealt with

my bad I missed the one in the matrix

This should add it as a separate job and make it more clear

according to Travis docs, the default is 3.5, which fails on xenial.

Clang now failing with 5.0 on trusty, though it worked before (as can be seen from the log)

* keep clang 5 for now to see if removing the cache was the problem in previous commit. * Show versions being used a little more clearly (before_script)

The travis docs say 3.5 is the default on trusty but the previous test showed 5.0 was being used. The other job specified for 5.0 on trusty failed again even after the ccache directive was removed. Switching that to xenial.

@elextr

trying this after reading jeaye/color_coded#28 and based on what @elextr mentioned about the incorrect libs.

https://travis-ci.org/geany/geany/jobs/528683477#L1038

andy5995 · 2019-05-08T06:39:10Z

@b4n does Mitchell actually check with the compilers he claims to support?

I'm not sure. And if it's due to the linemarker thing and this PR got rebased on master, it makes sense it breaks, and it's a good thing we know it.

This said, for me clang 3.8 builds master.

I tried with 3.8. fail.

elextr · 2019-05-08T06:45:25Z

This said, for me clang 3.8 builds master.

I tried with 3.8. fail.

Here clang 3.9 works, clang 5.0 fails. Note make clean needed to make it fail. Also I'm building GTK-3 all the time, but GTK-2 works with 3.9 as well.

elextr · 2019-05-08T06:48:39Z

Now its failed on another noexcept, @andy5995 please don't trigger another build, its not possible to see old build logs, wait for others to see it.

elextr · 2019-05-08T06:54:49Z

@b4n the comment above by @andy5995 about library versions was an observation I made that the LineMarker failure happened in an STL file in ".../c++/n.m/..." where the n.m version did not match the compiler version see https://travis-ci.org/geany/geany/builds/528683464 the 3.5 compiler used 4.8 STL and the 5.0 compiler used 5.4.0 STL?

@andy5995 installing different libraries isn't the solution, it needs to use whats on peoples systems, the point of travis is to find problems, not for us to force it to work by creating a non-standard configuration :)

b4n · 2019-05-08T07:25:32Z

Here clang 3.9 works, clang 5.0 fails. Note make clean needed to make it fail.

Do you build a clean clone each time? Needing make clean doesn't suggest so, and I personally wouldn't try rebuilding half the project with another compiler and expect things to go smoothly :)
Also, just to me sure: you need to re-run configure with the new values of CC and CXX, not merely run make CC=... CXX=... -- the later could work, but it also could fail.

https://travis-ci.org/geany/geany/jobs/528683476, build 1959.12:

$ export MATRIX_EVAL="CC=clang-3.5 && CXX=clang++-3.5"
$ export TRAVIS_COMPILER=gcc
$ export CXX=g++
$ export CXX_FOR_BUILD=g++
$ export CC=gcc
$ export CC_FOR_BUILD=gcc

That's already suspicious. And the job summary suggest Travis thinks it's gcc, but we set clang-3.5.

/usr/bin/../lib/gcc/x86_64-linux-gnu/4.8/../../../../include/c++/4.8/bits/unique_ptr.h:166:18:

That gets me worried: it looks like it's using GCC 4.8 include paths with Clang…

Similarly in build 1959.13 which is supposed to be Clang 5.0 :

$ export MATRIX_EVAL="CC=clang-5.0 && CXX=clang++-5.0"
$ export TRAVIS_COMPILER=clang
$ export CXX=clang++
$ export CXX_FOR_BUILD=clang++
$ export CC=clang
$ export CC_FOR_BUILD=clang
$ clang --version
clang version 7.0.0 (tags/RELEASE_700/final)
[…]
/usr/bin/../lib/gcc/x86_64-linux-gnu/5.4.0/../../../../include/c++/5.4.0/bits/unique_ptr.h:217:18:

Maybe it's nothing, but it'd get me looking.

elextr · 2019-05-08T07:37:13Z

Needing make clean doesn't suggest so, and I personally wouldn't try rebuilding half the project with another compiler and expect things to go smoothly :)

Yep, totally agree, thats why I did make clean in between (although with meson it wouldn't be needed </troll>) just that I forgot one time and noted it "worked", and despite the mixed compiles it ran too :)

Also, just to me sure: you need to re-run configure with the new values of CC and CXX, not merely run make CC=... CXX=... -- the later could work, but it also could fail.

Yes, I'm running configure each time.

That gets me worried: it looks like it's using GCC 4.8 include paths with Clang…

I think (but no expert on this level of details) that for the STL clang uses the same as gcc so the compiled products can be linked together, the clang package here is dependent on libstdc++ and libgcc not any clang specific version. On that basis I wonder if the version difference isn't an issue since one is a gcc version and the other is a clang version?

kugel- · 2019-05-08T07:46:10Z

(although with meson it wouldn't be needed )

Also not needed with https://github.com/kugel-/kmake :-)

b4n · 2019-05-08T13:09:14Z

@andy5995 what about the latest state on #2149? it's still verbose but indeed I don't know how to improve it, and has a solution I like better for running only some targets. Also I dropper clang builds failing because we have code they don't like, see that PR for details on why.

andy5995 · 2019-05-08T13:10:10Z

https://travis-ci.org/geany/geany/jobs/528683476, build 1959.12:
$ export MATRIX_EVAL="CC=clang-3.5 && CXX=clang++-3.5"
$ export TRAVIS_COMPILER=gcc
$ export CXX=g++
$ export CXX_FOR_BUILD=g++
$ export CC=gcc
$ export CC_FOR_BUILD=gcc
That's already suspicious. And the job summary suggest Travis thinks it's gcc, but we set clang-3.5.

Oh, looks like I forgot to set the "compiler" in that section. I'll push a new commit

to help with #2067 (comment)

andy5995 · 2019-05-08T13:18:33Z

https://travis-ci.org/geany/geany/jobs/528683476, build 1959.12:
$ export MATRIX_EVAL="CC=clang-3.5 && CXX=clang++-3.5"
$ export TRAVIS_COMPILER=gcc
$ export CXX=g++
$ export CXX_FOR_BUILD=g++
$ export CC=gcc
$ export CC_FOR_BUILD=gcc
That's already suspicious. And the job summary suggest Travis thinks it's gcc, but we set clang-3.5.
Oh, looks like I forgot to set the "compiler" in that section. I'll push a new commit

I see now you're trying stuff in #2149 so I'll hold off on any more changes.

btw, anyone with "maintainer" access can just edit the files in this PR.

andy5995 · 2019-05-08T13:25:28Z

Similarly in build 1959.13 which is supposed to be Clang 5.0 :

$ export MATRIX_EVAL="CC=clang-5.0 && CXX=clang++-5.0"
$ export TRAVIS_COMPILER=clang
$ export CXX=clang++
$ export CXX_FOR_BUILD=clang++
$ export CC=clang
$ export CC_FOR_BUILD=clang
$ clang --version
clang version 7.0.0 (tags/RELEASE_700/final)
[…]
/usr/bin/../lib/gcc/x86_64-linux-gnu/5.4.0/../../../../include/c++/5.4.0/bits/unique_ptr.h:217:18:

Maybe it's nothing, but it'd get me looking.

afaik, it's showing that near the start because 7.0 is the default. Later it installs the correct packages and sets the other vars appropriately.

But for the other quote (not included ) where it shows gcc vars being exported, I'll say again it's because I forgot to add the compiler line to that section.

andy5995 · 2019-05-09T09:45:22Z

@andy5995 what about the latest state on #2149? it's still verbose but indeed I don't know how to improve it, and has a solution I like better for running only some targets. Also I dropper clang builds failing because we have code they don't like, see that PR for details on why.

@b4n LGTM feel free to do any fine-tuning and finish it up

elextr · 2019-05-09T09:49:47Z

Closed as replaced by #2149, @andy5995 thanks for starting the process.

andy5995 · 2019-05-09T12:52:08Z

You're welcome

codebrainz mentioned this pull request Apr 30, 2019

Test with more compiler versions on travis #2139

Closed

andy5995 changed the title ~~add gcc 7 and 8 build to travis tests~~ [wip] add gcc 7 and 8 build to travis tests Apr 30, 2019

andy5995 changed the title ~~[wip] add gcc 7 and 8 build to travis tests~~ add gcc 7 and 8 build to travis tests Apr 30, 2019

b4n requested changes May 1, 2019

View reviewed changes

b4n reviewed May 7, 2019

View reviewed changes

.travis.yml Outdated Show resolved Hide resolved

b4n reviewed May 7, 2019

View reviewed changes

andy5995 added 4 commits May 8, 2019 01:14

.travis.yml:add tests for gcc 7 and 8

239733c

use build matrix #2139

6855d73

for gcc 8, specify xenial and the correct version

205d92e

try globalizing the GTK3 env directives

c9b867d

andy5995 added 13 commits May 8, 2019 01:14

add DISTCHECK_ONLY, don't hide it under gcc-5

2c69ef5

This should add it as a separate job and make it more clear

use xenial instead of trusty

7205fcc

reduce clang to 3.5

7b7b576

try default clang on xenial

752aa1a

try default clang on trusty

8a58f4a

according to Travis docs, the default is 3.5, which fails on xenial.

clang:revert to using 5.0 on trusty

329cd50

allow clang to fail

572bff1

Clang now failing with 5.0 on trusty, though it worked before (as can be seen from the log)

remove cache directive, try clang 3.5 again; also...

b93a681

* keep clang 5 for now to see if removing the cache was the problem in previous commit. * Show versions being used a little more clearly (before_script)

specifying 3.5 on trusty

1f33eaf

The travis docs say 3.5 is the default on trusty but the previous test showed 5.0 was being used. The other job specified for 5.0 on trusty failed again even after the ccache directive was removed. Switching that to xenial.

install libc++

4f3b004

trying this after reading jeaye/color_coded#28 and based on what @elextr mentioned about the incorrect libs.

clang:explicity enable C++11 support

253d685

https://travis-ci.org/geany/geany/jobs/528683477#L1038

make:temporarily add -n and VERBOSE=1

fef1581

use clang 3.8

139b670

use clang 3.5, add missing "compiler" line

7509dfe

to help with #2067 (comment)

elextr closed this May 9, 2019

andy5995 mentioned this pull request Aug 17, 2020

remove redundant "addon" sections, manually install toolchain repo b4n/geany#4

Closed

add gcc 7 and 8 build to travis tests #2067

add gcc 7 and 8 build to travis tests #2067

Conversation

andy5995 commented Jan 25, 2019

elextr commented Apr 30, 2019

andy5995 commented Apr 30, 2019

elextr commented Apr 30, 2019

andy5995 commented Apr 30, 2019

andy5995 commented Apr 30, 2019

elextr commented Apr 30, 2019

andy5995 commented Apr 30, 2019

techee commented Apr 30, 2019

elextr commented Apr 30, 2019

b4n left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

elextr commented May 1, 2019 • edited

andy5995 commented May 2, 2019

andy5995 commented May 6, 2019

elextr commented May 6, 2019

elextr commented May 6, 2019 • edited

elextr commented May 6, 2019

andy5995 commented May 6, 2019

andy5995 commented May 6, 2019

elextr commented May 6, 2019 • edited

andy5995 commented May 6, 2019

b4n commented May 7, 2019

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

andy5995 commented May 8, 2019

elextr commented May 8, 2019

elextr commented May 8, 2019

elextr commented May 8, 2019 • edited

b4n commented May 8, 2019

elextr commented May 8, 2019

kugel- commented May 8, 2019

b4n commented May 8, 2019

andy5995 commented May 8, 2019

andy5995 commented May 8, 2019

andy5995 commented May 8, 2019

andy5995 commented May 9, 2019

elextr commented May 9, 2019

andy5995 commented May 9, 2019

elextr commented May 1, 2019 •

edited

elextr commented May 6, 2019 •

edited

elextr commented May 6, 2019 •

edited

elextr commented May 8, 2019 •

edited