googleprojectzero/Jackalope

Update README instructions for building on macOS

stackotter opened this issue · 13 comments

The build commands in the README required some tweaking for me to build the project on macOS. It might just be my machine, but if this is a recurring issue here are the commands that worked for me (with the latest Xcode 14.0 beta installed). It might be worthwhile adding this to the README (I'm happy to PR if other mac users can reproduce the same issues).

# ... (the steps proceeding configuring and building with cmake didn't need tweaking)

cmake -D CMAKE_C_COMPILER="$(xcrun -find cc)" -D CMAKE_CXX_COMPILER="$(xcrun -find c++)" -D CMAKE_CXX_FLAGS="${CMAKE_CXX_FLAGS} -std=c++17 -stdlib=libc++ -std=c++1z" -G Xcode ..
cmake --build . --config Release

Issues this fixed

Without CMAKE_C_COMPILER and CMAKE_CXX_COMPILER set to those values, I get the error briefly mentioned in the README. Without the CMAKE_CXX_FLAGS that set the standard to c++17 I run into a lot of syntax errors (it seems to compile with something from before c++11 by default). It should be noted that the CMAKE_CXX_STANDARD option didn't seem to change anything at all so I had to use CMAKE_CXX_FLAGS instead.

I hardly ever touch cpp, so there might be something weird about those commands, but at least they work. Feel free to suggest changes.

Hi,

Interesting, can you paste the error(s) you get when compiling as instructed in the README?

A couple of questions:

  • did you use the -G Xcode flag as written in the README?
  • did you ever open XCode UI before trying to compile Jackalope? Note: XCode installs some tools on the first run / first run after update.
  • do you have multiple compiler toolchains installed?

Answers

  1. With the instructions from the readme I get the following errors:
    cmake -G Xcode ..
    -- The C compiler identification is unknown
    -- The CXX compiler identification is unknown
    CMake Error at TinyInst/third_party/CMakeLists.txt:30 (project):
      No CMAKE_C_COMPILER could be found.
    
    CMake Error at TinyInst/third_party/CMakeLists.txt:30 (project):
      No CMAKE_CXX_COMPILER could be found.
    
    -- Configuring incomplete, errors occurred!
    See also "/Users/stackotter/*/Jackalope/build/CMakeFiles/CMakeOutput.log".
    See also "/Users/stackotter/*/Jackalope/build/CMakeFiles/CMakeError.log".
  2. Yes
  3. No

More about the answer to 1

I tried both suggestions from the readme and neither worked. The following two options solve those two particular errors: -D CMAKE_C_COMPILER="$(xcrun -find cc)" -D CMAKE_CXX_COMPILER="$(xcrun -find c++)".

However, if I only use those options (along with -G Xcode) I get lots of compilation errors later on when running cmake --build . --config Release. See the attached build log.

buildlog.txt

Some more context

Xcode version: Xcode 14.0 beta 1
CMake version: 3.23.2 (the latest version from homebrew)

Can you run sudo xcode-select --reset and check if the problem persists?

Yep I tried that before creating the issue and it didn’t seem to do anything. Exact same situation.

Hmm well, the issue seems to be that cmake can't identify the C/C++ compiler, which seems unrelated to Jackalope itself, but I have no idea what the reason could be. This stack overflow thread has some other suggestions https://stackoverflow.com/questions/33998853/the-cxx-compiler-identification-is-unknown-xcode

Yeah I found that thread, I tried all of the suggestions.

I think it is something about the Jackalope cmake configuration that my cmake installation doesn't like because other cmake projects work fine. Adding those flags I presented in the original issue completely solves the issue for me, so my suggestion is just that they could be added to the readme as a last resort that people can try when building on macOS (unless someone can find out the root cause of course, but given that it's only affecting me at the moment that would probably be unnecessary).

Or if zero other macOS users with Xcode 14 and macOS Monterey are getting this issue, maybe this issue can just be closed. My main motivation for opening this issue was just to help other people not spend so long if they run into the same weird situation.

Indeed, and thanks for raising the issue. Let's leave it open for now, as anyone encountering the same issue can find it more easily.

If anyone else encounters the same issue, please let us know here.

Ok sounds good, thanks for the quick responses!

Update: I encountered this issue after upgrading to Ventura. It got resolved after updating cmake.

You might also want to add a cd TinyInst in the below section of the guide, otherwise the commands will fail

Run the following commands:

git clone --recurse-submodules git@github.com:googleprojectzero/TinyInst.git
(alternately: git clone --recurse-submodules https://github.com/googleprojectzero/TinyInst.git)
mkdir build
cd build
cmake <generator arguments> ..
cmake --build . --config Release

Hmm wait, that was not the intended way to build. How does your directory structure looks like? The idea was to run all of these commands starting from the Jackalope directory, which would then place TinyInst as a subdirectory of Jackalope. TinyInst should then build without the need to explicitly cd into it because of https://github.com/googleprojectzero/Jackalope/blob/main/CMakeLists.txt#L31

Perhaps instead, we should add cd Jackelope on top to make it more clear?

Hmm wait, that was not the intended way to build. How does your directory structure looks like? The idea was to run all of these commands starting from the Jackalope directory, which would then place TinyInst as a subdirectory of Jackalope. TinyInst should then build without the need to explicitly cd into it because of https://github.com/googleprojectzero/Jackalope/blob/main/CMakeLists.txt#L31

Perhaps instead, we should add cd Jackelope on top to make it more clear?

Yes, as I discovered later I was supposed to clone TinyInst inside Jackelope (I was tired and the missing cd confused me)

Is there a possibility of adding TinyInst as a module to the repo so just one clone is needed?

Also I agree on adding the cd, I'd also add the git clone of Jackelope as well and remove the precedent step

Is there a possibility of adding TinyInst as a module to the repo so just one clone is needed?

The problem with submodules is that (AFAIK) a submodule ends up linking to a specific commit. I always want to use the latest TinyInst and I don't want to have to update Jackalope every time I update TinyInst.