Don't invoke make directly; use CMAKE_BUILD_TOOL #10

ke6jjj · 2021-02-23T00:01:22Z

This project depends on GNUMake, which may not be called make on
all systems. Use the more portable CMAKE_BUILD_TOOL variable instead.

This project depends on GNUMake, which may not be called `make` on all systems. Use the more portable `CMAKE_BUILD_TOOL` variable instead.

Use `CMAKE_BUILD_TOOL` instead of `make`.

JayKickliter · 2021-02-23T19:01:35Z

let's hold off on merging this because erlang-pbc builds are failing on at least macOS after merging a similar PR. It shouldn't break anything, but I want to find the root cause

ke6jjj · 2021-02-23T19:03:07Z

@JayKickliter which branch is failing? I can test on MacOS and probably get to that root cause.

evanmcc · 2021-02-23T19:05:43Z

master and patch-1 are failing on big sur

ke6jjj · 2021-02-23T19:07:50Z

I see it:

checking whether we are cross compiling... configure: error: in `/Users/jeremy/src/git/erlang-erasure-check/_build/c_src/gf-complete/src/gf-complete':
configure: error: cannot run C compiled programs.
If you meant to cross compile, use `--host'.
See `config.log' for more details
make[3]: *** [gf-complete/src/gf-complete-stamp/gf-complete-configure] Error 1
make[2]: *** [CMakeFiles/gf-complete.dir/all] Error 2
make[1]: *** [all] Error 2

ke6jjj · 2021-02-23T19:10:41Z

Haven't gotten to the bottom of it yet, but I see the issue:

configure:3485: /Applications/Xcode.app/Contents/Developer/Toolchains/XcodeDefault.xctoolchain/usr/bin/cc -o conftest -O3 -DNDEBUG   conftest.c  >&5
conftest.c:11:10: fatal error: 'stdio.h' file not found
#include <stdio.h>
         ^~~~~~~~~
1 error generated.

JayKickliter · 2021-02-23T19:12:32Z

That's exactly the error that prompted this PR: helium/erlang-pbc#8

ke6jjj · 2021-02-23T19:22:45Z

What I'm noticing is that invoking cc as /usr/bin/cc produces different behavior than invoking it directly as /Applications/Xcode.app/Contents/Developer/Toolchains/XcodeDefault.xctoolchain/usr/bin/cc, even though, nominally, /usr/bin/cc is just a link for the latter. When compiling with /usr/bin/cc things work just fine.

It appears that this version of clang is checking argv[0] to determine how it should behave.

ke6jjj · 2021-02-23T19:26:06Z

I'm currently reading this Cmake article, which may be our solution:

https://discourse.cmake.org/t/help-with-cc-clang-environment-and-cmake/2221

ke6jjj · 2021-02-23T19:49:01Z

This works:

make SDKROOT=`xcrun --show-sdk-path`

Now to just find a portable/easy way to ensure it runs.

JayKickliter · 2021-02-23T20:04:56Z

this looks like a potential solution:

index e3b1f2b..41249e4 100644
--- a/c_src/CMakeLists.txt
+++ b/c_src/CMakeLists.txt
@@ -12,6 +12,11 @@ include(ASan)
 IF (APPLE)
   set(CMAKE_MODULE_LINKER_FLAGS "-flat_namespace -undefined suppress")
   set(CMAKE_MACOSX_RPATH 1)
+  execute_process(COMMAND
+    xcrun --show-sdk-path
+    OUTPUT_VARIABLE APPLE_SDK_ROOT
+    )
+  set(sludgeOS_SDKENV "SDKROOT=${APPLE_SDK_ROOT}")
 ENDIF()

 #
diff --git a/c_src/cmake/FindGFComplete.cmake b/c_src/cmake/FindGFComplete.cmake
index 18fa5ba..6cdd09b 100644
--- a/c_src/cmake/FindGFComplete.cmake
+++ b/c_src/cmake/FindGFComplete.cmake
@@ -19,7 +19,8 @@ ExternalProject_Add(gf-complete
                     $ENV{CONFIGURE_ARGS}
                     CC=${CMAKE_C_COMPILER}
                     CFLAGS=${CMAKE_C_FLAGS_${BUILD_TYPE_UC}}
-  BUILD_COMMAND     make -j
+                    ${sludgeOS_SDKENV}
+  BUILD_COMMAND     make -j ${sludgeOS_SDKENV}
   BUILD_BYPRODUCTS  ${CMAKE_CURRENT_BINARY_DIR}/lib/libgf_complete.a
   INSTALL_COMMAND   make install
   )
diff --git a/c_src/cmake/FindJerasure.cmake b/c_src/cmake/FindJerasure.cmake
index 3b7e800..87db016 100644
--- a/c_src/cmake/FindJerasure.cmake
+++ b/c_src/cmake/FindJerasure.cmake
@@ -23,7 +23,8 @@ ExternalProject_Add(jerasure
                     CC=${CMAKE_C_COMPILER}
                     "CFLAGS=-I${CMAKE_CURRENT_BINARY_DIR}/include ${CMAKE_C_FLAGS_${BUILD_TYPE_UC}}"
                     LDFLAGS=-L${CMAKE_CURRENT_BINARY_DIR}/lib
-  BUILD_COMMAND     make -j
+                    ${sludgeOS_SDKENV}
+  BUILD_COMMAND     make -j ${sludgeOS_SDKENV}
   BUILD_BYPRODUCTS  ${CMAKE_CURRENT_BINARY_DIR}/lib/libJerasure.a
   INSTALL_COMMAND   make install
   )

the same needs to be done for the jerasure dep and it we'd need to make sure it's portable (unless there's a better, less-ugly fix you can think of)

ke6jjj · 2021-02-23T20:11:04Z

It's definitely ugly, but I can't think of anything better for now. I tried this and it Works On My Box.

JayKickliter · 2021-02-23T20:14:45Z

do you want to incorporate it into your PR (see updated suggested diff above) so you don't have to rebase? either way is fine

ke6jjj · 2021-02-23T20:15:23Z

Yes, I'm testing right now. Will update this branch with OSX fixes.

JayKickliter · 2021-02-23T20:15:57Z

thanks! And sorry for mixing concerns and stepping on your PR

JayKickliter · 2021-02-23T20:16:52Z

once updated I make check that it still works on older macOS (which I usually run) and linux

I still don't know why passing SDKROOT to autools+make fixes the problem.

Fix builds for Mac OS X hosts using a method that JayKickliter and I derived while researching this current problem with CMake. In the future, CMake will probably come up with some reasonable defaults that update CFLAGS properly on Mac OS X so that it gets the proper `-isysroot <PATH>` that is sorely needed when invoking `clang` as directly as CMake is doing.

ke6jjj · 2021-02-23T20:23:56Z

Happy to mix concerns here. I am an OS X user as well, so this is a two-bugs with one patch win-win for me.

ke6jjj · 2021-02-23T20:26:34Z

Builds under FreeBSD 12.1-amd64 as well, now, too.

JayKickliter · 2021-02-23T20:38:21Z

I've confirmed this works on macOS 10.14 Mojave. CI will test linux

JayKickliter · 2021-02-23T20:39:06Z

we'll need to add this same fix to erlang-pbc

JayKickliter

Thanks. I'll merge when you think you're done tweaking.

ke6jjj · 2021-02-23T20:47:15Z

I think I'm done! I also updated my patch-1 branch of erlang-pbc just now.

JayKickliter · 2021-02-23T20:49:38Z

branch of erlang-pbc just now
that PR merged. mind doing a rebase and reopening a new PR with your branch?

ke6jjj added 2 commits February 22, 2021 16:01

Don't invoke make directly; use CMAKE_BUILD_TOOL

1aa9576

This project depends on GNUMake, which may not be called `make` on all systems. Use the more portable `CMAKE_BUILD_TOOL` variable instead.

More make/GNUMake protection.

6a24194

Use `CMAKE_BUILD_TOOL` instead of `make`.

ke6jjj added 2 commits February 23, 2021 12:33

Centralize XCode SDK ugliness.

0ed9a36

Don't explicitly set XCode SDK root if already specified.

4974e4a

JayKickliter approved these changes Feb 23, 2021

View reviewed changes

JayKickliter merged commit f8f9a80 into helium:master Feb 23, 2021

JayKickliter mentioned this pull request Feb 23, 2021

Fix Mac OS X builds under CMake. helium/erlang-pbc#14

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Don't invoke make directly; use CMAKE_BUILD_TOOL #10

Don't invoke make directly; use CMAKE_BUILD_TOOL #10

ke6jjj commented Feb 23, 2021

JayKickliter commented Feb 23, 2021

ke6jjj commented Feb 23, 2021

evanmcc commented Feb 23, 2021

ke6jjj commented Feb 23, 2021

ke6jjj commented Feb 23, 2021

JayKickliter commented Feb 23, 2021

ke6jjj commented Feb 23, 2021 •

edited

Loading

ke6jjj commented Feb 23, 2021

ke6jjj commented Feb 23, 2021 •

edited

Loading

JayKickliter commented Feb 23, 2021 •

edited

Loading

ke6jjj commented Feb 23, 2021

JayKickliter commented Feb 23, 2021

ke6jjj commented Feb 23, 2021

JayKickliter commented Feb 23, 2021

JayKickliter commented Feb 23, 2021 •

edited

Loading

ke6jjj commented Feb 23, 2021

ke6jjj commented Feb 23, 2021

JayKickliter commented Feb 23, 2021

JayKickliter commented Feb 23, 2021

JayKickliter left a comment

ke6jjj commented Feb 23, 2021

JayKickliter commented Feb 23, 2021

Don't invoke make directly; use CMAKE_BUILD_TOOL #10

Don't invoke make directly; use CMAKE_BUILD_TOOL #10

Conversation

ke6jjj commented Feb 23, 2021

JayKickliter commented Feb 23, 2021

ke6jjj commented Feb 23, 2021

evanmcc commented Feb 23, 2021

ke6jjj commented Feb 23, 2021

ke6jjj commented Feb 23, 2021

JayKickliter commented Feb 23, 2021

ke6jjj commented Feb 23, 2021 • edited Loading

ke6jjj commented Feb 23, 2021

ke6jjj commented Feb 23, 2021 • edited Loading

JayKickliter commented Feb 23, 2021 • edited Loading

ke6jjj commented Feb 23, 2021

JayKickliter commented Feb 23, 2021

ke6jjj commented Feb 23, 2021

JayKickliter commented Feb 23, 2021

JayKickliter commented Feb 23, 2021 • edited Loading

ke6jjj commented Feb 23, 2021

ke6jjj commented Feb 23, 2021

JayKickliter commented Feb 23, 2021

JayKickliter commented Feb 23, 2021

JayKickliter left a comment

Choose a reason for hiding this comment

ke6jjj commented Feb 23, 2021

JayKickliter commented Feb 23, 2021

ke6jjj commented Feb 23, 2021 •

edited

Loading

ke6jjj commented Feb 23, 2021 •

edited

Loading

JayKickliter commented Feb 23, 2021 •

edited

Loading

JayKickliter commented Feb 23, 2021 •

edited

Loading