conda-forge / cppcheck-feedstock

A conda-smithy repository for cppcheck.
BSD 3-Clause "New" or "Revised" License
0 stars 11 forks source link

cppcheck on Windows search the wrong cfg folder. #19

Open seanyen opened 4 years ago

seanyen commented 4 years ago

Currently when you run cppcheck on conda-forge on Windows, it will search the cfg files under %CONDA_PREFIX%\Library\bin or %CONDA_PREFIX%\Library\bin\cfg. However, those files are under %CONDA_PREFIX%\Library\share\cppcheck\cfg. I am not yet sure how to instruct the Cppcheck to search a different path, and keep a record here.

tovrstra commented 4 years ago

Sorry for the long delay. I'm not a Windows user, so hard to tell what exactly the problem is. (P.S. I've added you as co-maintainer in #22.) Also on other OSs, the cfg files are installed in share/Cppcheck/cfg, where they do get found. This means the path is somehow not set correctly at compile time.

@timsnyder: do you have by any chance experience with Windows?

pcasenove commented 2 years ago

Hi, Using cppcheck 2.6.1 package on Linux, I get the follwoing error: $ cppcheck . cppcheck: Failed to load library configuration file 'std.cfg'. File not found nofile:0:0: information: Failed to load std.cfg. Your Cppcheck installation is broken, please re-install. The Cppcheck binary was compiled with FILESDIR set to "/home/conda/feedstock_root/build_artifacts/cppcheck_1633254921711/_h_env_placehold_placehold_placehold_placehold_placehold_placehold_placehold_placehold_placehold_placehold_placehold_placehold_placehold_placehold_placehold_placehold_placehold_placehold_pl/share/Cppcheck" and will therefore search for std.cfg in /home/conda/feedstock_root/build_artifacts/cppcheck_1633254921711/_h_env_placehold_placehold_placehold_placehold_placehold_placehold_placehold_placehold_placehold_placehold_placehold_placehold_placehold_placehold_placehold_placehold_placehold_placehold_pl/share/Cppcheck/cfg. [failedToLoadCfg]

Can we do something about it?

Thanks,

tovrstra commented 2 years ago

@pcasenove Thanks for reaching out! It should be possible to fix this. Would you know how? I don't have a windows machine at my disposal (and also lack experience with said OS).

pcasenove commented 2 years ago

I have this on Linux (Centos 7) and I don't have a windows machine at hand either to test. If you prefer, I can open another issue on this, for linux only. I don't know what should be done, I think the .patch file is incorrect.

tovrstra commented 2 years ago

I'm sorry, I did not ready carefully. Fixing the patch would be great. I don't have much time at this stage to look into it.

pcasenove commented 2 years ago

OK... looking at the cppcheck code, it seems that the code to load std.cfg file has changed, explaining the crash: the patch is not applied I think... https://github.com/danmar/cppcheck/blob/e13eba86e5e0d2fbc992d056de9581c0fdc8816f/cli/cppcheckexecutor.cpp

pcasenove commented 2 years ago

Using this make command in the build script seems to fix (for us): VERBOSE=1 make install -j ${CPU_COUNT} CFGDIR=${PREFIX}/share/Cppcheck

tovrstra commented 2 years ago

Seems reasonable. To avoid similar problems in future, we should make a PR with changed tests, which result in this type of error, and then fix it.

senecal-whoop commented 2 years ago

I am currently seeing this issue on windows with cppcheck 2.6.2. Is there any resolution/fix here?

tovrstra commented 2 years ago

Can you try the suggestion by @pcasenove? Does this work for you?

I'm not a Windows user myself, so it would be good to have a contributor for this feedstock who can fix and test the recipe on Windows.

timsnyder commented 2 years ago

I took a serious look at this today and suspect that it has to do with limitations conda has in it's implementation of PREFIX substitution in binary files on windows (see https://github.com/conda/conda/blob/0fa068f7d54e685d66468d0eefe13379743dc125/conda/core/portability.py#L86-L92).

Looking at v1.7.2, it seems that CFGDIR is only used in the uninstall: target of the makefile. It may have helped in the past with related issues but I don't see how it would do anything in the current codebase.

I'll see if I can come up with something that will work on both Linux and Windows. I'm thinking of having something that will look with a path relative to where the executable is installed and lookup the executable path with something like https://github.com/gpakosz/whereami

timsnyder commented 2 years ago

The conan folks are also struggling with this behavior. I found https://github.com/conan-io/conan-center-index/pull/10423#discussion_r862150285 with some interesting pointers and discussions on the behavior of cppcheck.

timsnyder commented 2 years ago

They submitted a patch for mac in https://github.com/danmar/cppcheck/pull/4071, and looking in between the two additions of their patch I see argv[0] twiddling for #ifdef _WIN32. and that code has been in cppcheck since the dawn of time (2014..) https://github.com/MartinDelille/cppcheck/commit/f79e1b6d87c0c0347c47d7dfce3e8fe47198d476

I was concerned about _WIN32 being only set for 32-bit systems but some looking around shows 64-bit windows platforms also have it set https://docs.microsoft.com/en-us/windows-hardware/drivers/kernel/64-bit-compiler?redirectedfrom=MSDN

timsnyder commented 2 years ago

I'm beginning to think that since we shouldn't be setting FILESDIR so that cppcheck will use paths relative to it's installed executable to find the cfg file. When I said "I'll see if I can come up with something that will work for Linux and Windows", that's what I was thinking and it looks like cppcheck already does it if you configure correctly during the build.

Now, I just need to figure out how to get cmake to not set FILESDIR and give it a try.

timsnyder commented 2 years ago

Of course, the cmake lists just throw it in and don't have it behind any kind of option... https://github.com/danmar/cppcheck/blob/06b408ea205d0b056d83855dd420b77314bfdae3/cmake/compilerDefinitions.cmake#L44

I'm going to see what happens if I patch to remove FILESDIR being defined. I'm also going to start running the tests.

The unit tests run and don't care whether they can read the cfg. However, the --check-config test at the end of the linux build doesn't like being built without FILESDIR (and the associated patch) because cmake still installs the cfg to $PREFIX/share/Cppcheck/cfg. Given this, I'm surprised that the test passes in the windows build. I may have to fire up a windows build machine at some point to look into that interactively.

timsnyder commented 2 years ago

The output from the --check-config test on windows does look a little suss though:

(%PREFIX%) %SRC_DIR%>echo ';' ; cppcheck --check-config test.h 1>test.h 

(%PREFIX%) %SRC_DIR%>IF 0 NEQ 0 exit /B 1 

I suspect that the test might not actually be testing anything.

timsnyder commented 2 years ago

I fired up my Windows VM and installed the 2.7.5 package and looked at the info/test/run_test.bat and ran it from a prompt with call run_test.bat (with the env activated), the weird echoing with the redirect looked the same but it does create the test.h with only ; in it and run the config-check.

@senecal-whoop do you still see the issue with the latest version of the package? If so, could you copy paste the exact cmdline and output you are seeing into this ticket?