Load cfg files relative to executable before considering FILESDIR.

[firewave]

This is a very old change and after reviewing the code for the loading of non-cfg files it turns out that the existing behavior is actually a bug. All the other code was already preferring the application dir to the FILESDIR.

Original commit message:

Load cfg files relative to executable before considering
  FILESDIR, such that the testrunner uses the local files instead of those
  from an installed cppcheck.

[firewave]

This needs some unit tests to make sure this actually works as expected for all cases.

[danmar]

In a installed Cppcheck .. it sounds best to look in the FILESDIR first. I am not sure if it should even look in the executable path if the FILESDIR is set.
i.e. we should look in the /var/.. not in /usr/bin/..

[ledocc]

Old topic but not yet fixed.

Curious to add an absolute path in the code, event if it is configurable on build time.
But it’s done, so hard to remove it without break anything

But here what we can add to current behaviour :

• allow package builder to set FILESDIR on a relative path to cppcheck executable, this make cppcheck package relocatable
• allow final user to define a CPPCHECK_FILESDIR environment variable to replace FILESDIR define at build time

Thought ?

[danmar]

But here what we can add to current behaviour :

I feel I can accept both your suggestions.. if you feel that there is a real use case..

[firewave]

We need a single proper implementation of the lookup and tests for that. We also need to document the lookup order. I will also try to get feedback on some known maintainers on how they use it and how they expect it to work.

• allow package builder to set FILESDIR on a relative path to cppcheck executable, this make cppcheck package relocatable

Relative paths are bad idea.

• allow final user to define a CPPCHECK_FILESDIR environment variable to replace FILESDIR define at build time

That makes sense.

[danmar]

Relative paths are bad idea.

well this is how Cppcheck works in Windows. And the cppcheck premium also uses a relative path.

[firewave]

I filed https://trac.cppcheck.net/ticket/12240 about this. The user configuration should always override the system one otherwise a local binary is not usable at all.

In a installed Cppcheck .. it sounds best to look in the FILESDIR first. I am not sure if it should even look in the executable path if the FILESDIR is set.

FILESDIR is a rather awkward approach to be honest but I understand the reasoning behind it.

I also agree that looking in the /usr/bin folder sounds suspicious but we should have consistent behavior first and tune it later on.

We should probably also add user folder lookup and a command-line option. So the order would look like this

• command-line option (development case)
• binary folder (development case)
• user folder (user installation case)
• system folder aka FILESDIR (default installation case)

The command-line option could help with comparing different configurations more easily without copying them around.

Since the binary folder only seems necessary for development we could add a build flag to disable it so installed binaries are "safer".

[firewave]

After adding logging for this it shows that this is already working as intended.

looking for library 'none'
looking for library 'none.cfg'
looking for library '/mnt/s/GitHub/cppcheck-fw/cmake-build-debug-wsl-kali-clang-17/bin/none.cfg' <-- executable dir
looking for library '/mnt/s/GitHub/cppcheck-fw/cmake-build-debug-wsl-kali-clang-17/bin/../cfg/none.cfg'
looking for library '/mnt/s/GitHub/cppcheck-fw/cmake-build-debug-wsl-kali-clang-17/bin/cfg/none.cfg'
looking for library '/mnt/s/GitHub/cppcheck-fw/cfg/none.cfg' <-- FILESDIR

The loop which processes the path is doing cfgfolders.pop_back() so it looks backwards so the hard-coded path is used last.

Not closing this yet as the additional concerns still need to be tracked in tickets.

[danmar]

Not closing this yet as the additional concerns still need to be tracked in tickets.

good 👍

[firewave]

Closing as there is a much better understanding and test coverage of what it is going on - as well as various fixes. Any future discussion should be done in #7613.