Skip to content

cmake: Remove Windows-only install targets from Mac#205

Closed
jblazquez wants to merge 1 commit intogetsentry:masterfrom
jblazquez:master
Closed

cmake: Remove Windows-only install targets from Mac#205
jblazquez wants to merge 1 commit intogetsentry:masterfrom
jblazquez:master

Conversation

@jblazquez
Copy link
Copy Markdown
Contributor

This change fixes one part of #204. With this change, you can now build with -DBUILD_SHARED_LIBS=NO on macOS, as long as you also disable the install target using -DSENTRY_ENABLE_INSTALL=NO.

LIBRARY DESTINATION "${CMAKE_INSTALL_LIBDIR}"
ARCHIVE DESTINATION "${CMAKE_INSTALL_LIBDIR}"
)
if(WIN32)
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Copy link
Copy Markdown
Contributor Author

@jblazquez jblazquez Mar 30, 2020

Choose a reason for hiding this comment

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

Good catch. I was looking at the version of crashpad's CMakeLists.txt from the 0.2.2 release and that used to say IF(WIN32) there. I will update to say IF(MSVC) here for consistency with master.

EDIT: Actually, the sentry-native master branch points to a crashpad submodule commit that still says IF(WIN32), so this is consistent with that. Should we keep it this way until someone rolls the version of the crashpad submodule forward?

https://github.com/getsentry/crashpad/blob/8a2ddfc0a475bff9e25bfc7af693bac373adebe3/CMakeLists.txt#L75

@jblazquez
Copy link
Copy Markdown
Contributor Author

Looks like two CI builders failed with timeouts. How does one retry Azure Pipelines builds?

@Swatinem
Copy link
Copy Markdown
Contributor

How does one retry Azure Pipelines builds?

by having admin priviledges I guess :-D

Also, I don’t think this PR is necessary anymore, @madebr made very good progress restructuring the crashpad CMake files. I will review all this today.

@Swatinem
Copy link
Copy Markdown
Contributor

Closing this in favor of #207, but thanks a lot for looking into this!

@Swatinem Swatinem closed this Mar 31, 2020
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.

3 participants