Skip to content
Merged
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 2 additions & 2 deletions CMakeLists.txt
Original file line number Diff line number Diff line change
Expand Up @@ -222,7 +222,7 @@ if(NOT MSVC AND NOT EMSCRIPTEN)
endif()

# The version of clang currently on our Mac bots doesn't seem to support this flag.
if (CMAKE_CXX_COMPILER_ID MATCHES "Clang" AND (LINUX OR WINDOWS))
if (CMAKE_CXX_COMPILER_ID MATCHES "Clang" AND NOT APPLE)
Copy link
Member

Choose a reason for hiding this comment

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

Do APPLE is a global but LINUX is not? I guess LINUX must have been at some point in the past?

Copy link
Member Author

Choose a reason for hiding this comment

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

I'm not sure if it was in the past, or if some projects define it, or if I just hallucinated it.

add_link_flag("-fuse-ld=lld")
endif()

Expand Down Expand Up @@ -473,7 +473,7 @@ if(BUILD_STATIC_LIB)
else()
message(STATUS "Building libbinaryen as shared library.")
add_library(binaryen SHARED)
if(LINUX)
if(CMAKE_SYSTEM_NAME STREQUAL "Linux")
# Disable interposition and resolve Binaryen symbols locally.
add_link_flag("-Bsymbolic")
Copy link
Member

Choose a reason for hiding this comment

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

Do these two flags were just never being add on linux?

Copy link
Member Author

Choose a reason for hiding this comment

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

This flag was never being added. It was sort of a drive-by addition to #7618.
The line above was modified in #8204 just this week, and specifically in a change I added right before submitting the PR; I didn't test that version locally, only on the bots. The error means that -fuse-lld would just never be added, and it would have broken the next LTO release build (because our use of thinlto requires lld).

endif()
Expand Down
Loading