Tarantool development patches archive
 help / color / mirror / Atom feed
* [Tarantool-patches] [PATCH luajit 0/3] Integration testing and tiny fixes in CI
@ 2022-06-22 15:33 Igor Munkin via Tarantool-patches
  2022-06-22 15:33 ` [Tarantool-patches] [PATCH luajit 1/3] ci: fix --parallel argument for MacOS runners Igor Munkin via Tarantool-patches
                   ` (3 more replies)
  0 siblings, 4 replies; 21+ messages in thread
From: Igor Munkin via Tarantool-patches @ 2022-06-22 15:33 UTC (permalink / raw)
  To: Sergey Kaplun, Sergey Bronnikov; +Cc: tarantool-patches

After reusable workflow is merged into tarantool/tarantool master
branch[1], we can proceed with LuaJIT integration testing patchset. The
changeset contains a couple of CI-related fixes and introduces Tarantool
integration testing in LuaJIT repository. You can find more verbose
descriptions in the corresponding commit messages.

Branch: https://github.com/tarantool/luajit/tree/imun/ci-tarantool-tests
CI: https://github.com/tarantool/luajit/commit/0b72879

[1]: https://github.com/tarantool/tarantool/pull/7276

Igor Munkin (3):
  ci: fix --parallel argument for MacOS runners
  ci: remove GC64 matrix entries for ARM64 workflows
  ci: add Tarantool integration testing

 .github/workflows/linux-aarch64.yml | 28 ++++++++++++++---
 .github/workflows/linux-x86_64.yml  | 43 ++++++++++++++++++++++++--
 .github/workflows/macos-m1.yml      | 32 +++++++++++++++-----
 .github/workflows/macos-x86_64.yml  | 47 ++++++++++++++++++++++++++---
 4 files changed, 130 insertions(+), 20 deletions(-)

-- 
2.34.0


^ permalink raw reply	[flat|nested] 21+ messages in thread

* [Tarantool-patches] [PATCH luajit 1/3] ci: fix --parallel argument for MacOS runners
  2022-06-22 15:33 [Tarantool-patches] [PATCH luajit 0/3] Integration testing and tiny fixes in CI Igor Munkin via Tarantool-patches
@ 2022-06-22 15:33 ` Igor Munkin via Tarantool-patches
  2022-06-23  8:19   ` Sergey Kaplun via Tarantool-patches
                     ` (2 more replies)
  2022-06-22 15:33 ` [Tarantool-patches] [PATCH luajit 2/3] ci: remove GC64 matrix entries for ARM64 workflows Igor Munkin via Tarantool-patches
                   ` (2 subsequent siblings)
  3 siblings, 3 replies; 21+ messages in thread
From: Igor Munkin via Tarantool-patches @ 2022-06-22 15:33 UTC (permalink / raw)
  To: Sergey Kaplun, Sergey Bronnikov; +Cc: tarantool-patches

In scope of the commit 4195eb8f7e2abcf75f92eedd2859b7991cc8b363 ("ci:
make GitHub workflows more CMake-ish") --parallel value has been
explicitly set to $(nproc) + 1, since -j default behaviour differs for
Ninja and GnuMake. However, MacOS lacks nproc command, but CI silently
continues to execute pipeline.

This patch fixed MacOS-specific workflows changing nproc command with
sysctl -n hw.ncpu.

Signed-off-by: Igor Munkin <imun@tarantool.org>
---
 .github/workflows/macos-m1.yml     | 4 ++--
 .github/workflows/macos-x86_64.yml | 4 ++--
 2 files changed, 4 insertions(+), 4 deletions(-)

diff --git a/.github/workflows/macos-m1.yml b/.github/workflows/macos-m1.yml
index 8387bbcf..d1d144f0 100644
--- a/.github/workflows/macos-m1.yml
+++ b/.github/workflows/macos-m1.yml
@@ -68,6 +68,6 @@ jobs:
       - name: configure
         run: ${ARCH} cmake . ${{ matrix.CMAKEFLAGS }} -DLUAJIT_ENABLE_GC64=${{ matrix.GC64 }}
       - name: build
-        run: ${ARCH} cmake --build . --parallel $(($(nproc) + 1))
+        run: ${ARCH} cmake --build . --parallel $(($(sysctl -n hw.ncpu) + 1))
       - name: test
-        run: ${ARCH} cmake --build . --parallel $(($(nproc) + 1)) --target test
+        run: ${ARCH} cmake --build . --parallel $(($(sysctl -n hw.ncpu) + 1)) --target test
diff --git a/.github/workflows/macos-x86_64.yml b/.github/workflows/macos-x86_64.yml
index 2ab2c8d0..a7c2f4e3 100644
--- a/.github/workflows/macos-x86_64.yml
+++ b/.github/workflows/macos-x86_64.yml
@@ -63,6 +63,6 @@ jobs:
       - name: configure
         run: cmake . ${{ matrix.CMAKEFLAGS }} -DLUAJIT_ENABLE_GC64=${{ matrix.GC64 }}
       - name: build
-        run: cmake --build . --parallel $(($(nproc) + 1))
+        run: cmake --build . --parallel $(($(sysctl -n hw.ncpu) + 1))
       - name: test
-        run: cmake --build . --parallel $(($(nproc) + 1)) --target test
+        run: cmake --build . --parallel $(($(sysctl -n hw.ncpu) + 1)) --target test
-- 
2.34.0


^ permalink raw reply	[flat|nested] 21+ messages in thread

* [Tarantool-patches] [PATCH luajit 2/3] ci: remove GC64 matrix entries for ARM64 workflows
  2022-06-22 15:33 [Tarantool-patches] [PATCH luajit 0/3] Integration testing and tiny fixes in CI Igor Munkin via Tarantool-patches
  2022-06-22 15:33 ` [Tarantool-patches] [PATCH luajit 1/3] ci: fix --parallel argument for MacOS runners Igor Munkin via Tarantool-patches
@ 2022-06-22 15:33 ` Igor Munkin via Tarantool-patches
  2022-06-23  8:29   ` Sergey Kaplun via Tarantool-patches
  2022-07-01 13:21   ` Sergey Bronnikov via Tarantool-patches
  2022-06-22 15:33 ` [Tarantool-patches] [PATCH luajit 3/3] ci: add Tarantool integration testing Igor Munkin via Tarantool-patches
  2022-07-13 10:09 ` [Tarantool-patches] [PATCH luajit 0/3] Integration testing and tiny fixes in CI Igor Munkin via Tarantool-patches
  3 siblings, 2 replies; 21+ messages in thread
From: Igor Munkin via Tarantool-patches @ 2022-06-22 15:33 UTC (permalink / raw)
  To: Sergey Kaplun, Sergey Bronnikov; +Cc: tarantool-patches

CMake option LUAJIT_ENABLE_GC64 changes nothing for ARM64 LuaJIT builds
(i.e. LJ_GC64 is defined on ARM64 platforms despite the value set to
LUAJIT_ENABLE_GC64). As a result GC64 entry is removed from both Linux
and MacOS ARM64 workflows to reduce cloud resources costs and suspend
global warming.

Signed-off-by: Igor Munkin <imun@tarantool.org>
---
 .github/workflows/linux-aarch64.yml | 5 ++---
 .github/workflows/macos-m1.yml      | 5 ++---
 2 files changed, 4 insertions(+), 6 deletions(-)

diff --git a/.github/workflows/linux-aarch64.yml b/.github/workflows/linux-aarch64.yml
index 19412b3f..de360b12 100644
--- a/.github/workflows/linux-aarch64.yml
+++ b/.github/workflows/linux-aarch64.yml
@@ -35,13 +35,12 @@ jobs:
       fail-fast: false
       matrix:
         BUILDTYPE: [Debug, Release]
-        GC64: [ON, OFF]
         include:
           - BUILDTYPE: Debug
             CMAKEFLAGS: -DCMAKE_BUILD_TYPE=Debug -DLUA_USE_ASSERT=ON -DLUA_USE_APICHECK=ON
           - BUILDTYPE: Release
             CMAKEFLAGS: -DCMAKE_BUILD_TYPE=RelWithDebInfo
-    name: Linux/aarch64 ${{ matrix.BUILDTYPE }} GC64:${{ matrix.GC64 }}
+    name: Linux/aarch64 ${{ matrix.BUILDTYPE }} GC64:ON
     steps:
       - uses: actions/checkout@v2.3.4
         with:
@@ -52,7 +51,7 @@ jobs:
           sudo apt -y update
           sudo apt -y install cmake gcc make perl
       - name: configure
-        run: cmake . ${{ matrix.CMAKEFLAGS }} -DLUAJIT_ENABLE_GC64=${{ matrix.GC64 }}
+        run: cmake . ${{ matrix.CMAKEFLAGS }}
       - name: build
         run: cmake --build . --parallel $(($(nproc) + 1))
       - name: test
diff --git a/.github/workflows/macos-m1.yml b/.github/workflows/macos-m1.yml
index d1d144f0..a38f70f7 100644
--- a/.github/workflows/macos-m1.yml
+++ b/.github/workflows/macos-m1.yml
@@ -40,13 +40,12 @@ jobs:
       fail-fast: false
       matrix:
         BUILDTYPE: [Debug, Release]
-        GC64: [ON, OFF]
         include:
           - BUILDTYPE: Debug
             CMAKEFLAGS: -DCMAKE_BUILD_TYPE=Debug -DLUA_USE_ASSERT=ON -DLUA_USE_APICHECK=ON
           - BUILDTYPE: Release
             CMAKEFLAGS: -DCMAKE_BUILD_TYPE=RelWithDebInfo
-    name: macOS/m1 ${{ matrix.BUILDTYPE }} GC64:${{ matrix.GC64 }}
+    name: macOS/m1 ${{ matrix.BUILDTYPE }} GC64:ON
     steps:
       - uses: actions/checkout@v2.3.4
         with:
@@ -66,7 +65,7 @@ jobs:
           ${ARCH} brew install --force cmake gcc make perl ||
             ${ARCH} brew upgrade cmake gcc make perl
       - name: configure
-        run: ${ARCH} cmake . ${{ matrix.CMAKEFLAGS }} -DLUAJIT_ENABLE_GC64=${{ matrix.GC64 }}
+        run: ${ARCH} cmake . ${{ matrix.CMAKEFLAGS }}
       - name: build
         run: ${ARCH} cmake --build . --parallel $(($(sysctl -n hw.ncpu) + 1))
       - name: test
-- 
2.34.0


^ permalink raw reply	[flat|nested] 21+ messages in thread

* [Tarantool-patches] [PATCH luajit 3/3] ci: add Tarantool integration testing
  2022-06-22 15:33 [Tarantool-patches] [PATCH luajit 0/3] Integration testing and tiny fixes in CI Igor Munkin via Tarantool-patches
  2022-06-22 15:33 ` [Tarantool-patches] [PATCH luajit 1/3] ci: fix --parallel argument for MacOS runners Igor Munkin via Tarantool-patches
  2022-06-22 15:33 ` [Tarantool-patches] [PATCH luajit 2/3] ci: remove GC64 matrix entries for ARM64 workflows Igor Munkin via Tarantool-patches
@ 2022-06-22 15:33 ` Igor Munkin via Tarantool-patches
  2022-06-23  8:45   ` Sergey Kaplun via Tarantool-patches
  2022-07-01 13:39   ` Sergey Bronnikov via Tarantool-patches
  2022-07-13 10:09 ` [Tarantool-patches] [PATCH luajit 0/3] Integration testing and tiny fixes in CI Igor Munkin via Tarantool-patches
  3 siblings, 2 replies; 21+ messages in thread
From: Igor Munkin via Tarantool-patches @ 2022-06-22 15:33 UTC (permalink / raw)
  To: Sergey Kaplun, Sergey Bronnikov; +Cc: tarantool-patches

This patch introduces additional steps with Tarantool integration
testing to all existing LuaJIT workflows. All steps invoke reusable
workflow[1] from Tarantool repository for each matrix entry being set
for particular LuaJIT workflow.

[1]: https://github.com/tarantool/tarantool/blob/master/.github/workflows/luajit-integration.yml

Signed-off-by: Igor Munkin <imun@tarantool.org>
---
 .github/workflows/linux-aarch64.yml | 25 +++++++++++++++--
 .github/workflows/linux-x86_64.yml  | 43 +++++++++++++++++++++++++++--
 .github/workflows/macos-m1.yml      | 25 +++++++++++++++--
 .github/workflows/macos-x86_64.yml  | 43 +++++++++++++++++++++++++++--
 4 files changed, 124 insertions(+), 12 deletions(-)

diff --git a/.github/workflows/linux-aarch64.yml b/.github/workflows/linux-aarch64.yml
index de360b12..293b572e 100644
--- a/.github/workflows/linux-aarch64.yml
+++ b/.github/workflows/linux-aarch64.yml
@@ -1,4 +1,4 @@
-name: "LuaJIT test workflow (Linux/aarch64)"
+name: "Linux/aarch64 test workflow"
 
 on:
   push:
@@ -29,7 +29,7 @@ concurrency:
   cancel-in-progress: true
 
 jobs:
-  test-linux-aarch64:
+  test-luajit:
     runs-on: graviton
     strategy:
       fail-fast: false
@@ -40,7 +40,7 @@ jobs:
             CMAKEFLAGS: -DCMAKE_BUILD_TYPE=Debug -DLUA_USE_ASSERT=ON -DLUA_USE_APICHECK=ON
           - BUILDTYPE: Release
             CMAKEFLAGS: -DCMAKE_BUILD_TYPE=RelWithDebInfo
-    name: Linux/aarch64 ${{ matrix.BUILDTYPE }} GC64:ON
+    name: LuaJIT ${{ matrix.BUILDTYPE }} GC64:ON
     steps:
       - uses: actions/checkout@v2.3.4
         with:
@@ -56,3 +56,22 @@ jobs:
         run: cmake --build . --parallel $(($(nproc) + 1))
       - name: test
         run: cmake --build . --parallel $(($(nproc) + 1)) --target test
+
+  test-tarantool-debug-w-GC64:
+    name: Tarantool Debug GC64:ON
+    needs: test-luajit
+    uses: tarantool/tarantool/.github/workflows/luajit-integration.yml@master
+    with:
+      GC64: ON
+      buildtype: Debug
+      host: graviton
+      revision: ${{ github.sha }}
+  test-tarantool-release-w-GC64:
+    name: Tarantool Release GC64:ON
+    needs: test-luajit
+    uses: tarantool/tarantool/.github/workflows/luajit-integration.yml@master
+    with:
+      GC64: ON
+      buildtype: RelWithDebInfo
+      host: graviton
+      revision: ${{ github.sha }}
diff --git a/.github/workflows/linux-x86_64.yml b/.github/workflows/linux-x86_64.yml
index d6434afe..3eaf436d 100644
--- a/.github/workflows/linux-x86_64.yml
+++ b/.github/workflows/linux-x86_64.yml
@@ -1,4 +1,4 @@
-name: "LuaJIT test workflow (Linux/x86_64)"
+name: "Linux/x86_64 test workflow"
 
 on:
   push:
@@ -29,7 +29,7 @@ concurrency:
   cancel-in-progress: true
 
 jobs:
-  test-linux-x86_64:
+  test-luajit:
     runs-on: ubuntu-20.04-self-hosted
     strategy:
       fail-fast: false
@@ -41,7 +41,7 @@ jobs:
             CMAKEFLAGS: -DCMAKE_BUILD_TYPE=Debug -DLUA_USE_ASSERT=ON -DLUA_USE_APICHECK=ON
           - BUILDTYPE: Release
             CMAKEFLAGS: -DCMAKE_BUILD_TYPE=RelWithDebInfo
-    name: Linux/x86_64 ${{ matrix.BUILDTYPE }} GC64:${{ matrix.GC64 }}
+    name: LuaJIT ${{ matrix.BUILDTYPE }} GC64:${{ matrix.GC64 }}
     steps:
       - uses: actions/checkout@v2.3.4
         with:
@@ -57,3 +57,40 @@ jobs:
         run: cmake --build . --parallel $(($(nproc) + 1))
       - name: test
         run: cmake --build . --parallel $(($(nproc) + 1)) --target test
+
+  test-tarantool-debug-wo-GC64:
+    name: Tarantool Debug GC64:OFF
+    needs: test-luajit
+    uses: tarantool/tarantool/.github/workflows/luajit-integration.yml@master
+    with:
+      GC64: OFF
+      buildtype: Debug
+      host: ubuntu-20.04-self-hosted
+      revision: ${{ github.sha }}
+  test-tarantool-debug-w-GC64:
+    name: Tarantool Debug GC64:ON
+    needs: test-luajit
+    uses: tarantool/tarantool/.github/workflows/luajit-integration.yml@master
+    with:
+      GC64: ON
+      buildtype: Debug
+      host: ubuntu-20.04-self-hosted
+      revision: ${{ github.sha }}
+  test-tarantool-release-wo-GC64:
+    name: Tarantool Release GC64:OFF
+    needs: test-luajit
+    uses: tarantool/tarantool/.github/workflows/luajit-integration.yml@master
+    with:
+      GC64: OFF
+      buildtype: RelWithDebInfo
+      host: ubuntu-20.04-self-hosted
+      revision: ${{ github.sha }}
+  test-tarantool-release-w-GC64:
+    name: Tarantool Release GC64:ON
+    needs: test-luajit
+    uses: tarantool/tarantool/.github/workflows/luajit-integration.yml@master
+    with:
+      GC64: ON
+      buildtype: RelWithDebInfo
+      host: ubuntu-20.04-self-hosted
+      revision: ${{ github.sha }}
diff --git a/.github/workflows/macos-m1.yml b/.github/workflows/macos-m1.yml
index a38f70f7..d305e609 100644
--- a/.github/workflows/macos-m1.yml
+++ b/.github/workflows/macos-m1.yml
@@ -1,4 +1,4 @@
-name: "LuaJIT test workflow (macOS/m1)"
+name: "macOS/m1 test workflow"
 
 on:
   push:
@@ -34,7 +34,7 @@ env:
   ARCH: arch -arm64
 
 jobs:
-  test-macos-m1:
+  test-luajit:
     runs-on: macos-11-m1
     strategy:
       fail-fast: false
@@ -45,7 +45,7 @@ jobs:
             CMAKEFLAGS: -DCMAKE_BUILD_TYPE=Debug -DLUA_USE_ASSERT=ON -DLUA_USE_APICHECK=ON
           - BUILDTYPE: Release
             CMAKEFLAGS: -DCMAKE_BUILD_TYPE=RelWithDebInfo
-    name: macOS/m1 ${{ matrix.BUILDTYPE }} GC64:ON
+    name: LuaJIT ${{ matrix.BUILDTYPE }} GC64:ON
     steps:
       - uses: actions/checkout@v2.3.4
         with:
@@ -70,3 +70,22 @@ jobs:
         run: ${ARCH} cmake --build . --parallel $(($(sysctl -n hw.ncpu) + 1))
       - name: test
         run: ${ARCH} cmake --build . --parallel $(($(sysctl -n hw.ncpu) + 1)) --target test
+
+  test-tarantool-debug-w-GC64:
+    name: Tarantool Debug GC64:ON
+    needs: test-luajit
+    uses: tarantool/tarantool/.github/workflows/luajit-integration.yml@master
+    with:
+      GC64: ON
+      buildtype: Debug
+      host: macos-11-m1
+      revision: ${{ github.sha }}
+  test-tarantool-release-w-GC64:
+    name: Tarantool Release GC64:ON
+    needs: test-luajit
+    uses: tarantool/tarantool/.github/workflows/luajit-integration.yml@master
+    with:
+      GC64: ON
+      buildtype: RelWithDebInfo
+      host: macos-11-m1
+      revision: ${{ github.sha }}
diff --git a/.github/workflows/macos-x86_64.yml b/.github/workflows/macos-x86_64.yml
index a7c2f4e3..fc28c683 100644
--- a/.github/workflows/macos-x86_64.yml
+++ b/.github/workflows/macos-x86_64.yml
@@ -1,4 +1,4 @@
-name: "LuaJIT test workflow (macOS/x86_64)"
+name: "macOS/x86_64 test workflow"
 
 on:
   push:
@@ -29,7 +29,7 @@ concurrency:
   cancel-in-progress: true
 
 jobs:
-  test-macos-x86_64:
+  test-luajit:
     runs-on: macos-11
     strategy:
       fail-fast: false
@@ -41,7 +41,7 @@ jobs:
             CMAKEFLAGS: -DCMAKE_BUILD_TYPE=Debug -DLUA_USE_ASSERT=ON -DLUA_USE_APICHECK=ON
           - BUILDTYPE: Release
             CMAKEFLAGS: -DCMAKE_BUILD_TYPE=RelWithDebInfo
-    name: macOS/x86_64 ${{ matrix.BUILDTYPE }} GC64:${{ matrix.GC64 }}
+    name: LuaJIT ${{ matrix.BUILDTYPE }} GC64:${{ matrix.GC64 }}
     steps:
       - uses: actions/checkout@v2.3.4
         with:
@@ -66,3 +66,40 @@ jobs:
         run: cmake --build . --parallel $(($(sysctl -n hw.ncpu) + 1))
       - name: test
         run: cmake --build . --parallel $(($(sysctl -n hw.ncpu) + 1)) --target test
+
+  test-tarantool-debug-wo-GC64:
+    name: Tarantool Debug GC64:OFF
+    needs: test-luajit
+    uses: tarantool/tarantool/.github/workflows/luajit-integration.yml@master
+    with:
+      GC64: OFF
+      buildtype: Debug
+      host: macos-11
+      revision: ${{ github.sha }}
+  test-tarantool-debug-w-GC64:
+    name: Tarantool Debug GC64:ON
+    needs: test-luajit
+    uses: tarantool/tarantool/.github/workflows/luajit-integration.yml@master
+    with:
+      GC64: ON
+      buildtype: Debug
+      host: macos-11
+      revision: ${{ github.sha }}
+  test-tarantool-release-wo-GC64:
+    name: Tarantool Release GC64:OFF
+    needs: test-luajit
+    uses: tarantool/tarantool/.github/workflows/luajit-integration.yml@master
+    with:
+      GC64: OFF
+      buildtype: RelWithDebInfo
+      host: macos-11
+      revision: ${{ github.sha }}
+  test-tarantool-release-w-GC64:
+    name: Tarantool Release GC64:ON
+    needs: test-luajit
+    uses: tarantool/tarantool/.github/workflows/luajit-integration.yml@master
+    with:
+      GC64: ON
+      buildtype: RelWithDebInfo
+      host: macos-11
+      revision: ${{ github.sha }}
-- 
2.34.0


^ permalink raw reply	[flat|nested] 21+ messages in thread

* Re: [Tarantool-patches] [PATCH luajit 1/3] ci: fix --parallel argument for MacOS runners
  2022-06-22 15:33 ` [Tarantool-patches] [PATCH luajit 1/3] ci: fix --parallel argument for MacOS runners Igor Munkin via Tarantool-patches
@ 2022-06-23  8:19   ` Sergey Kaplun via Tarantool-patches
  2022-06-29 22:17     ` Igor Munkin via Tarantool-patches
  2022-07-01 13:12   ` Sergey Bronnikov via Tarantool-patches
  2022-07-05 17:20   ` Sergey Bronnikov via Tarantool-patches
  2 siblings, 1 reply; 21+ messages in thread
From: Sergey Kaplun via Tarantool-patches @ 2022-06-23  8:19 UTC (permalink / raw)
  To: Igor Munkin; +Cc: tarantool-patches

Hi, Igor!

Thanks for the patch!

LGTM, except a few nits regarding the commit message.

On 22.06.22, Igor Munkin wrote:
> In scope of the commit 4195eb8f7e2abcf75f92eedd2859b7991cc8b363 ("ci:

Typo: s/In scope/In the scope/

> make GitHub workflows more CMake-ish") --parallel value has been
> explicitly set to $(nproc) + 1, since -j default behaviour differs for
> Ninja and GnuMake. However, MacOS lacks nproc command, but CI silently
> continues to execute pipeline.

Typo: s/pipeline/the pipeline/

> 
> This patch fixed MacOS-specific workflows changing nproc command with

Nit: s/fixed/fixes/ as far as we prefer Present Simple for commits
description.
Feel free to ignore.

> sysctl -n hw.ncpu.
> 
> Signed-off-by: Igor Munkin <imun@tarantool.org>
> ---

<snipped>

> -- 
> 2.34.0
> 

-- 
Best regards,
Sergey Kaplun

^ permalink raw reply	[flat|nested] 21+ messages in thread

* Re: [Tarantool-patches] [PATCH luajit 2/3] ci: remove GC64 matrix entries for ARM64 workflows
  2022-06-22 15:33 ` [Tarantool-patches] [PATCH luajit 2/3] ci: remove GC64 matrix entries for ARM64 workflows Igor Munkin via Tarantool-patches
@ 2022-06-23  8:29   ` Sergey Kaplun via Tarantool-patches
  2022-06-29 22:17     ` Igor Munkin via Tarantool-patches
  2022-07-01 13:21   ` Sergey Bronnikov via Tarantool-patches
  1 sibling, 1 reply; 21+ messages in thread
From: Sergey Kaplun via Tarantool-patches @ 2022-06-23  8:29 UTC (permalink / raw)
  To: Igor Munkin; +Cc: tarantool-patches

Hi, Igor!

Thanks for the patch!
LGTM!

-- 
Best regards,
Sergey Kaplun

^ permalink raw reply	[flat|nested] 21+ messages in thread

* Re: [Tarantool-patches] [PATCH luajit 3/3] ci: add Tarantool integration testing
  2022-06-22 15:33 ` [Tarantool-patches] [PATCH luajit 3/3] ci: add Tarantool integration testing Igor Munkin via Tarantool-patches
@ 2022-06-23  8:45   ` Sergey Kaplun via Tarantool-patches
  2022-06-29 22:17     ` Igor Munkin via Tarantool-patches
  2022-07-01 13:39   ` Sergey Bronnikov via Tarantool-patches
  1 sibling, 1 reply; 21+ messages in thread
From: Sergey Kaplun via Tarantool-patches @ 2022-06-23  8:45 UTC (permalink / raw)
  To: Igor Munkin; +Cc: tarantool-patches

Hi, Igor!

Thanks for the patch!
Nice to see our CI improving!

Please consider my comments below.

On 22.06.22, Igor Munkin wrote:
> This patch introduces additional steps with Tarantool integration
> testing to all existing LuaJIT workflows. All steps invoke reusable
> workflow[1] from Tarantool repository for each matrix entry being set
> for particular LuaJIT workflow.

AFAICS, there are no workflows for different OS like Fedora, FreeBSD,
etc.. Same for integration testing workflow for Tarantool itself, plus
workflows with different compilers (gcc/clang).

Should developer still bump LuaJIT in Tarantool repo and create PR to
test full Tarantool CI?

> 
> [1]: https://github.com/tarantool/tarantool/blob/master/.github/workflows/luajit-integration.yml
> 
> Signed-off-by: Igor Munkin <imun@tarantool.org>
> ---
>  .github/workflows/linux-aarch64.yml | 25 +++++++++++++++--
>  .github/workflows/linux-x86_64.yml  | 43 +++++++++++++++++++++++++++--
>  .github/workflows/macos-m1.yml      | 25 +++++++++++++++--
>  .github/workflows/macos-x86_64.yml  | 43 +++++++++++++++++++++++++++--
>  4 files changed, 124 insertions(+), 12 deletions(-)
> 
> diff --git a/.github/workflows/linux-aarch64.yml b/.github/workflows/linux-aarch64.yml
> index de360b12..293b572e 100644
> --- a/.github/workflows/linux-aarch64.yml
> +++ b/.github/workflows/linux-aarch64.yml
> @@ -1,4 +1,4 @@
> -name: "LuaJIT test workflow (Linux/aarch64)"
> +name: "Linux/aarch64 test workflow"
>  
>  on:
>    push:
> @@ -29,7 +29,7 @@ concurrency:
>    cancel-in-progress: true
>  
>  jobs:
> -  test-linux-aarch64:
> +  test-luajit:
>      runs-on: graviton
>      strategy:
>        fail-fast: false
> @@ -40,7 +40,7 @@ jobs:
>              CMAKEFLAGS: -DCMAKE_BUILD_TYPE=Debug -DLUA_USE_ASSERT=ON -DLUA_USE_APICHECK=ON
>            - BUILDTYPE: Release
>              CMAKEFLAGS: -DCMAKE_BUILD_TYPE=RelWithDebInfo
> -    name: Linux/aarch64 ${{ matrix.BUILDTYPE }} GC64:ON
> +    name: LuaJIT ${{ matrix.BUILDTYPE }} GC64:ON
>      steps:
>        - uses: actions/checkout@v2.3.4
>          with:
> @@ -56,3 +56,22 @@ jobs:
>          run: cmake --build . --parallel $(($(nproc) + 1))
>        - name: test
>          run: cmake --build . --parallel $(($(nproc) + 1)) --target test
> +
> +  test-tarantool-debug-w-GC64:
> +    name: Tarantool Debug GC64:ON
> +    needs: test-luajit
> +    uses: tarantool/tarantool/.github/workflows/luajit-integration.yml@master

I suggest to set one variable for this to reduce copy-paste.
Also, why do you prefer copy workflows instead matrix usage?
The only thing changes here -- buildtype.

> +    with:
> +      GC64: ON
> +      buildtype: Debug
> +      host: graviton
> +      revision: ${{ github.sha }}
> +  test-tarantool-release-w-GC64:
> +    name: Tarantool Release GC64:ON
> +    needs: test-luajit
> +    uses: tarantool/tarantool/.github/workflows/luajit-integration.yml@master
> +    with:
> +      GC64: ON
> +      buildtype: RelWithDebInfo
> +      host: graviton
> +      revision: ${{ github.sha }}
> diff --git a/.github/workflows/linux-x86_64.yml b/.github/workflows/linux-x86_64.yml
> index d6434afe..3eaf436d 100644
> --- a/.github/workflows/linux-x86_64.yml
> +++ b/.github/workflows/linux-x86_64.yml
> @@ -1,4 +1,4 @@
> -name: "LuaJIT test workflow (Linux/x86_64)"
> +name: "Linux/x86_64 test workflow"
>  
>  on:
>    push:
> @@ -29,7 +29,7 @@ concurrency:
>    cancel-in-progress: true
>  
>  jobs:
> -  test-linux-x86_64:
> +  test-luajit:
>      runs-on: ubuntu-20.04-self-hosted
>      strategy:
>        fail-fast: false
> @@ -41,7 +41,7 @@ jobs:
>              CMAKEFLAGS: -DCMAKE_BUILD_TYPE=Debug -DLUA_USE_ASSERT=ON -DLUA_USE_APICHECK=ON
>            - BUILDTYPE: Release
>              CMAKEFLAGS: -DCMAKE_BUILD_TYPE=RelWithDebInfo
> -    name: Linux/x86_64 ${{ matrix.BUILDTYPE }} GC64:${{ matrix.GC64 }}
> +    name: LuaJIT ${{ matrix.BUILDTYPE }} GC64:${{ matrix.GC64 }}
>      steps:
>        - uses: actions/checkout@v2.3.4
>          with:
> @@ -57,3 +57,40 @@ jobs:
>          run: cmake --build . --parallel $(($(nproc) + 1))
>        - name: test
>          run: cmake --build . --parallel $(($(nproc) + 1)) --target test
> +
> +  test-tarantool-debug-wo-GC64:
> +    name: Tarantool Debug GC64:OFF
> +    needs: test-luajit
> +    uses: tarantool/tarantool/.github/workflows/luajit-integration.yml@master
> +    with:
> +      GC64: OFF

Same question here, but also with GC64 mode.

> +      buildtype: Debug
> +      host: ubuntu-20.04-self-hosted
> +      revision: ${{ github.sha }}
> +  test-tarantool-debug-w-GC64:
> +    name: Tarantool Debug GC64:ON
> +    needs: test-luajit
> +    uses: tarantool/tarantool/.github/workflows/luajit-integration.yml@master
> +    with:
> +      GC64: ON
> +      buildtype: Debug
> +      host: ubuntu-20.04-self-hosted
> +      revision: ${{ github.sha }}
> +  test-tarantool-release-wo-GC64:
> +    name: Tarantool Release GC64:OFF
> +    needs: test-luajit
> +    uses: tarantool/tarantool/.github/workflows/luajit-integration.yml@master
> +    with:
> +      GC64: OFF
> +      buildtype: RelWithDebInfo
> +      host: ubuntu-20.04-self-hosted
> +      revision: ${{ github.sha }}
> +  test-tarantool-release-w-GC64:
> +    name: Tarantool Release GC64:ON
> +    needs: test-luajit
> +    uses: tarantool/tarantool/.github/workflows/luajit-integration.yml@master
> +    with:
> +      GC64: ON
> +      buildtype: RelWithDebInfo
> +      host: ubuntu-20.04-self-hosted
> +      revision: ${{ github.sha }}
> diff --git a/.github/workflows/macos-m1.yml b/.github/workflows/macos-m1.yml
> index a38f70f7..d305e609 100644
> --- a/.github/workflows/macos-m1.yml
> +++ b/.github/workflows/macos-m1.yml

<snipped> as far as it is kinda the same.

> diff --git a/.github/workflows/macos-x86_64.yml b/.github/workflows/macos-x86_64.yml
> index a7c2f4e3..fc28c683 100644
> --- a/.github/workflows/macos-x86_64.yml
> +++ b/.github/workflows/macos-x86_64.yml

<snipped> as far as it is kinda the same.

> -- 
> 2.34.0
> 

-- 
Best regards,
Sergey Kaplun

^ permalink raw reply	[flat|nested] 21+ messages in thread

* Re: [Tarantool-patches] [PATCH luajit 1/3] ci: fix --parallel argument for MacOS runners
  2022-06-23  8:19   ` Sergey Kaplun via Tarantool-patches
@ 2022-06-29 22:17     ` Igor Munkin via Tarantool-patches
  0 siblings, 0 replies; 21+ messages in thread
From: Igor Munkin via Tarantool-patches @ 2022-06-29 22:17 UTC (permalink / raw)
  To: Sergey Kaplun; +Cc: tarantool-patches

Sergey,

Thanks for your review! See my comments inline and the changes on the
remote branch[1].

On 23.06.22, Sergey Kaplun wrote:
> Hi, Igor!
> 
> Thanks for the patch!
> 
> LGTM, except a few nits regarding the commit message.

Added your tag:
| Reviewed-by: Sergey Kaplun <skaplun@tarantool.org>

> 
> On 22.06.22, Igor Munkin wrote:
> > In scope of the commit 4195eb8f7e2abcf75f92eedd2859b7991cc8b363 ("ci:
> 
> Typo: s/In scope/In the scope/

I strongly doubt. Ignoring.

> 
> > make GitHub workflows more CMake-ish") --parallel value has been
> > explicitly set to $(nproc) + 1, since -j default behaviour differs for
> > Ninja and GnuMake. However, MacOS lacks nproc command, but CI silently
> > continues to execute pipeline.
> 
> Typo: s/pipeline/the pipeline/

Fixed.

> 
> > 
> > This patch fixed MacOS-specific workflows changing nproc command with
> 
> Nit: s/fixed/fixes/ as far as we prefer Present Simple for commits
> description.
> Feel free to ignore.

You're right, thanks! Fixed.

> 
> > sysctl -n hw.ncpu.
> > 
> > Signed-off-by: Igor Munkin <imun@tarantool.org>
> > ---
> 
> <snipped>
> 
> > -- 
> > 2.34.0
> > 
> 
> -- 
> Best regards,
> Sergey Kaplun

[1]: https://github.com/tarantool/luajit/commit/fe2df4c

-- 
Best regards,
IM

^ permalink raw reply	[flat|nested] 21+ messages in thread

* Re: [Tarantool-patches] [PATCH luajit 2/3] ci: remove GC64 matrix entries for ARM64 workflows
  2022-06-23  8:29   ` Sergey Kaplun via Tarantool-patches
@ 2022-06-29 22:17     ` Igor Munkin via Tarantool-patches
  0 siblings, 0 replies; 21+ messages in thread
From: Igor Munkin via Tarantool-patches @ 2022-06-29 22:17 UTC (permalink / raw)
  To: Sergey Kaplun; +Cc: tarantool-patches

Sergey,

Thanks for your review!

On 23.06.22, Sergey Kaplun wrote:
> Hi, Igor!
> 
> Thanks for the patch!
> LGTM!

Added your tag:
| Reviewed-by: Sergey Kaplun <skaplun@tarantool.org>

> 
> -- 
> Best regards,
> Sergey Kaplun

-- 
Best regards,
IM

^ permalink raw reply	[flat|nested] 21+ messages in thread

* Re: [Tarantool-patches] [PATCH luajit 3/3] ci: add Tarantool integration testing
  2022-06-23  8:45   ` Sergey Kaplun via Tarantool-patches
@ 2022-06-29 22:17     ` Igor Munkin via Tarantool-patches
  2022-07-04 10:24       ` Sergey Kaplun via Tarantool-patches
  0 siblings, 1 reply; 21+ messages in thread
From: Igor Munkin via Tarantool-patches @ 2022-06-29 22:17 UTC (permalink / raw)
  To: Sergey Kaplun; +Cc: tarantool-patches

Sergey,

Thanks for your review!

On 23.06.22, Sergey Kaplun wrote:
> Hi, Igor!
> 
> Thanks for the patch!
> Nice to see our CI improving!

Glad this makes somebody a little happier :)

> 
> Please consider my comments below.
> 
> On 22.06.22, Igor Munkin wrote:
> > This patch introduces additional steps with Tarantool integration
> > testing to all existing LuaJIT workflows. All steps invoke reusable
> > workflow[1] from Tarantool repository for each matrix entry being set
> > for particular LuaJIT workflow.
> 
> AFAICS, there are no workflows for different OS like Fedora, FreeBSD,
> etc.. Same for integration testing workflow for Tarantool itself, plus
> workflows with different compilers (gcc/clang).
> 
> Should developer still bump LuaJIT in Tarantool repo and create PR to
> test full Tarantool CI?

Well, I'm trying my best to make tarantool/luajit CI as close to the one
in tarantool/tarantool as it's possible. However, I'm afraid we will
face two challenges soon:
1. As far as we're going to add some custom LuaJIT configurations
   testing and instrumenting testing to LuaJIT CI, it will expand
   already extensive Tarantool testing.
2. tarantool/tarantool is the main repository and we share runners with
   it, so we should not forget about already overloaded infrastructure.

All in all we need to create the sufficient set of workflows that will
alleviate LuaJIT developer suffering without dropping patches quality.

If you're asking about this particular patch, then yes, you still need
to create a draft PR in tarantool/tarantool to check if everything is
fine. By the way, I hear many opinions whether developers should use
'full-ci' tag for PRs or not. If you don't use it, you can just run all
pipelines in tarantool/luajit and leave everything else to a mainainer.
Otherwise, there is no other option than draft PR (I believe you can
create a single PR in tarantool/tarantool for a bunch of your patches,
but I'm not a big fan of it).

Anyway, here is the list for the upcoming enhancements:
* FreeBSD workflow
* Various compilers matrix (GCC, Clang)
* JIT enabled/disabled matrix * Valgrind testing
* luarocks v3 world testing

> 
> > 
> > [1]: https://github.com/tarantool/tarantool/blob/master/.github/workflows/luajit-integration.yml
> > 
> > Signed-off-by: Igor Munkin <imun@tarantool.org>
> > ---
> >  .github/workflows/linux-aarch64.yml | 25 +++++++++++++++--
> >  .github/workflows/linux-x86_64.yml  | 43 +++++++++++++++++++++++++++--
> >  .github/workflows/macos-m1.yml      | 25 +++++++++++++++--
> >  .github/workflows/macos-x86_64.yml  | 43 +++++++++++++++++++++++++++--
> >  4 files changed, 124 insertions(+), 12 deletions(-)
> > 
> > diff --git a/.github/workflows/linux-aarch64.yml b/.github/workflows/linux-aarch64.yml
> > index de360b12..293b572e 100644
> > --- a/.github/workflows/linux-aarch64.yml
> > +++ b/.github/workflows/linux-aarch64.yml
> > @@ -1,4 +1,4 @@
> > -name: "LuaJIT test workflow (Linux/aarch64)"
> > +name: "Linux/aarch64 test workflow"
> >  
> >  on:
> >    push:
> > @@ -29,7 +29,7 @@ concurrency:
> >    cancel-in-progress: true
> >  
> >  jobs:
> > -  test-linux-aarch64:
> > +  test-luajit:
> >      runs-on: graviton
> >      strategy:
> >        fail-fast: false
> > @@ -40,7 +40,7 @@ jobs:
> >              CMAKEFLAGS: -DCMAKE_BUILD_TYPE=Debug -DLUA_USE_ASSERT=ON -DLUA_USE_APICHECK=ON
> >            - BUILDTYPE: Release
> >              CMAKEFLAGS: -DCMAKE_BUILD_TYPE=RelWithDebInfo
> > -    name: Linux/aarch64 ${{ matrix.BUILDTYPE }} GC64:ON
> > +    name: LuaJIT ${{ matrix.BUILDTYPE }} GC64:ON
> >      steps:
> >        - uses: actions/checkout@v2.3.4
> >          with:
> > @@ -56,3 +56,22 @@ jobs:
> >          run: cmake --build . --parallel $(($(nproc) + 1))
> >        - name: test
> >          run: cmake --build . --parallel $(($(nproc) + 1)) --target test
> > +
> > +  test-tarantool-debug-w-GC64:
> > +    name: Tarantool Debug GC64:ON
> > +    needs: test-luajit
> > +    uses: tarantool/tarantool/.github/workflows/luajit-integration.yml@master
> 
> I suggest to set one variable for this to reduce copy-paste.
> Also, why do you prefer copy workflows instead matrix usage?
> The only thing changes here -- buildtype.

Unfortunately, reusable workflow can't use test matrices, so we have no
other option than copy-paste this several times. I might be doing
something wrong, but I've failed with this exercise. You can try by
yourself, but now you're warned.

> 
> > +    with:
> > +      GC64: ON
> > +      buildtype: Debug
> > +      host: graviton
> > +      revision: ${{ github.sha }}
> > +  test-tarantool-release-w-GC64:
> > +    name: Tarantool Release GC64:ON
> > +    needs: test-luajit
> > +    uses: tarantool/tarantool/.github/workflows/luajit-integration.yml@master
> > +    with:
> > +      GC64: ON
> > +      buildtype: RelWithDebInfo
> > +      host: graviton
> > +      revision: ${{ github.sha }}
> > diff --git a/.github/workflows/linux-x86_64.yml b/.github/workflows/linux-x86_64.yml
> > index d6434afe..3eaf436d 100644
> > --- a/.github/workflows/linux-x86_64.yml
> > +++ b/.github/workflows/linux-x86_64.yml
> > @@ -1,4 +1,4 @@
> > -name: "LuaJIT test workflow (Linux/x86_64)"
> > +name: "Linux/x86_64 test workflow"
> >  
> >  on:
> >    push:
> > @@ -29,7 +29,7 @@ concurrency:
> >    cancel-in-progress: true
> >  
> >  jobs:
> > -  test-linux-x86_64:
> > +  test-luajit:
> >      runs-on: ubuntu-20.04-self-hosted
> >      strategy:
> >        fail-fast: false
> > @@ -41,7 +41,7 @@ jobs:
> >              CMAKEFLAGS: -DCMAKE_BUILD_TYPE=Debug -DLUA_USE_ASSERT=ON -DLUA_USE_APICHECK=ON
> >            - BUILDTYPE: Release
> >              CMAKEFLAGS: -DCMAKE_BUILD_TYPE=RelWithDebInfo
> > -    name: Linux/x86_64 ${{ matrix.BUILDTYPE }} GC64:${{ matrix.GC64 }}
> > +    name: LuaJIT ${{ matrix.BUILDTYPE }} GC64:${{ matrix.GC64 }}
> >      steps:
> >        - uses: actions/checkout@v2.3.4
> >          with:
> > @@ -57,3 +57,40 @@ jobs:
> >          run: cmake --build . --parallel $(($(nproc) + 1))
> >        - name: test
> >          run: cmake --build . --parallel $(($(nproc) + 1)) --target test
> > +
> > +  test-tarantool-debug-wo-GC64:
> > +    name: Tarantool Debug GC64:OFF
> > +    needs: test-luajit
> > +    uses: tarantool/tarantool/.github/workflows/luajit-integration.yml@master
> > +    with:
> > +      GC64: OFF
> 
> Same question here, but also with GC64 mode.

Ditto.

> 
> > +      buildtype: Debug
> > +      host: ubuntu-20.04-self-hosted
> > +      revision: ${{ github.sha }}
> > +  test-tarantool-debug-w-GC64:
> > +    name: Tarantool Debug GC64:ON
> > +    needs: test-luajit
> > +    uses: tarantool/tarantool/.github/workflows/luajit-integration.yml@master
> > +    with:
> > +      GC64: ON
> > +      buildtype: Debug
> > +      host: ubuntu-20.04-self-hosted
> > +      revision: ${{ github.sha }}
> > +  test-tarantool-release-wo-GC64:
> > +    name: Tarantool Release GC64:OFF
> > +    needs: test-luajit
> > +    uses: tarantool/tarantool/.github/workflows/luajit-integration.yml@master
> > +    with:
> > +      GC64: OFF
> > +      buildtype: RelWithDebInfo
> > +      host: ubuntu-20.04-self-hosted
> > +      revision: ${{ github.sha }}
> > +  test-tarantool-release-w-GC64:
> > +    name: Tarantool Release GC64:ON
> > +    needs: test-luajit
> > +    uses: tarantool/tarantool/.github/workflows/luajit-integration.yml@master
> > +    with:
> > +      GC64: ON
> > +      buildtype: RelWithDebInfo
> > +      host: ubuntu-20.04-self-hosted
> > +      revision: ${{ github.sha }}

<snipped>

> > -- 
> > 2.34.0
> > 
> 
> -- 
> Best regards,
> Sergey Kaplun

-- 
Best regards,
IM

^ permalink raw reply	[flat|nested] 21+ messages in thread

* Re: [Tarantool-patches] [PATCH luajit 1/3] ci: fix --parallel argument for MacOS runners
  2022-06-22 15:33 ` [Tarantool-patches] [PATCH luajit 1/3] ci: fix --parallel argument for MacOS runners Igor Munkin via Tarantool-patches
  2022-06-23  8:19   ` Sergey Kaplun via Tarantool-patches
@ 2022-07-01 13:12   ` Sergey Bronnikov via Tarantool-patches
  2022-07-04  7:05     ` Igor Munkin via Tarantool-patches
  2022-07-05 17:20   ` Sergey Bronnikov via Tarantool-patches
  2 siblings, 1 reply; 21+ messages in thread
From: Sergey Bronnikov via Tarantool-patches @ 2022-07-01 13:12 UTC (permalink / raw)
  To: Igor Munkin, Sergey Kaplun; +Cc: tarantool-patches

LGTM, thanks.

On 22.06.2022 18:33, Igor Munkin wrote:
> In scope of the commit 4195eb8f7e2abcf75f92eedd2859b7991cc8b363 ("ci:
> make GitHub workflows more CMake-ish") --parallel value has been
> explicitly set to $(nproc) + 1, since -j default behaviour differs for
> Ninja and GnuMake. However, MacOS lacks nproc command, but CI silently
> continues to execute pipeline.
>
> This patch fixed MacOS-specific workflows changing nproc command with
> sysctl -n hw.ncpu.
>
> Signed-off-by: Igor Munkin <imun@tarantool.org>
> ---
>   .github/workflows/macos-m1.yml     | 4 ++--
>   .github/workflows/macos-x86_64.yml | 4 ++--
>   2 files changed, 4 insertions(+), 4 deletions(-)
>
> diff --git a/.github/workflows/macos-m1.yml b/.github/workflows/macos-m1.yml
> index 8387bbcf..d1d144f0 100644
> --- a/.github/workflows/macos-m1.yml
> +++ b/.github/workflows/macos-m1.yml
> @@ -68,6 +68,6 @@ jobs:
>         - name: configure
>           run: ${ARCH} cmake . ${{ matrix.CMAKEFLAGS }} -DLUAJIT_ENABLE_GC64=${{ matrix.GC64 }}
>         - name: build
> -        run: ${ARCH} cmake --build . --parallel $(($(nproc) + 1))
> +        run: ${ARCH} cmake --build . --parallel $(($(sysctl -n hw.ncpu) + 1))
>         - name: test
> -        run: ${ARCH} cmake --build . --parallel $(($(nproc) + 1)) --target test
> +        run: ${ARCH} cmake --build . --parallel $(($(sysctl -n hw.ncpu) + 1)) --target test
> diff --git a/.github/workflows/macos-x86_64.yml b/.github/workflows/macos-x86_64.yml
> index 2ab2c8d0..a7c2f4e3 100644
> --- a/.github/workflows/macos-x86_64.yml
> +++ b/.github/workflows/macos-x86_64.yml
> @@ -63,6 +63,6 @@ jobs:
>         - name: configure
>           run: cmake . ${{ matrix.CMAKEFLAGS }} -DLUAJIT_ENABLE_GC64=${{ matrix.GC64 }}
>         - name: build
> -        run: cmake --build . --parallel $(($(nproc) + 1))
> +        run: cmake --build . --parallel $(($(sysctl -n hw.ncpu) + 1))
>         - name: test
> -        run: cmake --build . --parallel $(($(nproc) + 1)) --target test
> +        run: cmake --build . --parallel $(($(sysctl -n hw.ncpu) + 1)) --target test

^ permalink raw reply	[flat|nested] 21+ messages in thread

* Re: [Tarantool-patches] [PATCH luajit 2/3] ci: remove GC64 matrix entries for ARM64 workflows
  2022-06-22 15:33 ` [Tarantool-patches] [PATCH luajit 2/3] ci: remove GC64 matrix entries for ARM64 workflows Igor Munkin via Tarantool-patches
  2022-06-23  8:29   ` Sergey Kaplun via Tarantool-patches
@ 2022-07-01 13:21   ` Sergey Bronnikov via Tarantool-patches
  2022-07-04  7:05     ` Igor Munkin via Tarantool-patches
  1 sibling, 1 reply; 21+ messages in thread
From: Sergey Bronnikov via Tarantool-patches @ 2022-07-01 13:21 UTC (permalink / raw)
  To: Igor Munkin, Sergey Kaplun; +Cc: tarantool-patches

LGTM, thanks!

On 22.06.2022 18:33, Igor Munkin wrote:
> CMake option LUAJIT_ENABLE_GC64 changes nothing for ARM64 LuaJIT builds
> (i.e. LJ_GC64 is defined on ARM64 platforms despite the value set to
> LUAJIT_ENABLE_GC64). As a result GC64 entry is removed from both Linux
> and MacOS ARM64 workflows to reduce cloud resources costs and suspend
> global warming.
>
> Signed-off-by: Igor Munkin <imun@tarantool.org>
> ---
>   .github/workflows/linux-aarch64.yml | 5 ++---
>   .github/workflows/macos-m1.yml      | 5 ++---
>   2 files changed, 4 insertions(+), 6 deletions(-)
>
> diff --git a/.github/workflows/linux-aarch64.yml b/.github/workflows/linux-aarch64.yml
> index 19412b3f..de360b12 100644
> --- a/.github/workflows/linux-aarch64.yml
> +++ b/.github/workflows/linux-aarch64.yml
> @@ -35,13 +35,12 @@ jobs:
>         fail-fast: false
>         matrix:
>           BUILDTYPE: [Debug, Release]
> -        GC64: [ON, OFF]
>           include:
>             - BUILDTYPE: Debug
>               CMAKEFLAGS: -DCMAKE_BUILD_TYPE=Debug -DLUA_USE_ASSERT=ON -DLUA_USE_APICHECK=ON
>             - BUILDTYPE: Release
>               CMAKEFLAGS: -DCMAKE_BUILD_TYPE=RelWithDebInfo
> -    name: Linux/aarch64 ${{ matrix.BUILDTYPE }} GC64:${{ matrix.GC64 }}
> +    name: Linux/aarch64 ${{ matrix.BUILDTYPE }} GC64:ON
>       steps:
>         - uses: actions/checkout@v2.3.4
>           with:
> @@ -52,7 +51,7 @@ jobs:
>             sudo apt -y update
>             sudo apt -y install cmake gcc make perl
>         - name: configure
> -        run: cmake . ${{ matrix.CMAKEFLAGS }} -DLUAJIT_ENABLE_GC64=${{ matrix.GC64 }}
> +        run: cmake . ${{ matrix.CMAKEFLAGS }}
>         - name: build
>           run: cmake --build . --parallel $(($(nproc) + 1))
>         - name: test
> diff --git a/.github/workflows/macos-m1.yml b/.github/workflows/macos-m1.yml
> index d1d144f0..a38f70f7 100644
> --- a/.github/workflows/macos-m1.yml
> +++ b/.github/workflows/macos-m1.yml
> @@ -40,13 +40,12 @@ jobs:
>         fail-fast: false
>         matrix:
>           BUILDTYPE: [Debug, Release]
> -        GC64: [ON, OFF]
>           include:
>             - BUILDTYPE: Debug
>               CMAKEFLAGS: -DCMAKE_BUILD_TYPE=Debug -DLUA_USE_ASSERT=ON -DLUA_USE_APICHECK=ON
>             - BUILDTYPE: Release
>               CMAKEFLAGS: -DCMAKE_BUILD_TYPE=RelWithDebInfo
> -    name: macOS/m1 ${{ matrix.BUILDTYPE }} GC64:${{ matrix.GC64 }}
> +    name: macOS/m1 ${{ matrix.BUILDTYPE }} GC64:ON
>       steps:
>         - uses: actions/checkout@v2.3.4
>           with:
> @@ -66,7 +65,7 @@ jobs:
>             ${ARCH} brew install --force cmake gcc make perl ||
>               ${ARCH} brew upgrade cmake gcc make perl
>         - name: configure
> -        run: ${ARCH} cmake . ${{ matrix.CMAKEFLAGS }} -DLUAJIT_ENABLE_GC64=${{ matrix.GC64 }}
> +        run: ${ARCH} cmake . ${{ matrix.CMAKEFLAGS }}
>         - name: build
>           run: ${ARCH} cmake --build . --parallel $(($(sysctl -n hw.ncpu) + 1))
>         - name: test

^ permalink raw reply	[flat|nested] 21+ messages in thread

* Re: [Tarantool-patches] [PATCH luajit 3/3] ci: add Tarantool integration testing
  2022-06-22 15:33 ` [Tarantool-patches] [PATCH luajit 3/3] ci: add Tarantool integration testing Igor Munkin via Tarantool-patches
  2022-06-23  8:45   ` Sergey Kaplun via Tarantool-patches
@ 2022-07-01 13:39   ` Sergey Bronnikov via Tarantool-patches
  2022-07-04  7:07     ` Igor Munkin via Tarantool-patches
  1 sibling, 1 reply; 21+ messages in thread
From: Sergey Bronnikov via Tarantool-patches @ 2022-07-01 13:39 UTC (permalink / raw)
  To: Igor Munkin, Sergey Kaplun; +Cc: tarantool-patches

LGTM (see my comments inline).


On 22.06.2022 18:33, Igor Munkin wrote:
> This patch introduces additional steps with Tarantool integration
> testing to all existing LuaJIT workflows. All steps invoke reusable
> workflow[1] from Tarantool repository for each matrix entry being set
> for particular LuaJIT workflow.
>
> [1]: https://github.com/tarantool/tarantool/blob/master/.github/workflows/luajit-integration.yml
>
> Signed-off-by: Igor Munkin <imun@tarantool.org>
> ---
>   .github/workflows/linux-aarch64.yml | 25 +++++++++++++++--
>   .github/workflows/linux-x86_64.yml  | 43 +++++++++++++++++++++++++++--
>   .github/workflows/macos-m1.yml      | 25 +++++++++++++++--
>   .github/workflows/macos-x86_64.yml  | 43 +++++++++++++++++++++++++++--
>   4 files changed, 124 insertions(+), 12 deletions(-)
>
> diff --git a/.github/workflows/linux-aarch64.yml b/.github/workflows/linux-aarch64.yml
> index de360b12..293b572e 100644
> --- a/.github/workflows/linux-aarch64.yml
> +++ b/.github/workflows/linux-aarch64.yml
> @@ -1,4 +1,4 @@
> -name: "LuaJIT test workflow (Linux/aarch64)"
> +name: "Linux/aarch64 test workflow"
>   
Nit: seems changes here and below with renaming jobs are not related to 
commit.
>   on:
>     push:
> @@ -29,7 +29,7 @@ concurrency:
>     cancel-in-progress: true
>   
>   jobs:
> -  test-linux-aarch64:
> +  test-luajit:
>       runs-on: graviton
>       strategy:
>         fail-fast: false
> @@ -40,7 +40,7 @@ jobs:
>               CMAKEFLAGS: -DCMAKE_BUILD_TYPE=Debug -DLUA_USE_ASSERT=ON -DLUA_USE_APICHECK=ON
>             - BUILDTYPE: Release
>               CMAKEFLAGS: -DCMAKE_BUILD_TYPE=RelWithDebInfo
> -    name: Linux/aarch64 ${{ matrix.BUILDTYPE }} GC64:ON
> +    name: LuaJIT ${{ matrix.BUILDTYPE }} GC64:ON
>       steps:
>         - uses: actions/checkout@v2.3.4
>           with:
> @@ -56,3 +56,22 @@ jobs:
>           run: cmake --build . --parallel $(($(nproc) + 1))
>         - name: test
>           run: cmake --build . --parallel $(($(nproc) + 1)) --target test
> +
> +  test-tarantool-debug-w-GC64:
> +    name: Tarantool Debug GC64:ON
> +    needs: test-luajit
> +    uses: tarantool/tarantool/.github/workflows/luajit-integration.yml@master
> +    with:
> +      GC64: ON
> +      buildtype: Debug
> +      host: graviton
> +      revision: ${{ github.sha }}
> +  test-tarantool-release-w-GC64:
> +    name: Tarantool Release GC64:ON
> +    needs: test-luajit
> +    uses: tarantool/tarantool/.github/workflows/luajit-integration.yml@master
> +    with:
> +      GC64: ON
> +      buildtype: RelWithDebInfo
> +      host: graviton
> +      revision: ${{ github.sha }}
> diff --git a/.github/workflows/linux-x86_64.yml b/.github/workflows/linux-x86_64.yml
> index d6434afe..3eaf436d 100644
> --- a/.github/workflows/linux-x86_64.yml
> +++ b/.github/workflows/linux-x86_64.yml
> @@ -1,4 +1,4 @@
> -name: "LuaJIT test workflow (Linux/x86_64)"
> +name: "Linux/x86_64 test workflow"
>   
>   on:
>     push:
> @@ -29,7 +29,7 @@ concurrency:
>     cancel-in-progress: true
>   
>   jobs:
> -  test-linux-x86_64:
> +  test-luajit:
>       runs-on: ubuntu-20.04-self-hosted
>       strategy:
>         fail-fast: false
> @@ -41,7 +41,7 @@ jobs:
>               CMAKEFLAGS: -DCMAKE_BUILD_TYPE=Debug -DLUA_USE_ASSERT=ON -DLUA_USE_APICHECK=ON
>             - BUILDTYPE: Release
>               CMAKEFLAGS: -DCMAKE_BUILD_TYPE=RelWithDebInfo
> -    name: Linux/x86_64 ${{ matrix.BUILDTYPE }} GC64:${{ matrix.GC64 }}
> +    name: LuaJIT ${{ matrix.BUILDTYPE }} GC64:${{ matrix.GC64 }}
>       steps:
>         - uses: actions/checkout@v2.3.4
>           with:
> @@ -57,3 +57,40 @@ jobs:
>           run: cmake --build . --parallel $(($(nproc) + 1))
>         - name: test
>           run: cmake --build . --parallel $(($(nproc) + 1)) --target test
> +
> +  test-tarantool-debug-wo-GC64:
> +    name: Tarantool Debug GC64:OFF
> +    needs: test-luajit
> +    uses: tarantool/tarantool/.github/workflows/luajit-integration.yml@master
> +    with:
> +      GC64: OFF
> +      buildtype: Debug
> +      host: ubuntu-20.04-self-hosted
> +      revision: ${{ github.sha }}
> +  test-tarantool-debug-w-GC64:
> +    name: Tarantool Debug GC64:ON
> +    needs: test-luajit
> +    uses: tarantool/tarantool/.github/workflows/luajit-integration.yml@master
> +    with:
> +      GC64: ON
> +      buildtype: Debug
> +      host: ubuntu-20.04-self-hosted
> +      revision: ${{ github.sha }}
> +  test-tarantool-release-wo-GC64:
> +    name: Tarantool Release GC64:OFF
> +    needs: test-luajit
> +    uses: tarantool/tarantool/.github/workflows/luajit-integration.yml@master
> +    with:
> +      GC64: OFF
> +      buildtype: RelWithDebInfo
> +      host: ubuntu-20.04-self-hosted
> +      revision: ${{ github.sha }}
> +  test-tarantool-release-w-GC64:
> +    name: Tarantool Release GC64:ON
> +    needs: test-luajit
> +    uses: tarantool/tarantool/.github/workflows/luajit-integration.yml@master
> +    with:
> +      GC64: ON
> +      buildtype: RelWithDebInfo
> +      host: ubuntu-20.04-self-hosted
> +      revision: ${{ github.sha }}
> diff --git a/.github/workflows/macos-m1.yml b/.github/workflows/macos-m1.yml
> index a38f70f7..d305e609 100644
> --- a/.github/workflows/macos-m1.yml
> +++ b/.github/workflows/macos-m1.yml
> @@ -1,4 +1,4 @@
> -name: "LuaJIT test workflow (macOS/m1)"
> +name: "macOS/m1 test workflow"
>   
>   on:
>     push:
> @@ -34,7 +34,7 @@ env:
>     ARCH: arch -arm64
>   
>   jobs:
> -  test-macos-m1:
> +  test-luajit:
>       runs-on: macos-11-m1
>       strategy:
>         fail-fast: false
> @@ -45,7 +45,7 @@ jobs:
>               CMAKEFLAGS: -DCMAKE_BUILD_TYPE=Debug -DLUA_USE_ASSERT=ON -DLUA_USE_APICHECK=ON
>             - BUILDTYPE: Release
>               CMAKEFLAGS: -DCMAKE_BUILD_TYPE=RelWithDebInfo
> -    name: macOS/m1 ${{ matrix.BUILDTYPE }} GC64:ON
> +    name: LuaJIT ${{ matrix.BUILDTYPE }} GC64:ON
>       steps:
>         - uses: actions/checkout@v2.3.4
>           with:
> @@ -70,3 +70,22 @@ jobs:
>           run: ${ARCH} cmake --build . --parallel $(($(sysctl -n hw.ncpu) + 1))
>         - name: test
>           run: ${ARCH} cmake --build . --parallel $(($(sysctl -n hw.ncpu) + 1)) --target test
> +
> +  test-tarantool-debug-w-GC64:
> +    name: Tarantool Debug GC64:ON
> +    needs: test-luajit
> +    uses: tarantool/tarantool/.github/workflows/luajit-integration.yml@master
> +    with:
> +      GC64: ON
> +      buildtype: Debug
> +      host: macos-11-m1
> +      revision: ${{ github.sha }}
> +  test-tarantool-release-w-GC64:
> +    name: Tarantool Release GC64:ON
> +    needs: test-luajit
> +    uses: tarantool/tarantool/.github/workflows/luajit-integration.yml@master
> +    with:
> +      GC64: ON
> +      buildtype: RelWithDebInfo
> +      host: macos-11-m1
> +      revision: ${{ github.sha }}
> diff --git a/.github/workflows/macos-x86_64.yml b/.github/workflows/macos-x86_64.yml
> index a7c2f4e3..fc28c683 100644
> --- a/.github/workflows/macos-x86_64.yml
> +++ b/.github/workflows/macos-x86_64.yml
> @@ -1,4 +1,4 @@
> -name: "LuaJIT test workflow (macOS/x86_64)"
> +name: "macOS/x86_64 test workflow"
>   
>   on:
>     push:
> @@ -29,7 +29,7 @@ concurrency:
>     cancel-in-progress: true
>   
>   jobs:
> -  test-macos-x86_64:
> +  test-luajit:
>       runs-on: macos-11
>       strategy:
>         fail-fast: false
> @@ -41,7 +41,7 @@ jobs:
>               CMAKEFLAGS: -DCMAKE_BUILD_TYPE=Debug -DLUA_USE_ASSERT=ON -DLUA_USE_APICHECK=ON
>             - BUILDTYPE: Release
>               CMAKEFLAGS: -DCMAKE_BUILD_TYPE=RelWithDebInfo
> -    name: macOS/x86_64 ${{ matrix.BUILDTYPE }} GC64:${{ matrix.GC64 }}
> +    name: LuaJIT ${{ matrix.BUILDTYPE }} GC64:${{ matrix.GC64 }}
>       steps:
>         - uses: actions/checkout@v2.3.4
>           with:
> @@ -66,3 +66,40 @@ jobs:
>           run: cmake --build . --parallel $(($(sysctl -n hw.ncpu) + 1))
>         - name: test
>           run: cmake --build . --parallel $(($(sysctl -n hw.ncpu) + 1)) --target test
> +
> +  test-tarantool-debug-wo-GC64:
> +    name: Tarantool Debug GC64:OFF
> +    needs: test-luajit
> +    uses: tarantool/tarantool/.github/workflows/luajit-integration.yml@master
> +    with:
> +      GC64: OFF
> +      buildtype: Debug
> +      host: macos-11
> +      revision: ${{ github.sha }}
> +  test-tarantool-debug-w-GC64:
> +    name: Tarantool Debug GC64:ON
> +    needs: test-luajit
> +    uses: tarantool/tarantool/.github/workflows/luajit-integration.yml@master
> +    with:
> +      GC64: ON
> +      buildtype: Debug
> +      host: macos-11
> +      revision: ${{ github.sha }}
> +  test-tarantool-release-wo-GC64:
> +    name: Tarantool Release GC64:OFF
> +    needs: test-luajit
> +    uses: tarantool/tarantool/.github/workflows/luajit-integration.yml@master
> +    with:
> +      GC64: OFF
> +      buildtype: RelWithDebInfo
> +      host: macos-11
> +      revision: ${{ github.sha }}
> +  test-tarantool-release-w-GC64:
> +    name: Tarantool Release GC64:ON
> +    needs: test-luajit
> +    uses: tarantool/tarantool/.github/workflows/luajit-integration.yml@master
> +    with:
> +      GC64: ON
> +      buildtype: RelWithDebInfo
> +      host: macos-11
> +      revision: ${{ github.sha }}

^ permalink raw reply	[flat|nested] 21+ messages in thread

* Re: [Tarantool-patches] [PATCH luajit 1/3] ci: fix --parallel argument for MacOS runners
  2022-07-01 13:12   ` Sergey Bronnikov via Tarantool-patches
@ 2022-07-04  7:05     ` Igor Munkin via Tarantool-patches
  0 siblings, 0 replies; 21+ messages in thread
From: Igor Munkin via Tarantool-patches @ 2022-07-04  7:05 UTC (permalink / raw)
  To: Sergey Bronnikov; +Cc: tarantool-patches

Sergey,

Thanks for youre review!

On 01.07.22, Sergey Bronnikov wrote:
> LGTM, thanks.

Added your tag:
| Reviewed-by: Sergey Bronnikov <sergeyb@tarantool.org>

> 

<snipped>

-- 
Best regards,
IM

^ permalink raw reply	[flat|nested] 21+ messages in thread

* Re: [Tarantool-patches] [PATCH luajit 2/3] ci: remove GC64 matrix entries for ARM64 workflows
  2022-07-01 13:21   ` Sergey Bronnikov via Tarantool-patches
@ 2022-07-04  7:05     ` Igor Munkin via Tarantool-patches
  0 siblings, 0 replies; 21+ messages in thread
From: Igor Munkin via Tarantool-patches @ 2022-07-04  7:05 UTC (permalink / raw)
  To: Sergey Bronnikov; +Cc: tarantool-patches

Sergey,

Thanks for your review!

On 01.07.22, Sergey Bronnikov wrote:
> LGTM, thanks!

Added your tag:
| Reviewed-by: Sergey Bronnikov <sergeyb@tarantool.org>

> 

<snipped>

-- 
Best regards,
IM

^ permalink raw reply	[flat|nested] 21+ messages in thread

* Re: [Tarantool-patches] [PATCH luajit 3/3] ci: add Tarantool integration testing
  2022-07-01 13:39   ` Sergey Bronnikov via Tarantool-patches
@ 2022-07-04  7:07     ` Igor Munkin via Tarantool-patches
  2022-07-04 10:09       ` Sergey Bronnikov via Tarantool-patches
  0 siblings, 1 reply; 21+ messages in thread
From: Igor Munkin via Tarantool-patches @ 2022-07-04  7:07 UTC (permalink / raw)
  To: Sergey Bronnikov; +Cc: tarantool-patches

Sergey,

Thanks for your review!

On 01.07.22, Sergey Bronnikov wrote:
> LGTM (see my comments inline).

Added your tag:
| Reviewed-by: Sergey Bronnikov <sergeyb@tarantool.org>

Anyway, I've answered your comment with no fixes provided, so I postpone
the push until your explicit approvement.

> 
> 
> On 22.06.2022 18:33, Igor Munkin wrote:
> > This patch introduces additional steps with Tarantool integration
> > testing to all existing LuaJIT workflows. All steps invoke reusable
> > workflow[1] from Tarantool repository for each matrix entry being set
> > for particular LuaJIT workflow.
> >
> > [1]: https://github.com/tarantool/tarantool/blob/master/.github/workflows/luajit-integration.yml
> >
> > Signed-off-by: Igor Munkin <imun@tarantool.org>
> > ---
> >   .github/workflows/linux-aarch64.yml | 25 +++++++++++++++--
> >   .github/workflows/linux-x86_64.yml  | 43 +++++++++++++++++++++++++++--
> >   .github/workflows/macos-m1.yml      | 25 +++++++++++++++--
> >   .github/workflows/macos-x86_64.yml  | 43 +++++++++++++++++++++++++++--
> >   4 files changed, 124 insertions(+), 12 deletions(-)
> >
> > diff --git a/.github/workflows/linux-aarch64.yml b/.github/workflows/linux-aarch64.yml
> > index de360b12..293b572e 100644
> > --- a/.github/workflows/linux-aarch64.yml
> > +++ b/.github/workflows/linux-aarch64.yml
> > @@ -1,4 +1,4 @@
> > -name: "LuaJIT test workflow (Linux/aarch64)"
> > +name: "Linux/aarch64 test workflow"
> >   
> Nit: seems changes here and below with renaming jobs are not related to 
> commit.

Yes and no. I've added new steps, hence this becomes *not only* "LuaJIT
test workflow". I can move this renaming to the separate commit but
strongly doubt it worths. If it doesn't bother you a lot, I would
proceed with the patch with no changes.

> >   on:
> >     push:

<snipped>

-- 
Best regards,
IM

^ permalink raw reply	[flat|nested] 21+ messages in thread

* Re: [Tarantool-patches] [PATCH luajit 3/3] ci: add Tarantool integration testing
  2022-07-04  7:07     ` Igor Munkin via Tarantool-patches
@ 2022-07-04 10:09       ` Sergey Bronnikov via Tarantool-patches
  0 siblings, 0 replies; 21+ messages in thread
From: Sergey Bronnikov via Tarantool-patches @ 2022-07-04 10:09 UTC (permalink / raw)
  To: Igor Munkin; +Cc: tarantool-patches

Igor,

LGTM

On 04.07.2022 10:07, Igor Munkin wrote:
> Sergey,
>
> Thanks for your review!
>
> On 01.07.22, Sergey Bronnikov wrote:
>> LGTM (see my comments inline).
> Added your tag:
> | Reviewed-by: Sergey Bronnikov <sergeyb@tarantool.org>
>
> Anyway, I've answered your comment with no fixes provided, so I postpone
> the push until your explicit approvement.
>
>>
>> On 22.06.2022 18:33, Igor Munkin wrote:
>>> This patch introduces additional steps with Tarantool integration
>>> testing to all existing LuaJIT workflows. All steps invoke reusable
>>> workflow[1] from Tarantool repository for each matrix entry being set
>>> for particular LuaJIT workflow.
>>>
>>> [1]: https://github.com/tarantool/tarantool/blob/master/.github/workflows/luajit-integration.yml
>>>
>>> Signed-off-by: Igor Munkin <imun@tarantool.org>
>>> ---
>>>    .github/workflows/linux-aarch64.yml | 25 +++++++++++++++--
>>>    .github/workflows/linux-x86_64.yml  | 43 +++++++++++++++++++++++++++--
>>>    .github/workflows/macos-m1.yml      | 25 +++++++++++++++--
>>>    .github/workflows/macos-x86_64.yml  | 43 +++++++++++++++++++++++++++--
>>>    4 files changed, 124 insertions(+), 12 deletions(-)
>>>
>>> diff --git a/.github/workflows/linux-aarch64.yml b/.github/workflows/linux-aarch64.yml
>>> index de360b12..293b572e 100644
>>> --- a/.github/workflows/linux-aarch64.yml
>>> +++ b/.github/workflows/linux-aarch64.yml
>>> @@ -1,4 +1,4 @@
>>> -name: "LuaJIT test workflow (Linux/aarch64)"
>>> +name: "Linux/aarch64 test workflow"
>>>    
>> Nit: seems changes here and below with renaming jobs are not related to
>> commit.
> Yes and no. I've added new steps, hence this becomes *not only* "LuaJIT
> test workflow". I can move this renaming to the separate commit but
> strongly doubt it worths. If it doesn't bother you a lot, I would
> proceed with the patch with no changes.
>

I'll not insist with it, sure, let's proceed.

>>>    on:
>>>      push:
> <snipped>
>

^ permalink raw reply	[flat|nested] 21+ messages in thread

* Re: [Tarantool-patches] [PATCH luajit 3/3] ci: add Tarantool integration testing
  2022-06-29 22:17     ` Igor Munkin via Tarantool-patches
@ 2022-07-04 10:24       ` Sergey Kaplun via Tarantool-patches
  0 siblings, 0 replies; 21+ messages in thread
From: Sergey Kaplun via Tarantool-patches @ 2022-07-04 10:24 UTC (permalink / raw)
  To: Igor Munkin; +Cc: tarantool-patches

Thanks for the explanations, LGTM!

-- 
Best regards,
Sergey Kaplun

^ permalink raw reply	[flat|nested] 21+ messages in thread

* Re: [Tarantool-patches] [PATCH luajit 1/3] ci: fix --parallel argument for MacOS runners
  2022-06-22 15:33 ` [Tarantool-patches] [PATCH luajit 1/3] ci: fix --parallel argument for MacOS runners Igor Munkin via Tarantool-patches
  2022-06-23  8:19   ` Sergey Kaplun via Tarantool-patches
  2022-07-01 13:12   ` Sergey Bronnikov via Tarantool-patches
@ 2022-07-05 17:20   ` Sergey Bronnikov via Tarantool-patches
  2022-07-05 21:25     ` Igor Munkin via Tarantool-patches
  2 siblings, 1 reply; 21+ messages in thread
From: Sergey Bronnikov via Tarantool-patches @ 2022-07-05 17:20 UTC (permalink / raw)
  To: Igor Munkin, Sergey Kaplun; +Cc: tarantool-patches

Igor,

I found that hw.ncpu sysctl setting is deprecated and it is recommended 
to use

hw.logicalcpu, hw.logicalcpu_max, hw.physicalcpu, or hw.physicalcpu_max.

I believe hw.logicalcpu is the best choice for our case.

macOS manual pages are unavailable on apple.com [1], but there are other 
places:
[2], [3], [4].


1. 
https://developer.apple.com/library/mac/documentation/Darwin/Reference/ManPages/man3/sysctlbyname.3.html
2. https://www.unix.com/man-page/mojave/3/sysctlbyname/
3. Use of deprecated hw.ncpu https://github.com/ziglang/zig/issues/1252
4. sysctl name "hw.ncpu" (HW_NCPU) is deprecated in Mac OS X 
https://gitlab.haskell.org/ghc/ghc/-/issues/8594


Sergey

On 22.06.2022 18:33, Igor Munkin wrote:
> In scope of the commit 4195eb8f7e2abcf75f92eedd2859b7991cc8b363 ("ci:
> make GitHub workflows more CMake-ish") --parallel value has been
> explicitly set to $(nproc) + 1, since -j default behaviour differs for
> Ninja and GnuMake. However, MacOS lacks nproc command, but CI silently
> continues to execute pipeline.
>
> This patch fixed MacOS-specific workflows changing nproc command with
> sysctl -n hw.ncpu.
>
> Signed-off-by: Igor Munkin <imun@tarantool.org>
> ---
>   .github/workflows/macos-m1.yml     | 4 ++--
>   .github/workflows/macos-x86_64.yml | 4 ++--
>   2 files changed, 4 insertions(+), 4 deletions(-)
>
> diff --git a/.github/workflows/macos-m1.yml b/.github/workflows/macos-m1.yml
> index 8387bbcf..d1d144f0 100644
> --- a/.github/workflows/macos-m1.yml
> +++ b/.github/workflows/macos-m1.yml
> @@ -68,6 +68,6 @@ jobs:
>         - name: configure
>           run: ${ARCH} cmake . ${{ matrix.CMAKEFLAGS }} -DLUAJIT_ENABLE_GC64=${{ matrix.GC64 }}
>         - name: build
> -        run: ${ARCH} cmake --build . --parallel $(($(nproc) + 1))
> +        run: ${ARCH} cmake --build . --parallel $(($(sysctl -n hw.ncpu) + 1))
>         - name: test
> -        run: ${ARCH} cmake --build . --parallel $(($(nproc) + 1)) --target test
> +        run: ${ARCH} cmake --build . --parallel $(($(sysctl -n hw.ncpu) + 1)) --target test
> diff --git a/.github/workflows/macos-x86_64.yml b/.github/workflows/macos-x86_64.yml
> index 2ab2c8d0..a7c2f4e3 100644
> --- a/.github/workflows/macos-x86_64.yml
> +++ b/.github/workflows/macos-x86_64.yml
> @@ -63,6 +63,6 @@ jobs:
>         - name: configure
>           run: cmake . ${{ matrix.CMAKEFLAGS }} -DLUAJIT_ENABLE_GC64=${{ matrix.GC64 }}
>         - name: build
> -        run: cmake --build . --parallel $(($(nproc) + 1))
> +        run: cmake --build . --parallel $(($(sysctl -n hw.ncpu) + 1))
>         - name: test
> -        run: cmake --build . --parallel $(($(nproc) + 1)) --target test
> +        run: cmake --build . --parallel $(($(sysctl -n hw.ncpu) + 1)) --target test

^ permalink raw reply	[flat|nested] 21+ messages in thread

* Re: [Tarantool-patches] [PATCH luajit 1/3] ci: fix --parallel argument for MacOS runners
  2022-07-05 17:20   ` Sergey Bronnikov via Tarantool-patches
@ 2022-07-05 21:25     ` Igor Munkin via Tarantool-patches
  0 siblings, 0 replies; 21+ messages in thread
From: Igor Munkin via Tarantool-patches @ 2022-07-05 21:25 UTC (permalink / raw)
  To: Sergey Bronnikov; +Cc: tarantool-patches

Sergey,

On 05.07.22, Sergey Bronnikov wrote:
> Igor,
> 
> I found that hw.ncpu sysctl setting is deprecated and it is recommended 
> to use
> 
> hw.logicalcpu, hw.logicalcpu_max, hw.physicalcpu, or hw.physicalcpu_max.
> 
> I believe hw.logicalcpu is the best choice for our case.
> 
> macOS manual pages are unavailable on apple.com [1], but there are other 
> places:
> [2], [3], [4].
> 

Ouch... I've pushed this series yesterday, so I guess we can do a follow
up commit with s/hw.ncpu/hw.logicalcpu/g.

> 
> 1. 
> https://developer.apple.com/library/mac/documentation/Darwin/Reference/ManPages/man3/sysctlbyname.3.html
> 2. https://www.unix.com/man-page/mojave/3/sysctlbyname/
> 3. Use of deprecated hw.ncpu https://github.com/ziglang/zig/issues/1252
> 4. sysctl name "hw.ncpu" (HW_NCPU) is deprecated in Mac OS X 
> https://gitlab.haskell.org/ghc/ghc/-/issues/8594
> 
> 
> Sergey
> 

<snipped>

-- 
Best regards,
IM

^ permalink raw reply	[flat|nested] 21+ messages in thread

* Re: [Tarantool-patches] [PATCH luajit 0/3] Integration testing and tiny fixes in CI
  2022-06-22 15:33 [Tarantool-patches] [PATCH luajit 0/3] Integration testing and tiny fixes in CI Igor Munkin via Tarantool-patches
                   ` (2 preceding siblings ...)
  2022-06-22 15:33 ` [Tarantool-patches] [PATCH luajit 3/3] ci: add Tarantool integration testing Igor Munkin via Tarantool-patches
@ 2022-07-13 10:09 ` Igor Munkin via Tarantool-patches
  3 siblings, 0 replies; 21+ messages in thread
From: Igor Munkin via Tarantool-patches @ 2022-07-13 10:09 UTC (permalink / raw)
  To: Sergey Kaplun, Sergey Bronnikov; +Cc: tarantool-patches

I've checked the patchset into all long-term branches in
tarantool/luajit and bumped a new version in master, 2.10 and 1.10.

On 22.06.22, Igor Munkin wrote:
> After reusable workflow is merged into tarantool/tarantool master
> branch[1], we can proceed with LuaJIT integration testing patchset. The
> changeset contains a couple of CI-related fixes and introduces Tarantool
> integration testing in LuaJIT repository. You can find more verbose
> descriptions in the corresponding commit messages.
> 
> Branch: https://github.com/tarantool/luajit/tree/imun/ci-tarantool-tests
> CI: https://github.com/tarantool/luajit/commit/0b72879
> 
> [1]: https://github.com/tarantool/tarantool/pull/7276
> 
> Igor Munkin (3):
>   ci: fix --parallel argument for MacOS runners
>   ci: remove GC64 matrix entries for ARM64 workflows
>   ci: add Tarantool integration testing
> 
>  .github/workflows/linux-aarch64.yml | 28 ++++++++++++++---
>  .github/workflows/linux-x86_64.yml  | 43 ++++++++++++++++++++++++--
>  .github/workflows/macos-m1.yml      | 32 +++++++++++++++-----
>  .github/workflows/macos-x86_64.yml  | 47 ++++++++++++++++++++++++++---
>  4 files changed, 130 insertions(+), 20 deletions(-)
> 
> -- 
> 2.34.0
> 

-- 
Best regards,
IM

^ permalink raw reply	[flat|nested] 21+ messages in thread

end of thread, other threads:[~2022-07-13 10:09 UTC | newest]

Thread overview: 21+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2022-06-22 15:33 [Tarantool-patches] [PATCH luajit 0/3] Integration testing and tiny fixes in CI Igor Munkin via Tarantool-patches
2022-06-22 15:33 ` [Tarantool-patches] [PATCH luajit 1/3] ci: fix --parallel argument for MacOS runners Igor Munkin via Tarantool-patches
2022-06-23  8:19   ` Sergey Kaplun via Tarantool-patches
2022-06-29 22:17     ` Igor Munkin via Tarantool-patches
2022-07-01 13:12   ` Sergey Bronnikov via Tarantool-patches
2022-07-04  7:05     ` Igor Munkin via Tarantool-patches
2022-07-05 17:20   ` Sergey Bronnikov via Tarantool-patches
2022-07-05 21:25     ` Igor Munkin via Tarantool-patches
2022-06-22 15:33 ` [Tarantool-patches] [PATCH luajit 2/3] ci: remove GC64 matrix entries for ARM64 workflows Igor Munkin via Tarantool-patches
2022-06-23  8:29   ` Sergey Kaplun via Tarantool-patches
2022-06-29 22:17     ` Igor Munkin via Tarantool-patches
2022-07-01 13:21   ` Sergey Bronnikov via Tarantool-patches
2022-07-04  7:05     ` Igor Munkin via Tarantool-patches
2022-06-22 15:33 ` [Tarantool-patches] [PATCH luajit 3/3] ci: add Tarantool integration testing Igor Munkin via Tarantool-patches
2022-06-23  8:45   ` Sergey Kaplun via Tarantool-patches
2022-06-29 22:17     ` Igor Munkin via Tarantool-patches
2022-07-04 10:24       ` Sergey Kaplun via Tarantool-patches
2022-07-01 13:39   ` Sergey Bronnikov via Tarantool-patches
2022-07-04  7:07     ` Igor Munkin via Tarantool-patches
2022-07-04 10:09       ` Sergey Bronnikov via Tarantool-patches
2022-07-13 10:09 ` [Tarantool-patches] [PATCH luajit 0/3] Integration testing and tiny fixes in CI Igor Munkin via Tarantool-patches

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox