Don't pass extra includes; configure this with flags

[thomasjm]

This fixes an issue that came up when working on Nixpkgs packaging for xeus-cling here.

It's weird for xeus-cling to stick extra arguments on to pass to the interpreter when it already passes the user-provided ones from the command line. The user is perfectly capable of adding this LLVM include directory if desired.

[JohanMabille]

This sounds like asking developers to add the include directories of the standard library on a regular project. I'm not sure that would be convenient for regular users. On the other hand, I totally understand the issue for packaging xeus-cling; I think a conditional compilation could help here, but maybe @SylvainCorlay has a better idea?

By the way, if you remove the extra arg, you can also remove the extra interpreter_args and pass argc and argv directly to the interpreter.

[thomasjm]

By the way, if you remove the extra arg, you can also remove the extra interpreter_args and pass argc and argv directly to the interpreter.

Not quite, as it's still changing the process name in the 0th argument.

This sounds like asking developers to add the include directories of the standard library on a regular project. I'm not sure that would be convenient for regular users.

Do regular users actually invoke xcpp manually? My impression was that xeus-cling is packaged as part of Anaconda somehow and that such flags are handled there. (I'd love to see how, if you happen to know where that happens.)

Also, which standard library do you mean? In my experience of packaging cling and xeus-cling a number of things are needed, such as the C standard library, the C++ standard library, the includes and library paths for Cling itself, etc.

It wasn't clear to me what the purpose of this extra argument was, as it doesn't come with a flag before it such as -I or -isystem.

[SylvainCorlay]

Do regular users actually invoke xcpp manually? My impression was that xeus-cling is packaged as part of Anaconda somehow and that such flags are handled there. (I'd love to see how, if you happen to know where that happens.)

Pardon my brevity as I am commenting from my phone.

I think it would be fine to remove this and pass the command line argument from the kernelspec. @thomasjm, this is specified in the kernel.json.in file in this repository.

[SylvainCorlay]

@thomasjm here is such a kernel spec file: https://github.com/jupyter-xeus/xeus-cling/blob/main/share/jupyter/kernels/xcpp14/kernel.json.in

[thomasjm]

Thanks! I just tried this in the latest commit. I haven't tested it yet, had a little trouble using conda on NixOS.

[SylvainCorlay]

I don't think we actually define LLVM_DIR as a cmake variable.

[thomasjm]

Okay, how about using LLVM_MAIN_INCLUDE_DIR? Based on my reading of CMakeLists.txt, this is the most proper variable to use:

xeus-cling/CMakeLists.txt

Line 98 in 471b1b6

set(LLVM_MAIN_INCLUDE_DIR ${INCLUDE_DIR} CACHE PATH "Path to llvm/include")

Incidentally, the variable called LLVM_BINARY_DIR, which ends up being passed to the preprocessor as -DLLVM_DIR, is rather confusingly named since it's actually the LLVM prefix, i.e. the result of calling llvm-config --prefix.

[thomasjm]

Okay, I was able to get a Conda environment running on a normal Ubuntu machine and test this out. With the latest commit it seems to work!