Skip to content
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

MakeResourcesPath shouldn't be using a hardcoded absolute path on windows #194

Open
fsfod opened this issue Jan 28, 2024 · 3 comments
Open

Comments

@fsfod
Copy link
Contributor

fsfod commented Jan 28, 2024

The build path of the binary that CppInterOp.cpp is compiled in to on windows is unlikely to be the same it run with, but MakeResourcesPath always uses LLVM_BINARY_DIR because it always set by llvm cmake a far as a i can tell.
The macro is conditionally used in the code here:

#ifdef LLVM_BINARY_DIR
Dir = LLVM_BINARY_DIR;
#else
// Dir is bin/ or lib/, depending on where BinaryPath is.
void *MainAddr = (void *)(intptr_t)GetExecutablePath;
std::string BinaryPath = GetExecutablePath(/*Argv0=*/nullptr, MainAddr);

but in CMake its unconditionally set here:
set_source_files_properties(CppInterOp.cpp PROPERTIES COMPILE_DEFINITIONS
"LLVM_BINARY_DIR=\"${LLVM_BINARY_DIR}\";CPPINTEROP_VERSION=\"${_VAR}\""
)

@vgvassilev
Copy link
Contributor

Yes, that is the intent for now. This logic is needed to be able to pass the correct resource directory path (-resource-dir) to the compiler. Where does are the compiler resource headers put on windows if not relative to the LLVM_BINARY_DIR?

@fsfod
Copy link
Contributor Author

fsfod commented Jan 28, 2024

It would ..\lib from the executable directory i think, which is what the other side of the #ifdef gets close to doing.

@vgvassilev
Copy link
Contributor

It would ..\lib from the executable directory i think, which is what the other side of the #ifdef gets close to doing.

We don't have a reasonable way out I fear here. The problem with the other part of the ifdef is that it works well for the binary clang. The installation location of the clang binary is pretty well known and the relative computation of the paths works well. The problem for CppInterOp is that it is a library and can be installed on many more different locations. That's why we have relied on the LLVM_BINARY_DIR which seems to be somewhat more stable. Apparently that needs more work on Windows I guess...

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

No branches or pull requests

2 participants