Cmake + llvm + wine to work in tree view #6423
Conversation
| @@ -3083,6 +3083,14 @@ export class BaseCompiler implements ICompiler { | |||
| if (result.inputFilename) { | |||
| result.inputFilename = utils.maskRootdir(result.inputFilename); | |||
| } | |||
|
|
|||
| if (result.asm) { | |||
There was a problem hiding this comment.
On our machine running compile explorer the source was in /tmp/compile-exploer-[some random chars]/main.cpp for example. Thus the main.cpp would never match in the tree view with this to highlight source code to match the asm.
There was a problem hiding this comment.
I 100% understand the reasoning, but this is probably not the best way to solve the problem. I'll have a think about it
There was a problem hiding this comment.
I'm not 100% following what's happening here, but we can't have this in the mainline version of CE; it's a pretty core thing that ought to be handled elsewhere.
There was a problem hiding this comment.
If I take the cmake template it will create a main.cpp with a CMakeLists.txt. On our server it would compile that file into a /tmp folder. While on godbolt it would be in the /app folder. Then on our server when we would hover over the C++ code it would not high light the equivalent asm. To solve that I had to do this code change. I'm not sure why. If there is a proper way to fix that I am completely open :)
| @@ -50,11 +51,17 @@ export class ExternalParserBase implements IExternalParser { | |||
| private getObjdumpStarterScriptContent(filters: ParseFiltersAndOutputOptions) { | |||
| const parserArgs = this.getParserArguments(filters, true); | |||
|
|
|||
| let winePrefix = ""; | |||
There was a problem hiding this comment.
When the objdump is an exe it would not work. I'm not sure if this is the best way to find out if the objdump is an exe, please let me know if there is a better way.
There was a problem hiding this comment.
Winedebug things are probably best to leave to CE itself, but there probably isn't a way to set WINEDEBUG at the moment (although it's slightly insane wine requires this to run this, but that's a different can of worms probably)
There was a problem hiding this comment.
Should also probably check if this is running on linux as well.
| @@ -80,7 +80,7 @@ export function maskRootdir(filepath: string): string { | |||
| .replace(/^\/app\//, ''); | |||
| } else { | |||
| const re = getRegexForTempdir(); | |||
| return filepath.replace(re, '/app/').replace(/^\/app\//, ''); | |||
| return filepath.replace(/\\/, '/').replace(re, '/app/').replace(/^\/app\//, ''); | |||
There was a problem hiding this comment.
When using an objdump that is an exe it would return a path with \. This fixes that.
There was a problem hiding this comment.
does this work with paths on linux that have spaces? (cmdline they are sometimes written as \ and it's a bit of strange escape exception. I guess it would be ok until users actually passed \ and it might be ok to error on that)
There was a problem hiding this comment.
I didn't check with spaces. The wine clang objdump would send a path like /something/something\main.cpp
![github-advanced-security[bot]](https://avatars.githubusercontent.com/in/57789?s=60&v=4){kind=small}
| @@ -80,7 +80,7 @@ | |||
| .replace(/^\/app\//, ''); | |||
| } else { | |||
| const re = getRegexForTempdir(); | |||
| return filepath.replace(re, '/app/').replace(/^\/app\//, ''); | |||
| return filepath.replace(/\\/, '/').replace(re, '/app/').replace(/^\/app\//, ''); | |||
Check failure
Code scanning / CodeQL
Incomplete string escaping or encoding High
There was a problem hiding this comment.
Is this error worth heeding here?
There was a problem hiding this comment.
I'm not a regex expert so I'm going with a yes. Is this the proper fix to handle the case above?
|
It will be a while until I'll be coming back to this PR, sorry for the delay |
When trying to upload a zip and use cmake + llvm + wine it was not working with compile explorer. These changes were required.