bkryza / clang-uml

Customizable automatic UML diagram generator for C++ based on Clang.
Apache License 2.0
610 stars 44 forks source link

Nix package: add wrapper to fix stdlib includes #273

Closed pogobanane closed 5 months ago

pogobanane commented 5 months ago

Problem: Clang-uml doesnt find standard library includes on nixos.

Background: Since nixos is a bit of a special snowflake, compilers dont really work without some additional configuration. Compilers shipped by nixos are therefore wrapped by some small scripts. These wrappers make the compilation and linking steps of compilers compatible with the strict dependency management and versioning of nix. Many clang tools and libclang are not integrated in these wrappers yet, because they are not directly the compiler. This can lead to problems where clangtools/libclang can not find the c/cpp standard libraries.

Solution: Apply the same wrapper that nixos/nixpkgs uses for the clang-tools package. Using the clang-uml-wrapped exectuable, the stdlib import errors are resolved.

I think this wrapper is always necessary when using stdlibs from nixos, but perhaps @hatch01 can comment if the existing unwrapped clang-uml also works on some nixos installations. Anyways, in this PR i preserve both exectuables for compatibility.

hatch01 commented 5 months ago

This seems strange. I, personally, use NixOS and got no problem finding the stdlib. @bkryza tried on Ubuntu (said it here #258) and got no problem. Moreover, I think that patchelf should handle automatically what you are trying to do with your wrapper.

hatch01 commented 5 months ago

I retested it now with the last clang-UML version and got no problem. Moreover, I tried not to exclude the std namespace from my clang-UML config to ensure everything is working and got no problem with it. mermaid-diagram-2024-05-29-105721 screencopyt

pogobanane commented 5 months ago

Patchelf (specifially patching the rpath) only helps in finding the compiled .so llibraries. But in my case, clang-uml cant find the header files for standard libraries. On ubuntu systems they will likely still be found though, because they will not only be in the nix store, but also in standard locations that are automatically searched by clang (outside the nix store). I suspect this might also be the case for you @hatch01.

My system only has stdlib headers in the nix store, which is not automatically searched by clang without hints provided via environment variables. Perhaps you could try in a container/VM with a clean root filesystem. This would be mostly equivalent to writing a nix test to be run in a nix build sandbox.

hatch01 commented 5 months ago
docker run -it nixos/nix
nix-env --install git
nix-env --install coreutils
cd tmp
git clone https://github.com/bkryza/clang-uml.git
cd clang-uml/
nix --extra-experimental-features 'nix-command flakes' build
cd /tmp
git clone https://forge.univ-lyon1.fr/hatchcare/hatchcare.git
cd hatchcare/libs
nix --extra-experimental-features 'nix-command flakes' develop
mkdir build
cd build
cmake -DCMAKE_EXPORT_COMPILE_COMMANDS=ON ..
/tmp/clang-uml/result/bin/clang-uml --progress -g mermaid

got no problem with this sequence

bkryza commented 5 months ago

@pogobanane I've reproduced the steps described by @hatch01 above in clean NixOS VirtualBox VM (https://nixos.org/download/?ref=itsfoss.com#nixos-virtualbox) and it also works without a problem:

image

I've tried to read about this issue online and I realize that it is an issue in some cases, however it's not clear to me how to reproduce this problem. Also I've found some mentions that these wrappers cause some problems for others...

Can you describe in more detail how to reproduce the problem you were having in your setup?

bkryza commented 5 months ago

@pogobanane I have one more idea, can you try using these suggestions on master branch of clang-uml and see if it solves your issues?

I.e. try to add the following (if you're using CMake) to CMakeLists.txt:

set(CMAKE_CXX_STANDARD_INCLUDE_DIRECTORIES ${CMAKE_CXX_IMPLICIT_INCLUDE_DIRECTORIES})

or invoke clang-uml with option: --query-driver .

In general most issues related to using Clang based tools are related to include paths (be it a Linux, macos, Windows or Nix) so I've already built substantial amount of logic into clang-uml to deal with them:

https://clang-uml.github.io/md_docs_2common__options.html#resolving-include-path-and-compiler-flags-issues

Mic92 commented 5 months ago

@pogobanane I have one more idea, can you try using these suggestions on master branch of clang-uml and see if it solves your issues?

I.e. try to add the following (if you're using CMake) to CMakeLists.txt:

set(CMAKE_CXX_STANDARD_INCLUDE_DIRECTORIES ${CMAKE_CXX_IMPLICIT_INCLUDE_DIRECTORIES})

or invoke clang-uml with option: --query-driver .

In general most issues related to using Clang based tools are related to include paths (be it a Linux, macos, Windows or Nix) so I've already built substantial amount of logic into clang-uml to deal with them:

https://clang-uml.github.io/md_docs_2common__options.html#resolving-include-path-and-compiler-flags-issues

Will the query driver just parse locations for the standard library or will it also take include locations for other libraries into account? In Nix shells, we use NIX_CFLAGS_COMPILE in our devshells to provide a search path of all specified libraries:

$ nix-shell -p zlib --command 'echo $NIX_CFLAGS_COMPILE'
-frandom-seed=wf6is7s3v0 -frandom-seed=v2pq4kqbry -isystem /nix/store/qj9byzfvh7dd61kk0aglj7cwqj1xqg6l-zlib-1.3.1-dev/include -isystem /nix/store/qj9byzfvh7dd61kk0aglj7cwqj1xqg6l-zlib-1.3.1-dev/include

Also our CC wrapper should have the same include paths in its output if you call it with -v.

bkryza commented 5 months ago

@Mic92 --query-driver will extract only the system paths from the compiler specified in the compile_commands.json (if it's argument is . - otherwise you can specify any other compiler as provider of these paths) - but yes this will only include system libraries. The rest of the paths must come from compile_commands.json, where the paths must have been injected by your build system in order to compile the project in the first place.

Mic92 commented 5 months ago

This assumes that projects properly add includes for all and every header file. This is unfortunately not what people do in practice - Even if they wanted to, some libraries simply don't provide discovery mechanisms such as pkgconfig or cmake files, so instead most people rely on the global search path of their C compiler to find at least some header files. In Nix we deliberately don't have such global search path, because we want to enable a setup, where all dependencies have to be explicitly specified in a declarative way. The wrapper file above that @pogobanane posted fixes this by interpreting NIX_CFLAGS_COMPILE to build up a search path.

hatch01 commented 5 months ago

Okay, thank you very much for your explanations. Everything is much clearer now!

bkryza commented 5 months ago

@Mic92 Thanks for detailed explanations and patience :-)

What I don't still fully understand is that clang-uml (or any Clang based tool) requires a valid compile_commands.json, which is generated during build (or pre-build). If the project builds correctly it means that the paths were resolved properly. The only issue in my understanding can come from system headers (e.g. build used GCC implicit headers and clang-uml cannot find these paths in compile_commands.json), but this can be overcome using the steps described here.

Of course it would be better if these system paths were handled automatically, but my only concern here is whether this change can cause some issues for Nix users who currently can use clang-uml as is... @hatch01 can you comment on this?

FYI, I've reproduced again the steps posted by @hatch01 and here is an example entry from compile_commands.json, which seems to contain valid include paths (all start with /nix/store/) for all dependencies:

{
  "directory": "/tmp/hatchcare/libs/build",
  "command": "/nix/store/ac1hb5dc2z4biwgy8mjrhlifffkkrvdq-gcc-wrapper-13.2.0/bin/g++ -DhatchcareBack_EXPORTS -isystem /nix/store/r97382g1m15djmj60if45sjjscc716az-valgrind-3.22.0-dev/include -isystem /nix/store/s4hfymcv03adyxsp20m8f5s50ywqcw25-graphviz-10.0.1/include -isystem /nix/store/1n65s2f34xrx149ib4241pyspgy4lsmr-llvm-17.0.6-dev/include -isystem /nix/store/2nhjsfc4pg74vqnsbjmnpi7359y6f3wi-ncurses-6.4-dev/include -isystem /nix/store/znghzib6sz0pnpq5b6k2rl3r7wdcjvjw-zlib-1.3.1-dev/include -isystem /nix/store/56c86pykrx61i3cn85gxfj19g0cwcqfq-openjdk-17.0.7+7/include -isystem /nix/store/9s298pi4pw2njvp8jhx3wqmcf8rkchh1-gtk+3-3.24.41-dev/include -isystem /nix/store/qwyb3rpa682bjhmbvqwl776733sg4n2y-at-spi2-core-2.50.2-dev/include -isystem /nix/store/40p483s8gvk9z6rvii9957b732wndzkd-dbus-1.14.10-dev/include -isystem /nix/store/yl786b3zw96sa5fwr7lkd0lyjhvn0zqn-expat-2.6.2-dev/include -isystem /nix/store/z1xzvmw2bzzk3x403zab39lq0r2sxbkw-glib-2.78.4-dev/include -isystem /nix/store/g3m9mvzid3x524il0c3g44l23848ar5j-libffi-3.4.6-dev/include -isystem /nix/store/qjb0xvlkzdbrxxasmajw93phx3wh8vq6-gettext-0.21.1/include -isystem /nix/store/4ldmrq8by8sndh445x75507564gz7xm9-glibc-iconv-2.39/include -isystem /nix/store/y66wf2widknj05vg13lcmipdsl9xzlk0-cairo-1.18.0-dev/include -isystem /nix/store/cjsvyimzvhzrmkz7v05wj5jzg1f4gq2i-fontconfig-2.15.0-dev/include -isystem /nix/store/8kwwg4zrlvj3kyxpf0p55b05a3h0lr09-freetype-2.13.2-dev/include -isystem /nix/store/i2kf619g02j5p3cd4yh93ilr2d24i4m8-bzip2-1.0.8-dev/include -isystem /nix/store/fbkrax280ivbrr85q8a96fygr6abi610-brotli-1.1.0-dev/include -isystem /nix/store/ic5wzcpy0mbj7qpxg05j8zdihh3yc986-libpng-apng-1.6.43-dev/include -isystem /nix/store/73lsxaii5sk9zgm7g53mca4a65a9i5xh-pixman-0.43.4/include -isystem /nix/store/d9w0q9a4mavflx0xl1820wfs9q6hxld3-libXext-1.3.6-dev/include -isystem /nix/store/6pclsikg99ddpz9sw4fys897n637wq7l-xorgproto-2023.2/include -isystem /nix/store/n73hxr1f0vdn2p1l5x5d95hi144fad9c-libXau-1.0.11-dev/include -isystem /nix/store/b5xg48id93xsnx3ssrv2fwacakhwdsln-libXrender-0.9.11-dev/include -isystem /nix/store/1h1337d98nwp7776wk8nyvkq9cdh5gah-libX11-1.8.7-dev/include -isystem /nix/store/7rx4yhjj886kvhzwrg9qggvjxax5q12y-libxcb-1.16-dev/include -isystem /nix/store/q1z8qhf4gazrf4a1mdhczyn9kj3rccx1-fribidi-1.0.13-dev/include -isystem /nix/store/fbz0bx2sixjn963jqkjjphdcrm3ig268-gdk-pixbuf-2.42.10-dev/include -isystem /nix/store/0rj33s3j57rzb6nyyji42gfk8p55lvy9-libtiff-4.6.0-dev/include -isystem /nix/store/wj1nw4637h13mxsm5k6l5qrh52vq82d9-libdeflate-1.19/include -isystem /nix/store/31i0j43d8xcqriz5915vxsm26qawygzb-libjpeg-turbo-3.0.2-dev/include -isystem /nix/store/9zghvnkj49dnz9bdfa81b3h4kyarvzrp-xz-5.4.6-dev/include -isystem /nix/store/glky0wjlb1jc81awd8ddajr4fay1w27w-gsettings-desktop-schemas-45.0/include -isystem /nix/store/hqa36pjcdfjm88mwrvzcka4kc906acf6-libICE-1.1.1-dev/include -isystem /nix/store/axz4ckv434pd37ai8l6nww6x9dkwlchv-libSM-1.2.4-dev/include -isystem /nix/store/iph8d6z6sa5987mrq89s7ln7g3lc0qd8-libXcomposite-0.4.6-dev/include -isystem /nix/store/fr31iazmb97j4lr77sbi3rb5hsbswzzq-libXfixes-6.0.1-dev/include -isystem /nix/store/7kd5y1jpsipj0cqvl54phi0cvkqkjr9i-libXcursor-1.2.2-dev/include -isystem /nix/store/mrzi5dhf76qz0m4kdrcsa86qpr4ipj1v-libXdamage-1.1.6-dev/include -isystem /nix/store/rg4gpdyzvr7v9psvjrq9rd7v34bf042c-libXi-1.8.1-dev/include -isystem /nix/store/ccf58yf4hga49gjvbqbkkhwyn2f10abz-libXrandr-1.5.4-dev/include -isystem /nix/store/m1kx60ln0zyg4nywl8yrpl6zy0g10srl-pango-1.51.2-dev/include -isystem /nix/store/86flfz614gqln2znq398351jkbx93cd4-harfbuzz-8.3.0-dev/include -isystem /nix/store/prpdq8v8hvzwy93d9i20nkaq1qasg547-graphite2-1.3.14-dev/include -isystem /nix/store/0q1p4k8w4291z32n0m2ydq44d0girv4i-libXft-2.3.8-dev/include -isystem /nix/store/n6y0apdsfnj8xwb6wv752slh5bng14s1-libGL-1.7.0-dev/include -isystem /nix/store/80w55fw51p8qf0bx9ly6sil4n25fppwq-wayland-1.22.0-dev/include -isystem /nix/store/9laip3m5k8291q51lcxyiiivi2f9pi7n-libXinerama-1.1.5-dev/include -isystem /nix/store/bbni8mhs4qm9v5ijfbv6nfas8vg567hx-cups-2.4.7-dev/include -isystem /nix/store/vyfa79pccinnfgz3ggrlp1m4hjpvrvz5-gmp-with-cxx-6.3.0-dev/include -isystem /nix/store/7mchd4hl6qsfr3164awcrqzcjn25ca49-python3-3.11.8-env/include -isystem /nix/store/jrqfmy2g5cvvhn5bdqm8bz59mq4f4z9h-pcre2-10.43-dev/include -isystem /nix/store/qf0l3565v2zfpc8dy3y4zzb78x264j1r-util-linux-2.39.3-dev/include -isystem /nix/store/sfj68lj4wrd155b2incnha13d4wnv82z-libselinux-3.6-dev/include -isystem /nix/store/a4q541q1dax742wd2kxyz3xzg5y9anpv-libsepol-3.6-dev/include -isystem /nix/store/kr6g6jysg8hy5xjygagaia73zy91d9x7-libthai-0.1.29-dev/include -isystem /nix/store/b43jvji4zgd2g25fyvyn2csw2zv8jq31-libdatrie-2019-12-20-dev/include -isystem /nix/store/4937nh115knvqfzqhzgribc37b3r06gh-libXdmcp-1.1.4-dev/include -isystem /nix/store/pvj554bgy94y9j29b5xl8l4sw239vqkx-libxkbcommon-1.5.0-dev/include -isystem /nix/store/p7bgvpbymbp8m1lrqlvx4jfhg81h3piw-libepoxy-1.5.10-dev/include -isystem /nix/store/jkk691f3dhxsq6j0ap0djyilsa8mm6jq-libXtst-1.2.4/include -isystem /nix/store/680fjvdzv4i81a40yzwga0z64jw1yczb-compiler-rt-libc-17.0.6-dev/include -isystem /nix/store/a3mmr5jmrd0sjvmnc9vqvs388ppq1bnf-gcc-13.2.0/include/c++/13.2.0 -isystem /nix/store/a3mmr5jmrd0sjvmnc9vqvs388ppq1bnf-gcc-13.2.0/include/c++/13.2.0/x86_64-unknown-linux-gnu -isystem /nix/store/a3mmr5jmrd0sjvmnc9vqvs388ppq1bnf-gcc-13.2.0/include/c++/13.2.0/backward -isystem /nix/store/a3mmr5jmrd0sjvmnc9vqvs388ppq1bnf-gcc-13.2.0/lib/gcc/x86_64-unknown-linux-gnu/13.2.0/include -isystem /nix/store/a3mmr5jmrd0sjvmnc9vqvs388ppq1bnf-gcc-13.2.0/include -isystem /nix/store/a3mmr5jmrd0sjvmnc9vqvs388ppq1bnf-gcc-13.2.0/lib/gcc/x86_64-unknown-linux-gnu/13.2.0/include-fixed -isystem /nix/store/gzxqm8dyfirbysqjhh78ivam62ll0m87-glibc-2.39-5-dev/include -std=gnu++23 -fPIC -o CMakeFiles/hatchcareBack.dir/src/question/Question.cpp.o -c /tmp/hatchcare/libs/src/question/Question.cpp",
  "file": "/tmp/hatchcare/libs/src/question/Question.cpp",
  "output": "CMakeFiles/hatchcareBack.dir/src/question/Question.cpp.o"
}

Anyway, I've reproduced the steps also with the https://github.com/pogobanane/clang-uml/tree/master branch and they also work without an issue, both when using: /tmp/clang-uml/result/bin/clang-uml or /tmp/clang-uml/result/bin/clang-uml-wrapped, so if this change doesn't brake existing scenarios and possibly improves dealing with include paths I think we can merge it.

@hatch01 Can you please comment whether this change in your opinion can cause any issues? As I understand with this change the users will have the option to use either clang-uml or clang-uml-wrapper, and if they use clang-uml binary then there will be no change for them?

hatch01 commented 5 months ago

First of all, I am far from being a Nix expert. I don't see any potential issues that would be caused by this. Moreover, @Mic92 is a significant Nix contributor and has much more knowledge about this than I do, so I tend to trust his opinion. I also tested building the PR and using it on Hatchcare, and I encountered no problems. So, for me, it is okay to merge. Maybe just adding a short note in the documentation would be great, but that's it.

pogobanane commented 5 months ago

Your project indeed works just fine. This is one of my projects that need the clang-uml-wrapper:

git clone https://github.com/vmuxIO/vmuxIO.git
cd vmuxIO
git checkout dev/clang-uml
nix develop
meson build -De1000_emu=false
ninja -C build

# doesnt work:
nix shell github:pogobanane/clang-uml --command clang-uml -g mermaid -c ./design/clang-uml.yml
# --query-driver . works:
nix shell github:pogobanane/clang-uml --command clang-uml -g mermaid -c ./design/clang-uml.yml --query-driver .
# wrapper works without query-driver
nix shell github:pogobanane/clang-uml --command clang-uml-wrapped -g mermaid -c ./design/clang-uml.yml

--query-driver . indeed works for my project as well. I cant think of any case where --query-driver . would not resolve to the same as clang-uml-wrapper (it even accounts for target architecture).

Anyways, in this PR i preserves both exectuables for backwards-compatibility.

If we decide the wrapper is a safe replacement so that nix users can avoid additional --query-driver args, we can also make the wrapper the standard (clang-uml) and rename the binary to clang-uml-unwrapped.

bkryza commented 5 months ago

@pogobanane Thanks for the PR! @hatch01 @Mic92 Thanks for helping with the review!

bkryza commented 5 months ago

@pogobanane @hatch01 @Mic92 I've added a short section in documentation describing the wrapper https://github.com/bkryza/clang-uml/blob/master/docs/common_options.md#nix-wrapper

Thanks again!