This is an archive of the discontinued LLVM Phabricator instance.

Don't try to build Burg if yacc isn't installed.
ClosedPublic

Authored by jlebar on Jan 12 2016, 6:56 PM.

Details

Summary

Without this patch, ninja test-suite dies with

CMake Error at MultiSource/Applications/Burg/CMakeLists.txt:17 (add_yacc_parser):
  Unknown CMake command "add_yacc_parser".

I have no idea what I'm doing here in CMake world, please be gentle.

Diff Detail

Repository
rL LLVM

Event Timeline

jlebar updated this revision to Diff 44702.Jan 12 2016, 6:56 PM
jlebar retitled this revision from to Don't try to build Burg if yacc isn't installed..
jlebar updated this object.
jlebar added reviewers: rengolin, cmatthews, jmolloy, beanz.
jlebar added a subscriber: llvm-commits.
MatzeB added a subscriber: MatzeB.Jan 12 2016, 7:38 PM

What if we just commit the files generated by yacc so we are independent of missing yacc and independent of possible performance changes between yacc versions and implementations?

  • Matthias

I would also expect that MultiSource/Benchmarks/MallocBench/gawk/CMakeLists.txt is affected by the same problem.

I would also expect that MultiSource/Benchmarks/MallocBench/gawk/CMakeLists.txt is affected by the same problem.

Looks like it, although from its parent's CMakeLists.txt, it appears that dir isn't built at all!

I'm looking into whether I can run yacc myself and check in the results, will let you all know.

jlebar updated this revision to Diff 44782.Jan 13 2016, 1:30 PM

Pregenerate Burg's yacc files.

Please have another look. I didn't touch gawk since it's currently not being built (and frankly, my interest is in making the test-suite build -- I'd prefer to leave the wholesale cleanup to someone else).

MatzeB accepted this revision.Jan 13 2016, 2:01 PM
MatzeB added a reviewer: MatzeB.

For some reason I had to manually remove the burg subdirectory from my builddir maybe cmake/clang got confused by earlier bison outputs. Anyway after that this patch worked nicely and I don't see any portability problems in the generated code. LGTM

This revision is now accepted and ready to land.Jan 13 2016, 2:01 PM
This revision was automatically updated to reflect the committed changes.

Thank you for the review.

Please have another look. I didn't touch gawk since it's currently not being built (and frankly, my interest is in making the test-suite build -- I'd prefer to leave the wholesale cleanup to someone else).

Most people are in the same situation (at least I am). Thanks for fixing burg.