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.

Sign up for GitHub

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

Fix for cmake-conan issue #225 - Compile error on configuration on MacOS #226

Merged
merged 4 commits into from
Sep 3, 2020
Merged

Fix for cmake-conan issue #225 - Compile error on configuration on MacOS #226

Show file tree
Hide file tree
Changes from 2 commits
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
b89bf34
Fix for cmake-conan issue #225 - Compile error on configuration on MacOS
dforsten Apr 26, 2020
9060253
add test
czoido Sep 3, 2020
8badcd1
Update conan.cmake
memsharded Sep 3, 2020
86920da
Merge branch 'develop' of github.com:conan-io/cmake-conan into df-xco…
czoido Sep 3, 2020
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
6 changes: 5 additions & 1 deletion conan.cmake
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -264,9 +264,13 @@ function(conan_cmake_detect_unix_libcxx result)
endif()
endforeach()

if(APPLE)
memsharded marked this conversation as resolved.
Show resolved Hide resolved
set(xcode_sysroot_option "--sysroot=${CMAKE_OSX_SYSROOT}")
Copy link
Member

Choose a reason for hiding this comment

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

Is it possible that CMAKE_OSX_SYSROOT is not defined? For example in iOS? I would say it is possible, and this would break for some users not in OSX. Does it make sense to test for CMAKE_OSX_SYSROOT existence instead?

(sorry this took a while to review, Conan amazing growth keeps the team very busy)

Copy link
Contributor

Choose a reason for hiding this comment

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

I also would suggest to check CMAKE_OSX_SYSROOT instead of APPLE.
Also @dforsten I have added a test for this issue.

endif()

execute_process(
COMMAND ${CMAKE_COMMAND} -E echo "#include <string>"
COMMAND ${CMAKE_CXX_COMPILER} -x c++ ${compile_options} -E -dM -
COMMAND ${CMAKE_CXX_COMPILER} -x c++ ${xcode_sysroot_option} ${compile_options} -E -dM -
OUTPUT_VARIABLE string_defines
)

Expand Down
24 changes: 24 additions & 0 deletions tests.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -48,6 +48,30 @@ def tearDown(self):
os.environ.clear()
os.environ.update(self.old_env)

# https://github.com/conan-io/cmake-conan/issues/159
@unittest.skipIf(platform.system() != "Darwin", "Error message appears just in Macos")
def test_macos_sysroot_warning(self):
content = textwrap.dedent("""
cmake_minimum_required(VERSION 2.8)
project(FormatOutput CXX)
set(CMAKE_CXX_STANDARD 11)
include(conan.cmake)
conan_cmake_run(REQUIRES fmt/6.1.2
BASIC_SETUP
BUILD missing)

add_executable(main main.cpp)
target_link_libraries(main ${CONAN_LIBS})
""")
save("CMakeLists.txt", content)

os.makedirs("build")
os.chdir("build")
run("cmake .. %s -DCMAKE_BUILD_TYPE=Release 2> stderr_output.txt" % generator)
with open('stderr_output.txt', 'r') as file:
data = file.read()
assert "#include_next <string.h>" not in data

def test_global_update(self):
content = """set(CMAKE_CXX_COMPILER_WORKS 1)
set(CMAKE_CXX_ABI_COMPILED 1)
Expand Down