Skip to content

fix(cmake): Use CMAKE_BINARY_DIR for installation path #1510

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

Merged
merged 1 commit into from
May 2, 2025

Conversation

wuchangming
Copy link
Contributor

@wuchangming wuchangming commented Apr 23, 2025

Replace relative path with CMAKE_BINARY_DIR in Install.cmake to ensure consistent installation behavior across different platforms.
fix fail in macOS.

Replace relative path with CMAKE_BINARY_DIR in Install.cmake to ensure
consistent installation behavior across different platforms and support
out-of-source builds. This change follows CMake best practices by keeping
generated files within the build directory.
@bghgary
Copy link
Contributor

bghgary commented Apr 28, 2025

I'm not opposed to this change, but I don't see a problem on macOS. Can you elaborate on what you are seeing?

@wuchangming
Copy link
Contributor Author

When I build the 'install target' in xcode, I get the following error. And I saw other people also mentioned this problem, link

image

I'm not opposed to this change, but I don't see a problem on macOS. Can you elaborate on what you are seeing?

@bghgary
Copy link
Contributor

bghgary commented May 2, 2025

When I build the 'install target' in xcode, I get the following error.

Hmm, I've never built the install target from XCode.

Maybe this fix will make it so that you can pass relative paths to the prefix? Like this cmake --install --prefix ./install? And maybe that's what XCode is doing? If so, then it makes sense to me.

@bghgary bghgary merged commit fc7c7c2 into BabylonJS:master May 2, 2025
19 checks passed
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.

2 participants