Compiler choice limited
Currently I do not see any way to build this with MinGW on windows or Clang on Linux. I would like to use Clang for the code analysis tools, and on Windows I have a sophisticated project that uses C++14 which is not well supported by visual studio.
Having the compiler hardcoded in the makefile does not seem like the best way to do things. One possible solution, which seems common is to use a project file creation tool like Autoconf or CMake.
Possible Pros: Less maintenance of several project files, Larger selection of buildchains, more idiomatic build could ease developer transition, build would work on linuxes
Possible Cons: This would need to be documented, you would gain a dependency.
If I made a CMakeLists.txt that can emit all the Visual Studio solutions and working Makefiles on windows/Linux/MacOSX and allow arbitrary selection of compiler and build chain would you be receptive of that? I would also try to help with the documentation.
Hey there, I wouldn't really say that the compiler choices are limited in the sense that they're not supported. I just haven't tested them and they're not supported out of the box. But there's nothing stopping people from modifying the existing makefiles or integrating robot directly into their projects.
That being said, I don't have any experience with Autoconf / CMake but I'd be open to any solution that is beneficial to the project, that isn't overly complicated, and that I can understand how to document and use. Ultimately, I just want to be able to understand how the workflow would change and what the benefits of such a system would be.
One thing I would be opposed to, however, is auto-generating visual studio projects. I'm not sure why you'd need that. There could still be a makefile but I just need those solution files to stay the way they are to make my life easier.
Ok.
I only suggested removing the visual studio projects because they would be redundant and require separate maintanence. There is no reason they must be removed. Though the CMakeLists.txt I made already works with the community edition of visual studio 2015.
I forked Robot already and I the CMakelists.txt created that I can use to integrate Robot into my Linux build, but not yet the windows side with mingw. I am having trouble with building with mingw because of some std::mutex issues.
There are also some differences I would still like to resolve or discuss with you to see if I should skip resolving them. I will assemble a list and ask you about them once I have Mingw working and Clang tested.
Edit - Also thanks for being receptive to outside changes and criticisms. Maintaining build scripts or any patchsets separately is a huge pain.
No changes were needed and Clang++ works on Ubuntu.
No changes were needed for ninja build system to work on Ubuntu.
To be fair though I did not run all the tests, Types, Timers, Keyboard worked and others would interfere with things I am doing.
I don't think I used std::mutex yet, but I might in the future for memory functions (#5). I suspect that's when MSVC 2010 support will go out the window, among other compilers. If that's the case I'll probably convert some for loops to for-each as well.
Thanks again for contributing, whatever makes things easier for people. I will be collecting patches through pull requests to the dev branch. Please understand that I'd like to avoid doing too many smaller releases but instead focus on larger, more stable releases. That way, less time is spent running tests and synchronizing the ports to other languages. So please take as much time as you need!
For testing I suggest running ./test types timer, if those work then there's a good chance everything else will work as well. I might improve those tests in the future though, they're a bit too time consuming for certain things.
OK, that is good to know, so I might be able to make just a tiny change somewhere that prevents the inclusion of the offending std header I can likely fix the MinGW build issue.
Looking forward, If you want I have a mutex implementation that uses atomic integers to implement a simple spinlock. Reading #5 a spinlock might be ideal, because they use CPU instructions, which are cheap, rather than operating system calls, which tend to be expensive and you just need it to last long enough to write some memory. I used it over here: https://github.com/BlackToppStudios/DAGFrameScheduler/blob/master/src/mutex.h This is GPL'd but I have permission from BlackTopp Studios inc to copy small snippets for other project and re-license them. You could conditionally compile the mutex in when building without C++11 or on old versions of msvc.
I think I forked your master, I will import changes from your dev branch.
No changes yet from dev so you're all good.
I like that mutex implementation, it's small and efficient and something I can directly embed as part of Robot (given appropriate permissions). In fact, it's a coincidence because I just read about CriticalSections last week! It might be cool to expose it's API but I'm not sure since Robot isn't a threading library, it should probably remain private.
If I recall correctly CriticalSections boil down to memory fences which is how most C++ compilers implement atomic operations. So they are super fast, but not readily portable.
I spent way to long not carefully reading the build error. This is simple in principle, I am not sure about the fix yet.
[8/25] Building CXX object CMakeFiles/Robot.dir/Source/Hash.cc.obj
FAILED: C:\TDM-GCC-64\bin\c++.exe -IC:/Users/builder/Desktop/robot/Source -IC:/Users/builder/Desktop/robot/Test -std=
c++11 -Wall -pedantic-errors -fno-rtti -MMD -MT CMakeFiles/Robot.dir/Source/Hash.cc.obj -MF CMakeFiles/Robot.dir/Source/
Hash.cc.obj.d -o CMakeFiles/Robot.dir/Source/Hash.cc.obj -c C:/Users/builder/Desktop/robot/Source/Hash.cc
In file included from C:/TDM-GCC-64/lib/gcc/x86_64-w64-mingw32/5.1.0/include/c++/memory:74:0,
from C:/Users/builder/Desktop/robot/Source/process.h:17,
from C:/TDM-GCC-64/x86_64-w64-mingw32/include/pthread.h:66,
from C:/TDM-GCC-64/lib/gcc/x86_64-w64-mingw32/5.1.0/include/c++/x86_64-w64-mingw32/bits/gthr-default.h:
34,
from C:/TDM-GCC-64/lib/gcc/x86_64-w64-mingw32/5.1.0/include/c++/x86_64-w64-mingw32/bits/gthr.h:148,
from C:/TDM-GCC-64/lib/gcc/x86_64-w64-mingw32/5.1.0/include/c++/ext/atomicity.h:35,
from C:/TDM-GCC-64/lib/gcc/x86_64-w64-mingw32/5.1.0/include/c++/bits/ios_base.h:39,
from C:/TDM-GCC-64/lib/gcc/x86_64-w64-mingw32/5.1.0/include/c++/ios:42,
from C:/TDM-GCC-64/lib/gcc/x86_64-w64-mingw32/5.1.0/include/c++/istream:38,
from C:/TDM-GCC-64/lib/gcc/x86_64-w64-mingw32/5.1.0/include/c++/fstream:38,
from C:/Users/builder/Desktop/robot/Source/Hash.cc:16:
C:/TDM-GCC-64/lib/gcc/x86_64-w64-mingw32/5.1.0/include/c++/ext/concurrence.h:124:5: error: '__gthread_mutex_t' does not
name a type
__gthread_mutex_t _M_mutex;
^
like
How I read that was that I have screwed with the header search order and it found "Process.h" in Robot instead of the normal compiler search path. I suspect the only reason it did not do this on Linux is because the filesystem is case sensitive and it didn't break on mscv because they used different header names or isolate their header search.
I am going to see if I can reorder the include for system headers, or prevent system headers from looking at robot stuff.
Oh, I almost forgot. There are a lot of things like this:
#ifdef ROBOT_OS_WIN
#pragma warning (push)
// Ignore the VS C4251 warning
#pragma warning (disable:4251)
#endif
This breaks because MinGW is GCC and does not have the same pragma syntax as msvc. So I added a VS_COMPILER definition in global.h and I am using it as follows:
#ifdef VS_COMPILER
#pragma warning (push)
// Ignore the VS C4251 warning
#pragma warning (disable:4251)
#endif
EDIT - So I just learned about -iquote - https://gcc.gnu.org/onlinedocs/gcc/Directory-Options.html
Ha ha, now you see why I was so reluctant to support other compilers. By the way, if we're adding compiler defines in global, we're gonna do them like Qt does it and make a define for all supported compilers. I want to keep the implementation as clean and consistent as possible without having random defines everywhere. I don't know about other errors you're getting though, still surprised to see mutex there. I know I had a big problem with X11 Window conflicting with Robot::Window, which is why Robot must have a namespace :-P
I have done a few cross platform projects. They are always a mess because each platform and compiler maker choose different things to compromise on.
Presently I just have the two for vc style and nix style. It is my understanding that ICC (the intel compiler) accepts msvc style pragmas and arguments on windows but uses arguments and pragmas compatible with clang on posix platforms. So I just made the two, nix and vs compatible. This is useful to prevent complex #if statements from being scattered about the code that change as compilers change. Also I am curious about trying clang as shipped by microsoft in vc 2015.
That said, there are plenty of uses for a per compiler definition. I can go back and add that. Should I do it now?
Screw it, I am doing it now.
Mingw, GCC, msvc[12,13,14] and clang are all of them we care about right?
Absolutely! Do whatever you think is right and I will be more than happy to integrate it into the project during the next release cycle. I don't really have a huge preference on compiler support, it's something I'll have to look at more closely but I think you know best.
I still need to add Peon build support .
I should at least make a token attempt to add mac os X support. Though I won't really be able to test.
How picky are you about the binary names? I just left the default names that CMake produced from the "Robot" and "Test" I put in, so there is no "d" on debug libs and no ".BIN". I find extensions of executables a little silly because Posix guarantees and execute bit is present and this is what is checked when attempting to run a program or color highlight at the terminal. It wouldn't be hard to add this one though if you thought it worthwhile.
On the case of the "d", I find it counterproductive because it makes it hard to use a debug library with something looking for the non-debug library. I sometimes cannot get the app looking for dynamic lib to find it without rebuilding, which seems to defeat one of the main advantages of shared libs. Perhaps I am doing something wrong, but I think the built shared object knows its "libname".
What kind of documentation should I get you? Do you know how to use CMake at all?
Oh right, you should actually skip Peon. It's a pre-built application used for testing the Memory class. It might even just go away into it's own repository one day. It should never be built with other compilers because it's dependent on using specific offsets for testing purposes.
Do we need Mac support? Isn't everything compiled through clang/xcode?
I'd like to keep the names consistent. Not because I'm picky, but because it maintains consistency across all platforms. Since in the Windows world it's always App.exe and Appd.exe, having something without an extension on another platform makes it hard to tell what type of file it is and mode used to compile it (i.e. release or debug).
I'm more open to removing the "d" if you're saying it's that big of a problem. I'm more wondering why anybody would use the debug version of Robot. But I'm clueless when it comes to knowing what people actually do.
I'd like an outline of what changed, what the result of the change is, and how to test it. You can see the current documentation here and here for examples. The idea is to explain to a person like me, who never used cmake before how to build the library if they choose to use other compilers.
In that past I have swapped in debugging versions of libraries so I could understand how an application was calling it and to literally debug it. It not common a thing by I have found reasons to do it more than once. I will go add the .BIN.
So far I have only added. I so I will write up an outline you can turn into a doc like what you showed me that describes how to build Robot using the CMake script I added. I will described how to use it at from the command prompt and from the gui tool. Give me some time on this.
I guess I should also describe the Preprocessor macros, though most of them are just waiting for future use. You might wind up adding a page or something to your API docs.
Also, I just integrated Robot into the larger build of my main project. My main project has much more strict compiler warnings. Would you be interested in getting a more thorough list of warnings? It found 346 items.
Please, take all the time you need! There is no release schedule, a new release will be made when it's good and ready. That and I need a small break from the project after working on it for so many years :-P
I'd be interested in fixing as many warnings as possible. But within reason, I find a lot of them are quite useless. I'm not sure a large dump would be helpful, but if I could setup a similar environment to yours then perhaps I can take care of them that way.
I can easily make a commit where I add the warnings to the CMakeLists.txt and when you try a build on Linux or with Mingw with the outline/docs I will provide you should get the same warnings I got. Then you can decide what should and what should go.
Also on Windows I linked the robot library against Shlwapi and psapi, does that seems correct to you? Did I miss anything that would cause long term problems.
Do you enable any compiler options in visual studio other than /W3?
Yeah there are some configurations I changed in Visual Studio. Nothing too crazy, I think if you open the solution it gives you the make configuration in the console view.
Though one thing to consider is how we'd deal with compiling lib vs dll. Currently on Linux & Mac, Robot is always built as a static library. On windows, people get the choice to compile a dll. I'm pretty sure you'd be able to implement that right? something along the lines of make mode=debug type=x build where x is lib/static or dll/dynamic. Though I'm wondering if people even want to compile robot as a dll.
Choosing between Static and Dynamic linking is already is implemented and reasonably tested. Let me share some details, this might shed some light on my perspective, and might be useful somewhere in the documentation.
CMake is a Domain Specific Library for describing the build steps software. It has specific features for describing things like linking and exposing these as options to the developer (or whoever is building software).
Currently in when building robot with CMake and from the sqeaky/robot dev branch when using the GUI the developer is presented a checkbox labeled "BuildStatic". It defaults to checked. When the user hits the "Generate" button the resulting build scripts will build static libraries, when unchecked it will generate dynamic libraries.
When using the command line it also defaults to building static and this can be changed by calling cmake with "-DBuildStatic=OFF" to force dynamic linkage. If integrated into a build system or something else rigged to pass something then "-DBuildStatic=ON" will cause no change. If we change the default then "-DBuildStatic=ON" would force static linkage.
This is not a cmake default setting, you can see where I added this at: https://github.com/Sqeaky/robot/blob/master/CMakeLists.txt#L189 and where it is used: https://github.com/Sqeaky/robot/blob/master/CMakeLists.txt#L236
The CMake command "add_library" accepts either the word "SHARED" or "STATIC" then adjusts whatever is required in the emitted build scripts. So the checkbox I added just connects that single to one of those words and then we don't need to worry about (most of) the details.
I have tested this one Linux and windows with all vc14, clang, GCC and MinGW in static and dynamic settings.
As for what people will want to, I never try to limit what others do with software I publish. People always find reasons (even reasons I disagree with) to do things. If they want to link whatever way, why bother arguing it?
As for your break, are you finding my questions annoying or interrupting?
Not at all, if anything I appreciate all the time you've been putting into the project. Answering your questions is the least I can do. I just regret not being super knowledgeable about these technologies, but I'll do my best to learn and be accommodating. The information you've been providing is great and will help me improve the documentation.
I am writing some cmake Docs I know you just asked for an outline, but I have a bit more. Should I make a markdown file and commit it or just make a comment here?
Probably easier to just include the docs as part of the pull request. It'll eventually be rolled into the documentation website,
OK, I am adding a RobotCMake.md to my fork. I will also add an option to enable/disable all the extra warnings that should only take a few minutes, finish the docs, and test a bit then this ought to be done.
Super, Thanks again for your contribution!
Okay, I think I'm going to be returning to the project early June and I have a lot of features I want to add to the new release, your project being one of them! I might also drop MSVC 2010 if it gets in the way. I'll open up a milestone with some of my goals in the meantime!
I think I completed this and I committed the results to my fork. The main documentation file is RobotCMake.md and there are some screenshots in the "Images" folder.
I also found and fixed what I hope was an esoteric bug in the CMake script.
I still have not tested Mac OS X (I don't have a Mac) or done anything with Peon (as per your instructions).
Great, I'll take a look. Thanks again for your contribution.
You are welcome, with it being maintained in the main code we won't need to do as much with it.
I am sure there will be issues, let me know and I will do what I can.
I went back and fleshed out some items I was initially less sure of in the mingw build. I got the guts of process listing working correctly despite the use of functions not included with the mingw windows headers.
QueryFullProcessImageName is a Vista/Server 2008 and newer function. Best practices would lead me to believe that I should include a compile time option for using such a thing. Since no such option exists for vs builds it implies this won't work on XP at all. I am just guessing that is OK and I don't need an extra preprocessor check and an new build option, but I figured I should check with you.
I have added this in my repo in commit 227cdfc648.
EDIT - What happened in my commit with spaces?! This hasn't happened before.