-
Notifications
You must be signed in to change notification settings - Fork 86
Fix relative paths, again #418
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: master
Are you sure you want to change the base?
Conversation
…eader matching (danmar#362)" (danmar#415)" This reverts commit 9ce981c.
Note: See the compilation test for both UNICODE and non-UNICODE mode at compiler explorer (my best guess) |
ok I try it out here: danmar/cppcheck#7381 |
Well this does not work neither: https://github.com/danmar/cppcheck/actions/runs/13923783226/job/38962964585?pr=7381 CI output:
|
@Tal500 I am sorry for slow reply.. hope you can fix it anyway. |
There must be something wrong with how that you run this test suite. It comlains:
But the thing that I just fixed in this PR is to have only a single, "static", function definition of |
On second glance, the problem seems to be very stupid. Microsoft MSVC complains about redefinition of a function even when one of them is defined to be with "static" visibility, when you do static linkage. EDIT: namespacing seems like a more sane solution EDIT2: hhhhh seems you did exactly what I said already in the code: Lines 70 to 80 in 78f0f7c
So I'm joining the party for now. I suggest using Please try delivering these changes again. |
@Tal500 I tried this quickly. There is a testcase in cppcheck that fails: https://github.com/danmar/cppcheck/actions/runs/14283167360/job/40035087732?pr=7381 The full CI message:
So it seems there is a unwanted message:
|
The selfchecks in the CI also fail but I am not sure if that is caused by these changes right now. |
The test |
I claim that the "valid" value there is WRONG and the "actual" string value is CORRECT. |
It's a matter of taste. I do not mind to have a double coverage, one that checks within the library and one that checks the library within the usage. |
yeah I am not against that we have tests in cppcheck but it seems to me that we could test this directly in simplecpp also.. |
The test cases there seems to be redundant, you can then omit them at all. |
I am not sure am I missing something? There is a file
So we should find that shouldn't we? |
This manual test is the same isn't it?
gcc does not complain about a missing header3.h file. |
I didn't notice that the file is actually being created, so the behavior you claimed is correct. > ./simplecpp -Isystem test.c
(no output) After debugging, I uploaded a fix now. The error was that the wchar to char conversion of the current directory was failing for some reason in windows 2019 release (a very weird bug, really don't know the true cause). static std::string currentDirectoryOSCalc() {
const std::size_t size = 4096;
char currentPath[size];
#ifndef _WIN32
if (getcwd(currentPath, size) != nullptr)
#else
if (_getcwd(currentPath, size) != nullptr)
#endif
return std::string(currentPath);
return "";
} For debugging and verifying, I tried to create a pull request in my fork of cppcheck, to reproduce your checks. The checks looks very good now - Tal500/cppcheck#1 Sadly, I was unable to reproduce this weird behavior in simplecpp automated CI workflow, so while the logical test code is already covered by the python tests, the CI windows setup that reproduced it is not. Please try to integrate it again. |
Thanks I will try it out again |
CI is running now: |
Undo #415 , and hopefully finally fix the Windows string conversion error.
The windows string conversion part is based on https://gist.github.com/RyanJeong/04a4b92879ab10edfd9656d0aa4f3b77
Fixes #402