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

Unified CMake build system #35

Closed
wants to merge 4 commits into from
Closed

Conversation

scyptnex
Copy link
Contributor

i.e. the one we have been discussing

I've added a paragraph in the readme on how to build/use the cmake system

Changes to the documentation on the website come in a separate PR to that branch

@tsjensen
Copy link
Member

Thanks! Will need a few days to review, again ...
(Btw, you don't need to start new PRs every time; just update your branch, and the PR will follow.)

@scyptnex
Copy link
Contributor Author

yeah, but i dont have a local copy of mac, so i test it by pushing about 20 commits to travis and seeing how they go, closing the PR cuts down on spam to your message box

@tsjensen
Copy link
Member

Finally got around to boxes again. I see there was much progress 😄, but I still ran into a few problems:

  1. .gitignore does not contain the build directory that you recommend creating
  2. Python should be removed from the Travis dependency list
  3. Running the test suite did not work for me. In MinGW, after successful build, I tried:
    $ make test
    cd test; ./testrunner.sh -suite
    Usage: testrunner.sh {-suite | <executable> <configFile> <testCaseFile>}
           Returns 0 for success, else non-zero
    <...snip...>
    Usage: testrunner.sh {-suite | <executable> <configFile> <testCaseFile>}
           Returns 0 for success, else non-zero
    84 tests executed, 0 successful, 84 failed.
    make: *** [test] Error 1
    
  4. Following the instructions you added to README.md, running cmake failed (again on MinGW; just running it on a Windows command shell was hopeless). The first call was good:
    $ /c/PROGRA~2/JetBrains/CLion\ 1.2.4/bin/cmake/bin/cmake.exe ..
    -- Building for: Visual Studio 14 2015
    --
    --  For compilation info please refer to the boxes compilation FAQ
    --  at http://boxes.thomasjensen.com/docs/faq.html#q5
    --
    -- Found BISON: C:/MinGW/msys/1.0/bin/bison.exe (found version "2.4.2")
    -- Found FLEX: C:/MinGW/msys/1.0/bin/flex.exe (found version "2.5.35")
    -- Configuring done
    -- Generating done
    -- Build files have been written to: D:/Documents/Projects/boxes/build
    
    But then no Makefile was generated. Instead, I see lots of Visual Studio files (which is sort of consistent with the message above):
    ALL_BUILD.vcxproj
    ALL_BUILD.vcxproj.filters
    CMakeCache.txt
    CMakeFiles/
    ZERO_CHECK.vcxproj
    ZERO_CHECK.vcxproj.filters
    boxes.sln
    boxes.vcxproj
    boxes.vcxproj.filters
    cmake_install.cmake
    doc/
    src/
    test/
    

@scyptnex
Copy link
Contributor Author

ok.
1/2 fixed

3: Hopefully fixed, though there are two issues here:

  • Because we now support both CMake and gnu-make, i've had to hard code several locations where the boxes executable might end up depending on the build, previously the location was also hard-coded.
  • When using CMake, the test command is make check, since the test target is reserved by CMake, there is no way around this unfortunately.

4: for CMake to generate MinGW build files (at least on my windows machine) you have to tell it with the -G "Mingw Makefiles" argument, ive also added this to the documentation.

@tsjensen
Copy link
Member

Thanks! Real life and the upcoming Christmas holidays are slowing me down at the moment ... but I'll take a look at it as soon as possible.

@tsjensen
Copy link
Member

Finally got around to boxes again. I hope you had peaceful and enjoyable holidays.

Most issues are resolved afaics, but running the cmake build still does not work for me. Following your instructions from the README, I get the error that sh.exe must not be on the path. I resolved this manually, after which the Makefiles get generated. When I run make from the build subdirectory, nothing happens except that a cmd.exe is launched:

$ make
Microsoft Windows [Version 10.0.14393]
(c) 2016 Microsoft Corporation. All rights reserved.

d:\path\to\boxes\build>

This currently prevents me from checking the rest, such as running the tests.

I am thinking, since this is the third review round already, that we may want to reduce complexity of this PR a little bit. Maybe you want to split off the fix to boxes.c into a separate PR? I could merge that one right away. Potentially, we could even add appveyor with a MinGW based GNU make build. Then we could give the cmake build more time to figure out reliable instructions and configuration. What do you think?

@scyptnex scyptnex closed this Jan 1, 2017
@tsjensen
Copy link
Member

tsjensen commented Jan 8, 2017

I see you have closed the PR. Are you still working on this topic, or did you give up? (I hope not, since we are so close to the goal!) Let me know if you need anything from me.

@tsjensen tsjensen mentioned this pull request Jan 31, 2017
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

Successfully merging this pull request may close these issues.

None yet

2 participants