Follow symbolic links for filename comparison

[smithsg84]

In a src tree with several symbolic links in paths the filenames did not always match. At least with the waf build tool the real path name is appearing in the compile_commands.json file. The compiler option parsing was failing to remove the src file. Used file-truefilename to follow symbolic links for name comparison.

[Sarcasm]

I'm pretty sure I used to do a lot (expansive?) file-truename in the past, but it did not handle the case where the symbolic name was important.
Now it looks like it's the other way around.

We could fix it, but I think the libclang backend already solve this by using something like std::filesystem::equivalent.
Any reason why you aren't using the libclang backend?

[smithsg84]

First time user so could be I'm just confused.

What I was seeing was the source filename in the get-compile-options query was not being stripped off after the query. irony-cdb-libclang--adjust-options-and-remove-compiler is calling into the irony-cdb-json--adjust-compile-options to parse the response so I think both backends are using the same code to parse the compiler options?

[Sarcasm]

Oh I see, it's me that was no longer familiar with the code, sorry about that.

What issue did you notice with the filename being here?
I thought clang stripped it itself, and cleaning up the command line was nice but not mandatory.

Sorry for being wary about this change, but it adds a syscall (or way more, looking at the implementation) for each argument of the command, so I'm a bit reluctant. file-equal-p isn't really any better as it calls file-truename too.
Ideally (file-truename file) would be called only once and not in the loop.

[smithsg84]

No problem with being leery of adding overheads. Maybe problem lies more with how the compile_commands.json filenames I'm getting? The project is using Waf for configure/build which is not all that common. Comparing with a cmake generated file I notice the filenames are absolute in cmake and relative from Waf. The Waf build directory is the true path, the src filenames are relative.

Is it expected that paths be absolute in the compiler commands? First time I've used this file.

Waf generated

{
"directory": "/usr/WS2/smithsg/projects/scaling/clean/scaling-bake/source/scaling-simulator/build",
"command": "/usr/tce/packages/gcc/gcc-8.1.0/bin/g++ -O0 -ggdb -g3 -Wall ../src/core/model/des-metrics.cc -c -o/usr/WS2/smithsg/projects/scaling/clean/scaling-bake/source/scaling-simulator/build/src/core/model/des-metrics.cc.2.o",
"file": "../src/core/model/des-metrics.cc"
},

Cmake generated

{
"directory": "/g/g10/smithsg/tmp/cmake-example/build/src",
"command": "/usr/tce/packages/gcc/gcc-8.1.0/bin/g++ -Wall -O3 -o CMakeFiles/mylib_static.dir/mylib.cpp.o -c /g/g10/smithsg/tmp/cmake-example/src/mylib.cpp",
"file": "/g/g10/smithsg/tmp/cmake-example/src/mylib.cpp"
},

[Sarcasm]

It's not required for the paths to be absolute, but I'm pretty sure I encountered an issue a while back in a clang tools, might have been fixed since then.

What I'm wondering is what error did you notice?
Is company completion broken or flycheck broken?

[smithsg84]

Thanks for the help.

The real core of my issue, as far as I can debug, originates from the compile_commands.json having different paths in it (real path) vs editing in the context of a soft link path.

The error is:

Irony I/O task: error in callback: (irony-server-error complete-error "failed to perform code completion" "/g/g10/smithsg/projects/scaling/clean/scaling-bake/source/scaling-simulator/src/point-to-point/model/point-to-point-net-device.cc" 198 3)

Looking at the log file, the src file is being included in the complete instead of being stripped off.

execute: Command{action=Command::GetCompileOptions, file='/g/g10/smithsg/projects/scaling/clean/scaling-bake/source/scaling-simulator/src/point-to-point/model/point-to-point-net-device.cc', unsavedFile='', dir='/g/g10/smithsg/projects/scaling/clean/scaling-bake/source/scaling-simulator/build/', line=0, column=0, prefix='', caseStyle='exact', flags=[], opt=off}
execute: Command{action=Command::SetUnsaved, file='/g/g10/smithsg/projects/scaling/clean/scaling-bake/source/scaling-simulator/src/point-to-point/model/point-to-point-net-device.cc', unsavedFile='/var/tmp/smithsg/irony-unsaved-9Qb0x3', dir='', line=0, column=0, prefix='', caseStyle='exact', flags=[], opt=off}
execute: Command{action=Command::Complete, file='/g/g10/smithsg/projects/scaling/clean/scaling-bake/source/scaling-simulator/src/point-to-point/model/point-to-point-net-device.cc', unsavedFile='', dir='', line=198, column=3, prefix='', caseStyle='exact', flags=['-x', 'c++', '-working-directory', '/usr/WS2/smithsg/projects/scaling/clean/scaling-bake/source/scaling-simulator/build', '-O0', '-ggdb', '-g3', '-Wall', '-std=c++11', '-Wno-parentheses', '-Wno-error=deprecated-declarations', '-fstrict-aliasing', '-Wstrict-aliasing', '-fPIC', '-pthread', '-I.', '-I..', '-DNS3_BUILD_PROFILE_DEBUG', '-DNS3_ASSERT_ENABLE', '-DNS3_LOG_ENABLE', '-DHAVE_SYS_IOCTL_H=1', '-DHAVE_IF_NETS_H=1', '-DHAVE_NET_ETHERNET_H=1', '-DHAVE_PACKET_H=1', '-DHAVE_SQLITE3=1', '-DHAVE_IF_TUN_H=1', '-DHAVE_GSL=1', '-DHAVE_CRYPTO=1', '../src/point-to-point/model/point-to-point-net-device.cc'], opt=off}

Manually executing the get-compiler-options through the irony server, the returned data has the true paths in it.

~/.emacs.d/irony/bin/irony-server get-compile-options /g/g10/smithsg/projects/scaling/clean/scaling-bake/source/scaling-simulator/build/ /g/g10/smithsg/projects/scaling/clean/scaling-bake/source/scaling-simulator/src/point-to-point/model/point-to-point-net-device.cc

(("/usr/tce/packages/gcc/gcc-8.1.0/bin/g++" "-O0" "-ggdb" "-g3" "-Wall" "-std=c++11" "-Wno-parentheses" "-Wno-error=deprecated-declarations" "-fstrict-aliasing" "-Wstrict-aliasing" "-fPIC" "-pthread" "-I." "-I.." "-DNS3_BUILD_PROFILE_DEBUG" "-DNS3_ASSERT_ENABLE" "-DNS3_LOG_ENABLE" "-DHAVE_SYS_IOCTL_H=1" "-DHAVE_IF_NETS_H=1" "-DHAVE_NET_ETHERNET_H=1" "-DHAVE_PACKET_H=1" "-DHAVE_SQLITE3=1" "-DHAVE_IF_TUN_H=1" "-DHAVE_GSL=1" "-DHAVE_CRYPTO=1" "../src/point-to-point/model/point-to-point-net-device.cc" "-c" "-o/usr/WS2/smithsg/projects/scaling/clean/scaling-bake/source/scaling-simulator/build/src/point-to-point/model/point-to-point-net-device.cc.1.o" ) . "/usr/WS2/smithsg/projects/scaling/clean/scaling-bake/source/scaling-simulator/build")
))

[Sarcasm]

Ok thank you, so I guess libclang does not filter-out the sources itself, even though we specify the source file to process, and also, the file in the command line should be compatible...

Just to limit the impact of the change, I think we can do 2 things:

1. compute (file-truename file) only once, at the beginning, no need to do it for every arguments of the command line
2. skip the comparison if the current argument is an option (string-prefix-p "-" opt)

[smithsg84]

I pushed up a change. Wasn't super sure best place to put the "-" skipping, inline with the file check or adding an additional skip cond before the file check. Intent seemed a less order dependent and clear to me adding an AND to file cond.

[Sarcasm]

Great! Thank you for your patience.
Regarding the cond vs and+not, I was wondering the same when reading the code, I think you approach is good. 👍