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

Port the build system to cmake #94

Closed
cdbishop opened this issue Apr 10, 2015 · 167 comments
Closed

Port the build system to cmake #94

cdbishop opened this issue Apr 10, 2015 · 167 comments

Comments

@cdbishop
Copy link
Contributor

Would there be interest for a pull request which replaces the makefiles with a CMake build implementation?

This change will help maintain & improve the cross platform support.

It would be beneficial if I could build Civetweb for multiple versions of visual studio for instance.

Thoughts?

@mattyclarkson
Copy link
Contributor

I'd be all for this - will improve the sketchy MinGW and MacOSX support in the Makefiles. The added benefit would be that the library would be buildable across all Visual Studio versions 2005 - 2013 depending on support in the C/C++ code.

One build description to rule them all. Could provide a simple vs2012.bat file that would generate the current VS2012 solution files with one click. It could also automatically download the cmake binary if not found in PATH. Windows users wouldn't notice any difference to the current workflow but the project would get build support for other systems MinGW, Unix Makefile, Ninja Makefile, XCode IDE.

👍

@bel2125
Copy link
Member

bel2125 commented Apr 10, 2015

I don't know CMake - I mean, I know the name and I know there is a web site, but I never used it, and I did not even read any documentation yet. So personally, I can not add a CMake build myself - would you do this?

What does CMake actually do exactly? Is it a generator for tool / compiler specific project files / Makefile?

One problem with Mac OS X support is, I do not have a Mac, so I must mainly rely on other contributors to take care about Mac OS X support. My main development platforms are Windows and Linux. The standalone executable for Windows on SourceForge is a Visual Studio build.

Since Civetweb is mainly one .c file, the Makefile is often not too much of an issue: users that want to embed Civetweb into their C/C++ applications can simply add this .c file into their existing project files.
Still, I agree that generating all Makefiles/Projectfiles/... is easier than manually maintaining them, if this is possible. What would need to be maintained in this case, and what files could be generated?

@mattyclarkson
Copy link
Contributor

There is a Mac I have access to so can verify the build on that. CMake is just a 'meta' build system - it generates whatever you tell it to. As civetweb has just a few files to compile will make the cmakelists.txt really simple and easy to maintain.

As this is a open source project we can also submit Travis CI and appveyor support which is free online builds for Linux, MacOSX and Windows. They will build every commit to master and also each PR. Then you'll know if any pull requests will break the build on any platform.

I can work with @cdbishop to get a branch together for this support if you would be happy with those changes to the project?

@mattyclarkson
Copy link
Contributor

Bit more on CMake. You define the build files and the targets you want to build (executable/libraries) and it then has all the information needed for generating the correct build files. Its really simple and easy to maintain.

We will look into the lua and openssl dependencies as part of the branch. CMake has support for modules that find third party libraries in a nice manner. I'm almost certain popular projects such as lua and openssl will be supported.

There are generators for pretty much any build system and OS combination you can imagine! This greatly reduces the burden on the project to support build systems that users will need.

As part of the branch we will add build instructions in the README.md so that everyone has an easy migration experience.

@bel2125
Copy link
Member

bel2125 commented Apr 11, 2015

Indeed, this could be quite useful. Does this system build binaries you can also download and test?

Currently the build instructions can be found in this file:
https://github.com/bel2125/civetweb/blob/master/docs/Building.md

@mattyclarkson
Copy link
Contributor

Travis CI and appveyor both have the concept of build artefacts that are stored. These are generally the result of the build; executables, libraries, log files, etc. So you could store the built binaries for download if you like.

@bel2125
Copy link
Member

bel2125 commented Apr 13, 2015

This sounds quite interesting.
So, would it be possible to have a daily build available for download there?

Could it be used to build the archives in a form they are stored at the SourceForge page (http://sourceforge.net/projects/civetweb/) for the different platforms (e.g. http://sourceforge.net/projects/civetweb/files/1.6/ for the last release version).

@mattyclarkson
Copy link
Contributor

I'm not sure how long the CI's keep the artefacts. They both have simple methods to upload the resulting artefacts to Amazon S3 built in, if you want the deployment automatic. Would require a S3 account though. Here are the deployments they support:

@mattyclarkson
Copy link
Contributor

Probably best to use github releases:

@kainjow
Copy link
Contributor

kainjow commented Apr 14, 2015

Just adding my +1 for CMake support.

@bel2125
Copy link
Member

bel2125 commented Apr 14, 2015

I don't know if github releases are suitable as daily builds. I would download the daily built executable files for tests.
In the AppVeyor documentation, I found there are "draft releases" and "pre-releases" (whatever this means exactly), I don't know if this could be used for this.
Can Travis or AppVeyor be used to perform automatic tests on the created executable?

@kainjow
Copy link
Contributor

kainjow commented Apr 14, 2015

Yep. Travis has a "script:" field and you provide it the command to run.

@bel2125
Copy link
Member

bel2125 commented Apr 14, 2015

There is actually a "travis" file I inherited with the repository, but it's not maintained (I don't know what it actually does): https://github.com/bel2125/civetweb/blob/master/.travis.yml

@kainjow
Copy link
Contributor

kainjow commented Apr 14, 2015

Create an account at http://travis-ci.org then it tries to build all your GitHub projects that have that file in them.

@mattyclarkson
Copy link
Contributor

@bel2125, Both travis and appveyor support post build tests - I was planning to port the tests to CMakeTest so that they are correctly translated into each of the generated build files.

I've started a branch at vcatechnology/civetweb@cmake, I'll keep working on it when I have time.

@bel2125
Copy link
Member

bel2125 commented Apr 16, 2015

Which system would you recommend: Travis or AppVeyor?

@mattyclarkson
Copy link
Contributor

@bel2125, both. Travis CI only supports Linux and MacOSX. Appveyor supports Windows. I recently did the appveyor support for google/benchmark#64 (It already had travis CI support)

@bel2125
Copy link
Member

bel2125 commented Apr 16, 2015

OK. So we will need both anyway.
Both can work with GitHub repositories,
and both are for free for open source projects,
right?

@mattyclarkson
Copy link
Contributor

Yep! All free and continuous. Also, any pull requests are automatically built so you know before you merge if it'll break the build.

@mattyclarkson
Copy link
Contributor

@bel2125, I've pushed up the first tiny bootstrapping to the cmake branch.

git remote add vcatechnology git@github.com:vcatechnology/civetweb.git
git fetch vcatechnology
git checkout cmake
./build # A shell script that does a instant build
rm -rf output
./build --shared

It adds a load of warning flags if the compiler supports them. -Wshadow catches an error in #98.

@mattyclarkson
Copy link
Contributor

Things to do:

  • Remove Visual Studio solution files
  • Remove CMake files
  • Add MinGW download script
  • CMake modules to check for compilation flags
  • Basic build support (no extra options)
  • FindLibRt.cmake module
  • FindLibM.cmake module
  • FindLibDl.cmake module
  • Enable building of standalone executable
  • Enable building websockets
  • Enable building IP version 6 support
  • Build C++ wrappers
  • Enable building Lua support with CMake
  • Remove old method of building Lua
  • Port unit tests to CMakeTest (in progress, coveralls reports coverage)
  • Visual Studio support
  • build.cmd that uses CMake to generate VS solution then MSbuild to build the library
  • Travis CI support
  • appveyor support
  • Update docs with build instructions
  • Remove old build methods in favour of CMake build (Make, VS, buildroot, XCode, Android, ...)
  • Lots of other stuff I haven't thought about yet... 😨

@bel2125
Copy link
Member

bel2125 commented Apr 23, 2015

[X] being fascinated, that check boxes can be added as comments
[ ] being able to make nice check boxes on my own
[X] being grateful for your work

;-)

@mattyclarkson
Copy link
Contributor

@bel2125, gotta put a - before the [x] and [ ] 😉!

@mattyclarkson
Copy link
Contributor

@bel2125, how on earth do you keep track of not breaking anything in this project? There are so many different paths the compilation can go through with the different features that can get turned on. The only way I can see this being checked is to build all configurations in Travis CI and appveyor. What have you been doing up until now?

@bel2125
Copy link
Member

bel2125 commented Apr 24, 2015

;-))

I use different development environments. Sometimes I use Visual Studio 2010 Express on an XP PC, which I consider a "minimal Windows". Sometimes I use a rather plain Ubuntu, sometimes Qt on Win8, VS2012 on Win7, then I have a couple of VMs, ... ARM Devices ... Sometimes I work here, then I work there. And every time I switch from one machine to another I rebuilt all projects to get the different #ifdef combinations tested.
Certain compilers that I do not have are sometimes not working - like MinGW (there have been a couple of issues that are now solved), like today a Solaris Compiler I never heard of (#106), and like MacOS (I don't have a device to test), where I have to rely on other contributors.

This way, the server stays free from compile errors. Or better, if some code is introduced that compiles only on one machine, after a short while when I move to another one it will be corrected.
But it is not always warning free, so there are sometimes issues, (e.g., recently #103, #104, #105), and since I also do merge code from other contributors that probably only use one compiler and one #ifdef combination, being completely free from warnings in every case does not seem reachable (that's what I meant a week ago in my comment #104 (comment) - I don't want to introduce new errors by adding #ifdefs to remove warnings for compilers I don't have).

@bel2125
Copy link
Member

bel2125 commented Apr 25, 2015

One of the most important tests for me is that the selected examples are building.
These are some old examples that I inherited with the repository, which are no longer maintained.

Probably I should sort the repository into maintained examples and old ones, and maybe even remove the old ones one I am sure they do not show anything that is not also shown in an actively maintained example. For windows, the active examples are the ones in the solution file. For Linux I have a script.

@mattyclarkson
Copy link
Contributor

@bel2125, OK. I'll try and set up the build servers to build all combinations so they are always tested. That'll reduce the issue noise for "You broke my build on [insert OS], [insert compiler]". The can't cover all combinations (especially ARM devices) but should help out a lot.

I think we should remove anything from the repository that isn't maintained (examples, tests, etc) and add them back in if they are going to be maintained. I'm finding it a little confusing what is the 'correct' things to be porting to the new build system!

I've got the build working for the plain build (no websockets, no ipv6, etc) and will port over the unit tests to CTest (CMake's testing environment). If I then set up the build server do you think you could work with me to sort out the compilation issues. I have quite a few unused function, shadowing variables and function pointer casting errors on GCC. I have a few fixes in my branch but need to run them past you as I don't know the history of the project.

@bel2125
Copy link
Member

bel2125 commented Apr 27, 2015

I can start to move the unsupported files in an "obsolete" directory. I would delete them when I'm sure that everything shown there is shown in a new, maintained example as well.
If I remove them now and add them later, I think I am loosing all the change history of these files.

Or I just rename "examples" to "examples_old", create a new "examples" directory and move them back one by one.

Should I have a look at some fixes you needed?

@nihildeb
Copy link
Contributor

I am just now reading this, after implementing the Travis CI support to use the make file and run some tests. I am familiar with CMake and would be happy to help here. I don't do Windows, so that doesn't help on many of the issues. I can update the Travis stuff I've done as soon as the CMakeLists is committed, just ping me here.

@kainjow
Copy link
Contributor

kainjow commented Mar 24, 2016

Should be fixed for latest HEAD.

@bel2125
Copy link
Member

bel2125 commented Mar 24, 2016

"Should be fixed" means I should add the patch mentioned in this comment (libcheck/check#26 (comment)) somewhere in a file in civetweb?
Or I don`t have to do anything and everything will be fine?

@kainjow
Copy link
Contributor

kainjow commented Mar 24, 2016

You could add the patch, or update civetweb to load the library from git directly instead of the archive.

@bel2125
Copy link
Member

bel2125 commented Mar 24, 2016

Probably using a released version of the check library instead of the development version is the better choice in the long term. I don't want to have civetweb builds failing because I did use some unstable intermediate development version of the check library.

@jd-boyd
Copy link
Contributor

jd-boyd commented Mar 24, 2016

I suggest reverting 2a90968, and then using libcheck 0.10.0 and applying libcheck/check@c82fe88 (taken from libcheck/check#29 ).

libcheck/check@c82fe88 applies cleanly to 0.10.0 when cherry picked, so I'd imagine it would work just as well when extracted to a .patch file to be applied with the patch utility in the PATCH_COMMAND argument in the ExternalProject_Add call in the test CMakeLists.txt file.

The URLs for 0.10.0 and the patch are:
https://github.com/libcheck/check/files/71408/check-0.10.0.tar.gz
https://github.com/libcheck/check/commit/c82fe8888aacfe784476112edd3878256d2e30bc.patch

I can make a pull request following this suggestion later.

@bel2125
Copy link
Member

bel2125 commented Mar 24, 2016

I just wanted to know if it works with the current version of check on github - I did not want to use this as a default on the long term, but switch to a released version later.

A pull request with released version + patch would be welcome.

jd-boyd added a commit to jd-boyd/civetweb that referenced this issue Mar 27, 2016
jd-boyd added a commit to jd-boyd/civetweb that referenced this issue Mar 27, 2016
jd-boyd added a commit to jd-boyd/civetweb that referenced this issue Mar 27, 2016
Revert "Test newest development version of libckeck (see civetweb#94)"

This reverts commit 2a90968.
jd-boyd added a commit to jd-boyd/civetweb that referenced this issue Mar 27, 2016
Revert "Test newest development version of libckeck (see civetweb#94)"

This reverts commit 2a90968.
@jd-boyd
Copy link
Contributor

jd-boyd commented Mar 27, 2016

A pull request with released version + patch would be welcome.

I'm not sure that it nails civetweb's osx issues, but #286 updates to the latest released libcheck and adds the patch that was needed to make libcheck's own tests work on OSX.

@bel2125
Copy link
Member

bel2125 commented Mar 27, 2016

Thank you for the patch.
Once this problem is fixed, we will see if there are other issues for OSX as well.

@bel2125
Copy link
Member

bel2125 commented Mar 28, 2016

The current reason OSX does not build seems to be related to an "unreachable code" warning in the unit test:
https://travis-ci.org/civetweb/civetweb/jobs/118886665#L563

/Users/travis/build/civetweb/civetweb/test/public_server.c:117:9: error: 'return' will never be executed [-Werror,-Wunreachable-code-return]

Since we build with warnings as errors, the entire build fails.
For whatever reason, this only applies to OSX and no other code.
Actually there is even a code to disable this warning temporarily, but it does not seem to work:
https://github.com/civetweb/civetweb/blob/master/test/public_server.c#L108

#pragma GCC diagnostic ignored "-Wunreachable-code"

GCC does not seem to respect this pragma

It actualy does not state -Wunreachable-code but -Wunreachable-code-return, however, using ```
#pragma GCC diagnostic ignored "-Wunreachable-code-return"

yields in a different warning:
https://travis-ci.org/civetweb/civetweb/jobs/118885608#L395

/home/travis/build/civetweb/civetweb/test/public_server.c:108:9: error: unknown option after ‘#pragma GCC diagnostic’ kind [-Werror=pragmas]
cc1: all warnings being treated as errors

So all builds fail when using `-Wunreachable-code-return` in the pragma.

Does anyone know a way to disable all warnings for the OSX builds only?

@bel2125
Copy link
Member

bel2125 commented Mar 28, 2016

Formating went wrong in the message above, and the edit button does not work now ...
Anyway, ...

I want to disable warning = error for OSX - the warnings should show up but not abort the OSX build. The other systems ...Linux GCC + clang and Windows/Visual Studio should still build with warnings as errors.
Furthermore, even if an OSX builds fails, it should not cause the entire build to fail. I tried this with an allow_failure entry in the .travis.yml file, but this does not seem to work either: https://github.com/civetweb/civetweb/blob/master/.travis.yml#L2329

The builds for Linux are stable, and the CI works well for them - the build for OSX is experimental, and since I do not have an OSX device available locally, I can not even do much to improve this.
So it may take some time, until the OSX build is stable, and I don't want the Linux build to be affected by failures of the OSX build.

Otherwise I would have to deactivate the OSX builds again.

@kainjow
Copy link
Contributor

kainjow commented Mar 28, 2016

Looks like in the top-level CMakeLists.txt CIVETWEB_ALLOW_WARNINGS is set OFF by default. Maybe you want ON there?

@bel2125
Copy link
Member

bel2125 commented Mar 31, 2016

Allowing warnings for OSX only worked well. Thank you for this hint.

However, it seems the check framework has another issue with OSX:
https://travis-ci.org/civetweb/civetweb/jobs/119610774#L587

Undefined symbols for architecture x86_64:

  "_timer_create", referenced from:

      _tcase_run_tfun_fork in libcheck.a(check_run.c.o)

  "_timer_delete", referenced from:

      _tcase_run_tfun_fork in libcheck.a(check_run.c.o)

  "_timer_settime", referenced from:

      _tcase_run_tfun_fork in libcheck.a(check_run.c.o)

ld: symbol(s) not found for architecture x86_64

clang: error: linker command failed with exit code 1 (use -v to see invocation)

@bel2125
Copy link
Member

bel2125 commented Apr 2, 2016

In the current TravisCI build, all configurations work, except those for OSX.
It seems the check library does not have a timer code for OSX (see last comment).

Unfortunetely the allow_failure configuration (see comment from 5 days ago) does not work, so the overall buils status is allways failed.
It does not work like stated in the documentation (https://docs.travis-ci.com/user/multi-os/), however, even the documentation states "The feature described in this document is considered beta. Some features may not work as described."

I think I'll have to remove OSX again from the build script.

bel2125 added a commit that referenced this issue Apr 2, 2016
Currently TravisCI only offers experimental support for OSX,
the check unit test framework does not work for OSX now,
and I can not do the tests locally.
See #94
@kainjow
Copy link
Contributor

kainjow commented May 15, 2016

Since the OS X CMake build is functioning now, I think this can be closed?

@bel2125
Copy link
Member

bel2125 commented May 16, 2016

This thread was created for the entire transition to CMake, the Travis CI integration (Linux and OSX), AppVeyor (CI for Windows) and Coveralls (Test coverage check), including building third_party components like Lua with CMake.
There is still an open TODO list (see #94 (comment)), although this list is not up to date.
E.g. "IPv6" works, but "Enable building Lua support with CMake" still does not work. So I still have to do all builds and tests with Lua manually locally.

But after more than a year and 165 comments, this issue became somewhat overloaded and unclear.
So probably it is better to create new issues for remaining open topics.

  • AppVeyor already has a new issue Appveyor tests failing #278.
  • Coveralls remains red until 70% test coverage is reached - I probably don't need a new issue for this.
  • For Lua, a way to do an automated CI test would be desirable. I do it manually, The current using the current Makefile and the Visual Studio project. That's why I cannot remove the old build instructions, but now have to maintain both, the new CMake build and the previous builds. Probably this can accepted until including the next release (1.8).
  • Probably there are other topics mentioned here as well ... somewhere in the 165 posts. On the other hand, if they are still important, they will come up again.

@bel2125
Copy link
Member

bel2125 commented May 26, 2016

I think, the CMake integration has reached a state where I can consider this issue as closed.
In particular, unfinished CMake build issues should no longer stop the 1.8 release (#265).

  • The remaining AppVeyor problems in civetweb (Appveyor tests failing #278) now seem to be caused by AppVeyor, not civetweb
  • Coverage reached 72% (https://coveralls.io/jobs/14813964):
    70.79% src/civetweb.c
    74.73% src/handle_form.inl
    95.35% src/md5.inl
    The badge is still red, so the color threshold seems to be higher.
    I think a coverage of 100% is not reachable for technical reasons. Some lines are falsely detected as "not covered". Details: Test coverage of forking programs lemurheavy/coveralls-public#785
  • There is no CMake build for the Lua integration.
    I will not make one by myself. I am not a build system expert, and I will never be - most of the time I can deal reasonably well with the existing CMake integration, but I don't feel able to modify the build so external components can be integrated. This also means the existing Makefile and the Visual Studio project will stay with us for undefined time.
  • Maybe something got lost in the 165 posts above, maybe not. If I missed something important, please create a new issue. This issue here is open for too long an has too much posts, so it's no longer handy to track sub-issues here.

@bel2125 bel2125 closed this as completed May 26, 2016
hunyadi-dev pushed a commit to hunyadi-dev/civetweb that referenced this issue Dec 18, 2020
…gnore.

This closes civetweb#94

Signed-off-by: Tony Kurc <tkurc@apache.org>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

6 participants