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

build/ops: use devtoolset-7 on centos/rhel-7 #18863

Merged
merged 4 commits into from Dec 5, 2017

Conversation

tchaikov
Copy link
Contributor

No description provided.

@tchaikov
Copy link
Contributor Author

see also ceph/ceph-build#909

" HAVE_GLIBCXX_5_1)
if(NOT HAVE_GLIBCXX_5_1)
# libc++'s list::size() is O(1) also.
message(WARNING "performance regression is expected due to an O(n) implementation of 'std::list::size()' in libstdc++ older than 5.1.0")
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

should this change to ERROR then?

Copy link
Contributor Author

@tchaikov tchaikov Nov 10, 2017

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

then, we need to use ubuntu-toolchain-r ppa repo on trusty.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Wait, Mimic will still support trusty?

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hmm, I'm not sure we've discussed it. Is trusty the only one that is problematic here? I would very happily drop trusty!

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

b/c xenial comes with gcc 5.3, and centos7 will be using devtoolset-7, (hope @ktdreyer is fine with this). and current debian stable (stretch) offers gcc 6.3. the debian's old stable (jessie) offers gcc 4.9. SLE 11 has gcc-6.

but i don't think serious user of centos/rhel 7 and debian jessie would ignore the performance regression warning and continue using whatever the toolchain the distro offers.

so the only blocker is ubuntu trusty. and it's fixable, we just need to update the pbuilderrc script and set the DEB_CC and DEB_CXX env variables. i am still looking into this.

but if we can drop the trusty, that'd be easier.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Let's drop trusty.

Would you please not use devtoolset-7 until we can discuss it directly with the team at Red Hat that maintains that? There is no published EOL date for that, so it could become unsupported at any time, and their security problems become ours.

The last time I talked with the SCL team they were very hesitant to publish clear EOL dates on centos.org. We need to bring this up with them again.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

SLE 11 has gcc-6

@tchaikov I guess you meant SLE 12 (the current version). SLE 11 is very old and no recent Ceph runs on it. I checked and the current SLE-12-SP3 has gcc 6.2.1.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

thanks for confirming @smithfarm! no, i did mean SLE 11. b/c i found that SLE-11.4's EOL is 2019-03-31, that's why i thought we should consider SLE 11.4. and i was wrong, it has gcc-5.2 not gcc-6, per the release notes

anyway, it's great that we don't need to worry about SLE =)

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@ktdreyer sorry for using devtoolset-7 without any notice beforehand. i thought it's the future of devtoolset and will be well supported. may i know which devtoolset is supported? so we can use it?

@liewegas
Copy link
Member

If this works (install-deps.sh does the right thing on ubuntu and centos) I think we can probably turn the O(n) size() into an error instead of a warning? Then we can live happily ever after in C++11 land!

@tchaikov tchaikov force-pushed the wip-devtoolset-7 branch 3 times, most recently from f1cbb67 to d435396 Compare November 11, 2017 01:41
}
" HAVE_GLIBCXX_5_1)
if(NOT HAVE_GLIBCXX_5_1)
# libc++'s list::size() is O(1) also.
Copy link
Contributor Author

@tchaikov tchaikov Nov 11, 2017

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

and libstdc++ 5.1 will also bring us a C++11 standard compliant regex implementation. see http://tracker.ceph.com/issues/20112

@tchaikov tchaikov force-pushed the wip-devtoolset-7 branch 4 times, most recently from ce13c82 to 35ad383 Compare November 12, 2017 03:56
@tchaikov
Copy link
Contributor Author

https://shaman.ceph.com/builds/ceph/wip-devtoolset-7-kefu/ centos builds are green.

@tchaikov
Copy link
Contributor Author

change on trusty side, ceph/ceph-build#914.

ready them for gcc7 and libstdc++-7 for better performance.

Signed-off-by: Kefu Chai <kchai@redhat.com>
Signed-off-by: Kefu Chai <kchai@redhat.com>
for better performance.

Signed-off-by: Kefu Chai <kchai@redhat.com>
* always install gcc-7 on trusty
* point g++ to g++-7 if not yet

Signed-off-by: Kefu Chai <kchai@redhat.com>
@tchaikov
Copy link
Contributor Author

tchaikov commented Dec 5, 2017

@liewegas @ktdreyer ping?

Copy link
Member

@liewegas liewegas left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

lgtm!

@liewegas
Copy link
Member

liewegas commented Dec 5, 2017

While we're at it.. what would it take to get to C++14? As long as we're using custom toolchains we may as well modernize!

@tchaikov
Copy link
Contributor Author

tchaikov commented Dec 5, 2017

@liewegas i will check the C++14 support matrix of GCC. what features we are after in C++14 in our minds? @adamemerson @cbodley

@tchaikov
Copy link
Contributor Author

tchaikov commented Dec 5, 2017

@tchaikov
Copy link
Contributor Author

tchaikov commented Dec 5, 2017

@tchaikov tchaikov merged commit 7c3906c into ceph:master Dec 5, 2017
@tchaikov tchaikov deleted the wip-devtoolset-7 branch December 5, 2017 13:27
@ktdreyer
Copy link
Member

ktdreyer commented Dec 5, 2017

What is the impact to qemu when we build librbd1 like this? Can users continue to use a qemu that is built with the older GCC that dynamically links to librbd1 built with the newer toolchain?

xiexingguo pushed a commit to ceph/ceph-ci that referenced this pull request Jan 17, 2018
    ceph/ceph#19548

cmake: check gcc version not release date for libstdc++ saneness
cmake: bail out if GCC version is less than 5.1

    ceph/ceph#18938
    ceph/ceph#19344

build/ops: use devtoolset-7 on centos/rhel-7
ceph.spec: use devtoolset-6-gcc-c++ on aarch64

    ceph/ceph#18863
    ceph/ceph#19341

install-deps: use DTS-7 on aarch64 and only download mirrored package indexes

    ceph/ceph#19645

Signed-off-by: luo.runbing <luo.runbing@zte.com.cn>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
4 participants