Skip to content

Use path to executable when trying to load library#5082

Merged
orbitcowboy merged 12 commits into
cppcheck-opensource:mainfrom
chrchr-github:chr_execpath
May 26, 2023
Merged

Use path to executable when trying to load library#5082
orbitcowboy merged 12 commits into
cppcheck-opensource:mainfrom
chrchr-github:chr_execpath

Conversation

@chrchr-github
Copy link
Copy Markdown
Collaborator

No description provided.

Comment thread lib/library.cpp Outdated
@chrchr-github
Copy link
Copy Markdown
Collaborator Author

misra/misra-ctu-2-test.c:0:0: information: Bailing out from checking misra/misra-ctu-2-test.c since there was an internal error:
Failed to execute 'python3 /cygdrive/d/a/cppcheck/cppcheck/addons/runaddon.py /cygdrive/d/a/cppcheck/cppcheck/addons/misra.py --cli misra/misra-ctu-2-test.c.1185.dump'.
C:\hostedtoolcache\windows\Python\3.9.13\x64\python.exe: can't open file 'D:\cygdrive\d\a\cppcheck\cppcheck\addons\runaddon.py': [Errno 2] No such file or directory [internalError]

cppcheck/cppcheck in the path looks suspicious. But the path is composed by getFullPath(), which should already check for existence, so dunno.

Copy link
Copy Markdown
Collaborator

@danmar danmar left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think this looks good.

@chrchr-github
Copy link
Copy Markdown
Collaborator Author

I think this looks good.

Except on Cygwin. I wonder how this worked before.
We get the exename /cygdrive/d/a/cppcheck/cppcheck/cppcheck, which looks valid, and we also check the existence of the derived paths. Yet Python can't find the scripts.
@chrfranke As our resident Cygwin user, maybe you can take a look?

@chrchr-github
Copy link
Copy Markdown
Collaborator Author

So apparently we need a relative path in Cygwin for some reason. This is the command line from CI-cygwin.yml:
..\..\cppcheck.exe --dump -DDUMMY --suppress=uninitvar --inline-suppr misra\misra-test.c --std=c89 --platform=unix64

This reverts commit 5a68d71.
@chrfranke
Copy link
Copy Markdown
Contributor

Except on Cygwin. I wonder how this worked before. We get the exename /cygdrive/d/a/cppcheck/cppcheck/cppcheck, which looks valid, and we also check the existence of the derived paths. Yet Python can't find the scripts.

Cygwin emulates Linux as far as possible so readlink("/proc/self/exe", ...) should work on Cygwin. The Cygwin(!) version of python should find the scripts, but any other Windows version never would.

If you want to support both, the following may work because Cygwin programs also understand Windows pathnames:

+#if defined(_WIN32) || defined(__CYGWIN__)
+    success = (GetModuleFileNameA(nullptr, buf, sizeof(buf)) < sizeof(buf));
+#elif defined(__APPLE__)

@chrchr-github
Copy link
Copy Markdown
Collaborator Author

chrchr-github commented May 24, 2023

Thanks for your quick response.

It seems that the Cygwin path (from readlink()) gets translated to a Windows path automatically, but the result is invalid(?):
C:\hostedtoolcache\windows\Python\3.9.13\x64\python.exe: can't open file 'D:\cygdrive\d\a\cppcheck\cppcheck\addons\runaddon.py': [Errno 2] No such file or directory

@chrfranke
Copy link
Copy Markdown
Contributor

It usually does not make any sense to pass a Cygwin path containing /cygdrive/ to a non-Cygwin program.
Install Cygwin's version of python (https://cygwin.com/packages/summary/python3.html) and retry.

Someone using the Cygwin version of cppcheck for some reason also wants to use the Cygwin version of python :-)

@chrchr-github
Copy link
Copy Markdown
Collaborator Author

That did it, thanks again 👍

@pfultz2
Copy link
Copy Markdown
Contributor

pfultz2 commented May 26, 2023

Can this be merged?

@orbitcowboy orbitcowboy merged commit fb850a8 into cppcheck-opensource:main May 26, 2023
@firewave
Copy link
Copy Markdown
Collaborator

I ran into this as well and when I tried to fix it I came up with the same horrible code we have now because it required the path to be available before we have access to argv[0]. But I deemed that unacceptable so I dumped it.

I didn't even realize that our code already contained the logic partially. If I had I would have just put my code up for review and it would have been fixed earlier.

Sorry about that and thanks for fixing this.

@firewave
Copy link
Copy Markdown
Collaborator

It turns out I didn't delete the code but just didn't finish it: firewave@a997b4e. I haven't checked if there is still something to salvage in there yet.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

6 participants