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

Add Azure Pipelines configuration for CI builds using CMake #172

Merged
merged 1 commit into from May 28, 2019

Conversation

Projects
None yet
4 participants
@mloskot
Copy link
Contributor

commented May 23, 2019

This AzP provides comprehensive setup for CI builds:

  • on Linux, MacOS, Windows
  • compiling with
    • clang 3.5 - 8
    • GCC 4.9 - 8
    • Visual Studio 2017, 2015, 2019
    • Xcode 8.3.3 to 10.1
  • targeting C++ standard versions: 11, 14, 17, 20 (2a)
  • configuring with CMake 3.12.4 (Linux), 3.14.3 (MacOS), 3.14.4 (Windows)
  • with CMAKE_BUILD_TYPE=Release

This configuration allows any contributor to set up CI pipelines with AzP at https://dev.azure.com/<owner>/geos and build directly from GitHub upstream (libgeos/geos) or own fork in order to test own topic branches as well the master.


/cc @dbaston

Sample build matrix available at https://dev.azure.com/mloskot/geos/_build/

@mloskot mloskot force-pushed the mloskot:ml/azure-pipelines branch from b41e2d2 to 8736cf4 May 23, 2019

@pramsey

This comment has been minimized.

Copy link
Member

commented May 23, 2019

Does it really do all those combinations? Seems... like a a lot?

@dbaston

This comment has been minimized.

Copy link
Member

commented May 23, 2019

Thanks @mloskot . For some reason I can not access the pipeline page on this PC (browser extension gone haywire) but I will check it out later to look at the output, build times, etc. Initial thought is that this should replace AppVeyor and Travis, so either needs to include autotools or wait until we are able to replace autools w/cmake. cmake versions should probably be set to the minimum (3.8 for Linux, higher for Windows). I'm also wondering why we test building w/C++14, 17, and 20 standards?

@mloskot

This comment has been minimized.

Copy link
Contributor Author

commented May 23, 2019

@pramsey Yes, it pretty much does do it all with some exceptions like no C++11 in Visual Studio or may not be building C++11 with latest GCC/clang as time-to-build optimisation or some C++ versions may be skipped where not supported by the compiler version.

image

image

As you can see in my sample pipeline, it already helps to discover C++ portability issues.

In fact, there should be more:

  1. Each build should not do just Release but Release and Debug.
  2. Three additional builds using clang and UBSan for undefined, nullability and integer checks.

Something for future.


For numeric/float/bit intensive codebases, testing variety of compilers and versions of the same compilers and C++ language support within particular compiler version does help to find bugs.
For example, in one case of image processing library ~30 versions of compilers build fine, but one particular fail to compile or an optimised binary generated by a particular version is failing checksum-based tests at run-time, etc.

@mloskot

This comment has been minimized.

Copy link
Contributor Author

commented May 23, 2019

@dbaston

Initial thought is that this should replace AppVeyor and Travis, so either needs to include autotools or wait until we are able to replace.

I am not going to propose to replace those.
I dedicate this as auxiliary CI setup for GEOS developers to test their patches from their forks in non-intrusive manner. I intentionally stick to CMake only. I intentionally did not add any badges to the README either.

I'm also wondering why we test building w/C++14, 17, and 20 standards?

GEOS requires C++11 only as minimum supported version.
It does not require C++11 as exclusively supported version.
Such requirement would be highly restricting and in fact very impractical.
For example, GCC 6.x and Clang 6.x (or later) already default to -std=c++14.

Users should be able to compile their GEOS clients as C++11 or any later version of C++.

@mloskot mloskot force-pushed the mloskot:ml/azure-pipelines branch from 8736cf4 to 872d902 May 24, 2019

@strk

This comment has been minimized.

Copy link
Member

commented May 25, 2019

The pipeline page is not usable from mobile phone (but I've no objection with merging this)

@mloskot

This comment has been minimized.

Copy link
Contributor Author

commented May 28, 2019

What's stopping the merge?

@strk strk merged commit 872d902 into libgeos:master May 28, 2019

2 checks passed

Travis CI - Pull Request Build Passed
Details
continuous-integration/appveyor/pr AppVeyor build succeeded
Details
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
You can’t perform that action at this time.