skade / leveldb-sys

MIT License
7 stars 15 forks source link

Fix CMake build for paths with single quotes and spaces #22

Open michaelsproul opened 3 years ago

michaelsproul commented 3 years ago

Currently, building leveldb-sys at a path with spaces in it fails with an error about the C compiler being unable to compile a simple program. I isolated the cause as the directory arguments to -I and -L being passed unquoted. Quoting them allows builds to succeed for project paths with spaces in them.

The issue only occurs when snappy is enabled, so to reproduce you need to:

  1. Checkout leveldb-sys to a path name with spaces, e.g. ~/space dir/leveldb-sys
  2. Compile with cargo build --features snappy
michaelsproul commented 3 years ago

An example of the error produced:

CMake Error at /usr/share/cmake-3.16/Modules/CMakeTestCCompiler.cmake:60 (message):
    The C compiler

      "/usr/bin/cc"

    is not able to compile a simple test program.

    It fails with the following output:

      Change Dir: /home/michael/Programming/space test/leveldb-sys/target/debug/build/leveldb-sys-28c0b75fbf8d24a6/out/build/CMakeFiles/CMakeTmp

      Run Build Command(s):/usr/bin/make cmTC_5b5eb/fast && /usr/bin/make -f CMakeFiles/cmTC_5b5eb.dir/build.make CMakeFiles/cmTC_5b5eb.dir/build
      make[1]: Entering directory '/home/michael/Programming/space test/leveldb-sys/target/debug/build/leveldb-sys-28c0b75fbf8d24a6/out/build/CMakeFiles/CMakeTmp'
      Building C object CMakeFiles/cmTC_5b5eb.dir/testCCompiler.c.o
      /usr/bin/cc   -I'/home/michael/Programming/space test/leveldb-sys/target/debug/build/leveldb-sys-28c0b75fbf8d24a6/out/include' -ffunction-sections -fdata-sections -fPIC -m64    -o CMakeFiles/cmTC_5b5eb.dir/testCCompiler.c.o   -c "/home/michael/Programming/space test/leveldb-sys/target/debug/build/leveldb-sys-28c0b75fbf8d24a6/out/build/CMakeFiles/CMakeTmp/testCCompiler.c"
      Linking C executable cmTC_5b5eb
      /usr/bin/cmake -E cmake_link_script CMakeFiles/cmTC_5b5eb.dir/link.txt --verbose=1
      /usr/bin/cc  -I'/home/michael/Programming/space test/leveldb-sys/target/debug/build/leveldb-sys-28c0b75fbf8d24a6/out/include' -ffunction-sections -fdata-sections -fPIC -m64   -L/home/michael/Programming/space test/leveldb-sys/target/debug/build/leveldb-sys-28c0b75fbf8d24a6/out/lib  CMakeFiles/cmTC_5b5eb.dir/testCCompiler.c.o  -o cmTC_5b5eb 
      cc: error: test/leveldb-sys/target/debug/build/leveldb-sys-28c0b75fbf8d24a6/out/lib: No such file or directory
      make[1]: *** [CMakeFiles/cmTC_5b5eb.dir/build.make:87: cmTC_5b5eb] Error 1
      make[1]: Leaving directory '/home/michael/Programming/space test/leveldb-sys/target/debug/build/leveldb-sys-28c0b75fbf8d24a6/out/build/CMakeFiles/CMakeTmp'
      make: *** [Makefile:121: cmTC_5b5eb/fast] Error 2
michaelsproul commented 3 years ago

I've realised this might need some more tweaking for paths that contain quotes. I'll check the behaviour and update accordingly

skade commented 3 years ago

@michaelsproul thank you, would be happy to accept this patch!

michaelsproul commented 3 years ago

Awesome! As I suspected my original solution was broken for paths containing ', so I've pushed what I hope will be a more robust solution using the shell_escape crate. I tested it for paths with single quotes, and spaces and it worked well, but I've just found that the build fails at a different point for paths with ".

I've had enough of build system wrangling for one day, but I can try fixing it for " sometime next week.

skade commented 3 years ago

Thanks! The additional shell_escape dependency looks okay in this case!

michaelsproul commented 6 months ago

@skade Sorry to bump an ancient PR. I've finally gotten around to improving handling of paths with double quotes. I gave up on getting it to actually work – I suspect the upstream CMake build system can't handle it.

Instead I've made the build script produce an explicit error highlighting what the problem is (a path containing double quotes). For the simple cases of spaces & single quotes, compilation now works!

It would be awesome if you could merge this PR and cut a new release. I'm also happy to volunteer as a maintainer if you need more hands. You can check my record on open source, including contributions to rust-lang/rust and many years of work on sigp/lighthouse (my day job).