dajobe / raptor

Redland Raptor RDF syntax library
https://librdf.org/raptor/
Other
156 stars 62 forks source link

CMake build system upgrade to allow building from git sources #29

Closed 0u812 closed 9 years ago

0u812 commented 9 years ago

Hi, I was trying the CMake build system the other day and ran into some trouble. I decided to bring the CMake build system up-to-date on Linux first before tackling the Windows build issues. The changes in this PR are mainly to allow it to generate the lexer and parser for turtle etc. at build time (previously it ostensibly required a release). With these changes, it does compile from the git sources and I can run rapper from the CMake installation.

Are there any specific tests I should run? I tried rapper on some basic examples like tests/rdfxml/ex-01.rdf and it gave identical output to the system-installed version. These changes only affect the CMake build system.

dajobe commented 9 years ago

Hey Kyle. You might have noticed I made some CMake changes recently to raptor, because I was playing with building it on windows (which I don't use). I was using appveyor which I think makes the results public at https://ci.appveyor.com/project/dajobe/raptor

So I'm reasonably sure it works on windows... in a certain way

As for tests, I don't know the cmake world of tests.

0u812 commented 9 years ago

Okay, I think I have all the paths absolute now. Just tested it and it still builds (I'm doing an out-of-source build btw).

dajobe commented 9 years ago

I tried this with an in-tree and out-of-tree build on OSX with cmake and it works with a couple of caveats.

  1. It failed to check that bison was V3 or newer, so it failed to work until I put a bison V3 binary in the PATH.
  2. make test ran and gave a bunch of rdfa and rdaf11 errors, which are "expected failures" and ok but the current cmake test run macros can't represent that
0u812 commented 9 years ago

Thanks for letting me know about the rdfa expected failures. I modified the macro (now a function) that tests each case so you can specify the extra arg EXPECTED_FAILURE, which I did for all the rdfa/rdfa11 tests which were currently failing (4 in rdfa and 26 in rdfa11). Other than rdfa/rdfa11, it's showing 100% passing over 553 tests (on Linux).

For Bison, I specified a minimum required version so it should complain immediately if it finds one which is too old (just tested this on Windows and confirmed it works). Should I try the appveyor to make sure it still works?

0u812 commented 9 years ago

Btw ran into the following problem: Win flex-bison project has Windows binaries of Bison that go up to 3.0, but the version checking fails (within Bison) if 3.0.0 is specified in the *.y files for some reason.

dajobe commented 9 years ago

@0u812 not sure what you are saying here. What check "within Bison" fails?

0u812 commented 9 years ago

The turtle_parser.y file has %require "3.0.0". Win flex-bison (as of the 2.5.1 release) reports its version as 3.0 and the version check fails (it must be set to %require "3.0 in order to pass). On Windows, I'm currently building this w/o Cygwin, and those were the only pre-built binaries I could find for Bison 3. Maybe it would be better to postpone that change since it's a domain-specific workaround. Please let me know what your preference is regarding whether it should be part of this PR or not.

dajobe commented 9 years ago

I checked the bison sources and it uses strverscmp() so %require "3.0" is sufficient and portable. You can include that change in this PR.

0u812 commented 9 years ago

Whew! Sorry it's been so long. I've been traveling a lot recently and this sort of fell by the wayside. As you can see from the commit above, I updated the appveyor.yml to let CMake generate the lexing/parsing tables and the build is passing:

https://ci.appveyor.com/project/0u812/raptor

One thing I noticed was that Appveyor's version of Cygwin's setup-x86.exe was out-of-date, and it was literally breaking the build. Therefore, I had to modify the appveyor.yml to download a new copy as you can see from the Invoke-WebRequest $env:CYG_SETUP_URL -OutFile $env:CYG_ROOT\setup-x86.exe line in 1a710997765a0b6294b0d7882cbb8c144145aa33.

dajobe commented 9 years ago

Looks good to me. github says "Merge with caution!" but I think it'll be ok