search

think-cell / think-cell-library

think-cell core library
https://www.think-cell.com/en/career/devblog/overview
Boost Software License 1.0
415 stars 51 forks source link

Allow building tc::range on linux #18

Closed philippludwig closed 7 years ago

philippludwig commented 7 years ago

This pull request comprises several changes which are necessary to build tc::range under linux. I'm aware that think-cell does not focus on this platform, so feel free to reject this PR if you are not interested in linux-specific fixes; I won't bother you any more then.

Anyway, commit 04edeff makes sure that clang++ is selected as a compiler, since g++ is usually the default one. Commit 179ce92 just changes one #include directive; this is necessary due to the case-sensitive nature of most filesystems commonly used under linux.

Commit f30c833 is not so trivial; it provides std::is_trivially_copyable when a GCC version older than 5.0 is installed. This is a useful addition since a lot of the recent, currently supported distributions (e.g. CentOS) are shipping with gcc 4.9 (or even older). If we can build tc::range under these systems, we maybe could provide pre-compiled packages for these distros, which (hopefully) would lead to a wider acceptance and usage of this library and therefore more contributions.

treh commented 7 years ago

Dear Philipp,

we would like to merge your github pull requests for the think-cell range library. However think-cell requires you to sign a contributor license agreement (based on Apache’s CLA) before continuing with that. Sorry for so much paperwork, but without that we would have to refuse any pull request you make. Furthermore your already included pull requests would have to be overwritten by the next update from our internal repository.

So it would be great if you could print, fill out, sign, scan and email the attached CLA to info@think-cell.com

Thanks for your interest in the range library and best regards, Tobias Reh

philippludwig commented 7 years ago

Dear Tobias,

I've sent the CLA as you requested. If anything else is necessary, feel free to contact me.

Best regards Philipp

philippludwig commented 7 years ago

Hi Tobias,

I've commit the changes you suggested. You were right, the lines 41 and 44 could be factored and 42 was unnecessary. I experimented with different flags when I tried building the range library under linux, that's where they came from.

treh commented 7 years ago

Hi Philipp,

Combining these commits rises a question: 04edeff (with the fixup b3e5f68) shows that we still only support either MSVC++ or clang with libc++. Under these circumstances I don't see the purpose of providing a workaround for an old gcc in f30c833.

Tobi

philippludwig commented 7 years ago

Hi Tobi,

the point is that on certain distributions clang ships without it's libc++, which causes it to use the headers supplied by libstdc++ installation. Note that in this cases clang simply ignores the request for libc++, instead of exiting with some error message (at least on the installations which I tried).

I'm not sure what yours or think-cells position of adding workarounds for these kind of configurations is; as the maintainer, you have of course the right to say that you don't want to support this. So I respect if you would rather not include this fix if it doesn't support your quality standards, especially since the latest commits to the Range library require a more recent version of boost anyway, which the distributions with these kind of issues don't provide.

Sincerely Philipp

treh commented 7 years ago

Dear Philipp,

I cherry-picked your commit 179ce92. We have also added some rationale to readme.md regarding what our aims with the library are and what pull requests are thus likely to be accepted.

Best, Tobi