Hi! 

I believe we move from patch to two files originally
+++ b/CMakeLists.txt
+++ b/cmake/LuaJITUtils.cmake

to just one, in 432cdf62303b0d609525acc84b01b92ae468d327
+++ b/cmake/LuaJITUtils.cmake

I expect to see a ‘v2’ in such a case - for the forthcoming patches.

Anyways, if I read the correct patch above from the imun/gh-5983-fix-build-on-m1
branch then it’s LGTM with just a typo fix.

Sergos


On 19 May 2021, at 14:38, Igor Munkin <imun@tarantool.org> wrote:

As a result of offline discussion with Sergey, I've finally understood
the docs and fixed the issue. Diff is below:

================================================================================

diff --git a/cmake/LuaJITUtils.cmake b/cmake/LuaJITUtils.cmake
index 3497edc4..29c425de 100644
--- a/cmake/LuaJITUtils.cmake
+++ b/cmake/LuaJITUtils.cmake
@@ -4,7 +4,12 @@ function(LuaJITTestArch outvar strflags)
  # machinery) or explicitly (manually by configuration options).
  # Need -isysroot flag on recentish MacOS after command line
  # tools no longer provide headers in /usr/include.
-  if(CMAKE_OSX_SYSROOT)
+  # XXX: According to CMake documentation[1], CMAKE_OSX_SYSROOT
+  # variable *should* be ignored on the plaforms other than MacOS.
  platforms
+  # It is ignored by CMake, but since this routine is extension
+  # it also should follow this policy.
+  # [1]: https://cmake.org/cmake/help/v3.1/variable/CMAKE_OSX_SYSROOT.html
+  if(CMAKE_SYSTEM_NAME STREQUAL "Darwin" AND CMAKE_OSX_SYSROOT)
    set(strflags "${strflags} -isysroot ${CMAKE_OSX_SYSROOT}")
  endif()
  # XXX: <execute_process> simply splits the COMMAND argument by

================================================================================

On 18.05.21, Igor Munkin wrote:
Sergey,


<snipped>


diff --git a/cmake/LuaJITUtils.cmake b/cmake/LuaJITUtils.cmake
index d9f8b12a..3497edc4 100644
--- a/cmake/LuaJITUtils.cmake
+++ b/cmake/LuaJITUtils.cmake
@@ -1,4 +1,12 @@
function(LuaJITTestArch outvar strflags)
+  # XXX: This routine uses external headers (e.g. system ones),
+  # which location is specified either implicitly (within CMake
+  # machinery) or explicitly (manually by configuration options).
+  # Need -isysroot flag on recentish MacOS after command line
+  # tools no longer provide headers in /usr/include.
+  if(CMAKE_OSX_SYSROOT)

Nit: This variable should be taken into account only for OS X
platforms [1]. I suppose not only by CMake, but by our build system too.

I've got the error on Linux if I try to build LuaJIT like the following
(by mistake of course):

| $ uname -s
| Linux
| $ cmake . -DCMAKE_OSX_SYSROOT="/tmp/blablabalblablbalalbla" -DLUA_USE_ASSERT=ON -DCMAKE_BUILD_TYPE=Debug

Sigh. After all, I've tried for three patches, seems like ninety...

BTW, I see failures even on MacOS:
| $ uname -msr
| Darwin 20.3.0 arm64
| $ cmake . -DCMAKE_OSX_SYSROOT="/tmp"
| -- The C compiler identification is AppleClang 12.0.0.12000032
| -- Detecting C compiler ABI info
| -- Detecting C compiler ABI info - failed
| -- Check for working C compiler: /Library/Developer/CommandLineTools/usr/bin/cc
| -- Check for working C compiler: /Library/Developer/CommandLineTools/usr/bin/cc - broken
| CMake Error at /opt/homebrew/Cellar/cmake/3.20.1/share/cmake/Modules/CMakeTestCCompiler.cmake:66 (message):
|   The C compiler
|
|     "/Library/Developer/CommandLineTools/usr/bin/cc"
|
|   is not able to compile a simple test program.
|
|   It fails with the following output:
|
|     Change Dir: /Users/tntmac07.tarantool.i/imun/tarantool-luajit/CMakeFiles/CMakeTmp
|
|     Run Build Command(s):/usr/bin/make -f Makefile cmTC_da00c/fast && /Library/Developer/CommandLineTools/usr/bin/make  -f CMakeFiles/cmTC_da00c.dir/build.make CMakeFiles/cmTC_da00c.dir/build
|     Building C object CMakeFiles/cmTC_da00c.dir/testCCompiler.c.o
|     /Library/Developer/CommandLineTools/usr/bin/cc   -arch arm64 -isysroot /tmp -mmacosx-version-min=11.1 -MD -MT CMakeFiles/cmTC_da00c.dir/testCCompiler.c.o -MF CMakeFiles/cmTC_da00c.dir/testCCompiler.c.o.d -o CMakeFiles/cmTC_da00c.dir/testCCompiler.c.o -c /Users/tntmac07.tarantool.i/imun/tarantool-luajit/CMakeFiles/CMakeTmp/testCCompiler.c
|     Linking C executable cmTC_da00c
|     /opt/homebrew/Cellar/cmake/3.20.1/bin/cmake -E cmake_link_script CMakeFiles/cmTC_da00c.dir/link.txt --verbose=1
|     /Library/Developer/CommandLineTools/usr/bin/cc  -arch arm64 -isysroot /tmp -mmacosx-version-min=11.1 -Wl,-search_paths_first -Wl,-headerpad_max_install_names CMakeFiles/cmTC_da00c.dir/testCCompiler.c.o -o cmTC_da00c
|     ld: library not found for -lSystem
|     clang: error: linker command failed with exit code 1 (use -v to see invocation)
|     make[1]: *** [cmTC_da00c] Error 1
|     make: *** [cmTC_da00c/fast] Error 2
|
|
|
|
|
|   CMake will not be able to correctly generate this project.
| Call Stack (most recent call first):
|   CMakeLists.txt:12 (project)
|
|
| -- Configuring incomplete, errors occurred!
| See also "/Users/tntmac07.tarantool.i/imun/tarantool-luajit/CMakeFiles/CMakeOutput.log".
| See also "/Users/tntmac07.tarantool.i/imun/tarantool-luajit/CMakeFiles/CMakeError.log".

I have no idea how to fix this, so I propose to classify manual
definition of CMAKE_OSX_SYSROOT as PEBKAC and move on. Thoughts?

Why we can't just change the check to the following?

For what? The problem still exists on MacOS. Furthermore, the problem
with the same nature (from my point of view).


| if(CMAKE_SYSTEM_NAME STREQUAL "Darwin" AND CMAKE_OSX_SYSROOT)

At least it fixes the case I mentioned above, doesn't it? And we
definitely can't protect users from wrong sysroot paths on OS X systems
like in your example. This case looks like "Сам себе злобный Буратино".

Yes, it fixes. But it does not on MacOS. Hence, why do we need to handle
this only on Linux? Moreover, who will set CMAKE_OSX_SYSROOT (not for
the testing reason) on Linux? There is even *OSX* in the variable name.
I am more concerned with the fact, one can break the build with this on
MacOS. Unfortunately, it seems in this case we can do nothing with it.

Sorry, but I see no arguments for solving this particular case.

Ignoring.



| In file included from lua.h:14,
|                  from lj_arch.h:9:
| /usr/lib/gcc/x86_64-pc-linux-gnu/9.3.0/include/stdint.h:9:16: fatal error: stdint.h: No such file or directory
|     9 | # include_next <stdint.h>
|       |                ^~~~~~~~~~
| compilation terminated.
| CMake Error at cmake/LuaJITUtils.cmake:48 (message):
|   [LuaJITArch] Unsupported target architecture
| Call Stack (most recent call first):
|   cmake/SetTargetFlags.cmake:16 (LuaJITArch)
|   src/CMakeLists.txt:17 (include)

+    set(strflags "${strflags} -isysroot ${CMAKE_OSX_SYSROOT}")
+  endif()
  # XXX: <execute_process> simply splits the COMMAND argument by
  # spaces with no further parsing. At the same time GCC is bad in
  # argument handling, so let's help it a bit.

================================================================================

[1]: https://gitlab.kitware.com/cmake/cmake/-/blob/master/Source/cmLocalGenerator.cxx#L1911-1916
[2]: https://gitlab.kitware.com/cmake/cmake/-/blob/master/Tests/FindPackageModeMakefileTest/CMakeLists.txt#L24-28


-- 
Best regards,
IM

[1]: https://cmake.org/cmake/help/v3.0/variable/CMAKE_OSX_SYSROOT.html

-- 
Best regards,
Sergey Kaplun

-- 
Best regards,
IM

-- 
Best regards,
Sergey Kaplun

[1]: https://github.com/LuaJIT/LuaJIT/commit/8961a92

-- 
Best regards,
IM

-- 
Best regards,
IM