Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Require C++17 #585

Open
jsiwek opened this issue Sep 19, 2019 · 11 comments

Comments

@jsiwek
Copy link
Member

commented Sep 19, 2019

We'll look into requiring/using C++17 for v3.1.0+.

  • Change CMake scripts to require/use C++17 instead of C++11
  • Update our minimum compiler version documentation to say GCC 7+ and Clang 4+.
  • Possibly a brief survey to see if there's any supported/LTS platform that we don't expect to meet compiler requirements by default and provide explicit suggestions for those in the Zeek installation docs.
@jsiwek jsiwek added this to the 3.1.0 milestone Sep 19, 2019
@jsiwek jsiwek added this to Unassigned / Todo in Release 3.1.0 via automation Sep 19, 2019
@Neverlord

This comment has been minimized.

Copy link
Member

commented Sep 26, 2019

I did a brief survey on the compiler landscape for the oldest still supported releases in major distros:

  • Ubuntu 16.04 LTS (xenial, support until April 2021): yum install clang-4.0
  • Arch Linux: Rolling release distro with two C++17 compilers (GCC 9 and Clang 8) available.
  • Fedora 29 (maintained until 1 month after the release of Fedora 31) ships with GCC 8.
  • Red Hat / CentOS: RHEL 6/7: e.g., install devtoolset-7 or llvm-toolset-7.
  • SuSE (SLES 12, support until Oct 2024) has GCC 7.
  • Debian 8 (Jessie, support until June 2020): ships with clang-4.0.

Hope that helps. Did I miss some distros/crucial information/link?

@jsiwek

This comment has been minimized.

Copy link
Member Author

commented Sep 26, 2019

Yeah, thanks for the helpful summary. Another main one is FreeBSD which I just checked: the release notes for all their supported version suggest they'll ship with Clang 6.0.0+.

@rsmmr

This comment has been minimized.

Copy link
Member

commented Sep 27, 2019

Great, thanks. Ready to go ahead then I think and make the change in cmake: check for minimum compiler versions and activate c++17.

What's the best place to document the notes on distros? We could just link to this ticket from CHANGES?

@jsiwek

This comment has been minimized.

Copy link
Member Author

commented Sep 27, 2019

What's the best place to document the notes on distros?

In the INSTALL file whose content lives in the zeek-docs repo: https://github.com/zeek/zeek-docs/blob/master/install/install.rst

@0xxon

This comment has been minimized.

Copy link
Member

commented Sep 27, 2019

As a small side note - it might take a while to get nightly builds on debian based systems to work again with this (mainly due to the fact that with rpm specfiles it is trivial to have per-system-changes in required packages - with the debian system it is not).

But - that is not a stopper in any way.

@grigorescu

This comment has been minimized.

Copy link
Member

commented Oct 3, 2019

If we're proceeding with this, I'd like to see Zeek's configure and cmake scripts be a bit more robust in finding newer versions of compilers/cmake. For instance, on CentOS 7, one has to install the new packages, as well as run:

sudo alternatives --install /usr/local/bin/cmake cmake /usr/bin/cmake3

This step is necessary because Zeek's configure script doesn't look for a binary named cmake3. However, this changes the default cmake instance on the system as a whole, which seems overly intrusive. (I know a user could then run update-alternatives and downgrade/upgrade as needed).

Basically, if Zeek is bumping up the requirement, I feel like it should be doing more to find the newer versions itself, and decreasing the burden on the user and the impact on the system as a whole.

@0xxon

This comment has been minimized.

Copy link
Member

commented Oct 3, 2019

I agree. Also - not everyone might have root on their systems. Our package build infrastructure would probably require me to patch the configure script to be able to compile it on that system.

@Neverlord

This comment has been minimized.

Copy link
Member

commented Oct 4, 2019

This step is necessary because Zeek's configure script doesn't look for a binary named cmake3.

Using alternatives certainly isn't pretty. Another idea instead of guessing the correct cmake path/binary: we could simply add a --with-cmake=<path> switch to the configure script. This avoids distribution-specific hacks and also enables users to use CMake in userspace locations without meddling with the PATH variable.

I agree. Also - not everyone might have root on their systems.

Spinning this a bit further: what if you don't have root access but the system's default compiler is too old? Just thinking loudly: bootstrapping your own build environment has become quite easy with LLVM. Maybe we can add a configure mode like configure --bootstrap that builds LLVM 8 locally (you can build that with compilers as old as GCC 4.8). Then we could also be more aggressive with new language features. No one's getting left behind and Zeek isn't locked into arbitrary release cycles of Linux distributions.

@0xxon

This comment has been minimized.

Copy link
Member

commented Oct 4, 2019

How about a combination of guessing and --with-cmake? I agree that --with-cmake is a good idea. It would still be nice if we could try to have the configure script work on the major distributions - without the user having to do that.

@0xxon

This comment has been minimized.

Copy link
Member

commented Oct 4, 2019

As for the second part - I actually don't really have an opinion on that; I have no clue how easy or hard it is to bootstrap llvm nowadays. One problem that I see is that people regularly have Zeek installed on system without direct internet access -- --bootstrap will probably need that I assume.

@rsmmr

This comment has been minimized.

Copy link
Member

commented Oct 7, 2019

How about a combination of guessing and --with-cmake? I agree that --with-cmake is a good idea.

Yeah, that makes sense to me, too. For "guessing", just looking for (and preferring) cmake3 would probably solve a lot of this already.

For the compiler, a --bootstrap would go a bit too far I think. Compiling a recent LLVM/clang isn't rocket science but can still be tricky enough in specific settings that I wouldn't really want to get into supporting that. Let's rather be clear in what the problem is and how to solve it ("you need XX; look how to get that").

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
Release 3.1.0
  
Unassigned / Todo
5 participants
You can’t perform that action at this time.