From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from [87.239.111.99] (localhost [127.0.0.1]) by dev.tarantool.org (Postfix) with ESMTP id D8CDE6EFDA; Thu, 2 Jun 2022 19:07:16 +0300 (MSK) DKIM-Filter: OpenDKIM Filter v2.11.0 dev.tarantool.org D8CDE6EFDA DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=tarantool.org; s=dev; t=1654186037; bh=b6ezMoWCc9oSY8id/jfA6Ltr+ezHgHY+kndBttA9wc4=; h=Date:To:References:In-Reply-To:Subject:List-Id:List-Unsubscribe: List-Archive:List-Post:List-Help:List-Subscribe:From:Reply-To:Cc: From; b=QdBKCDrHlgDdmaEniboImo8ecsBKGFZrTYAIzyfvMTAIAoOl+Upntw5nyydFfRfdr FPocNBQBljvAUTBnQv2fjrgVLC2hDijrH/ubJ029XslTqOr7ogBHSnFRBKI9RK0HHw pPHfvl/JlB4IcinFx9uYTiea/m3YDnQH1EoB7zpQ= Received: from smtpng2.i.mail.ru (smtpng2.i.mail.ru [94.100.179.3]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by dev.tarantool.org (Postfix) with ESMTPS id A6A006EFDA for ; Thu, 2 Jun 2022 19:07:14 +0300 (MSK) DKIM-Filter: OpenDKIM Filter v2.11.0 dev.tarantool.org A6A006EFDA Received: by smtpng2.m.smailru.net with esmtpa (envelope-from ) id 1nwnLt-0004zU-Lo; Thu, 02 Jun 2022 19:07:14 +0300 Date: Thu, 2 Jun 2022 19:00:10 +0300 To: Sergey Bronnikov Message-ID: References: <8826142ef8ae2e352728e3b4fdf5390851fe2e25.1654098694.git.imun@tarantool.org> <58ae0b0e-387a-2171-1d3b-dbdfa746c421@tarantool.org> MIME-Version: 1.0 Content-Type: text/plain; charset=utf-8 Content-Disposition: inline Content-Transfer-Encoding: 8bit In-Reply-To: <58ae0b0e-387a-2171-1d3b-dbdfa746c421@tarantool.org> X-Clacks-Overhead: GNU Terry Pratchett X-4EC0790: 10 X-7564579A: 646B95376F6C166E X-77F55803: 4F1203BC0FB41BD9A866A199C4CD7E1071F787FD1D87DD886398D2FD36F3E881182A05F538085040657D3369B56F8CF11CA059ED2D6CE20B72A4C06F4286D3F3EFD1EDD3001B55DF X-7FA49CB5: FF5795518A3D127A4AD6D5ED66289B5278DA827A17800CE73A0E02362971E860EA1F7E6F0F101C67BD4B6F7A4D31EC0BCC500DACC3FED6E28638F802B75D45FF8AA50765F790063763CA9D338DE912778638F802B75D45FF36EB9D2243A4F8B5A6FCA7DBDB1FC311F39EFFDF887939037866D6147AF826D8616C414DAFF26E5B738FA6C69A237715117882F4460429724CE54428C33FAD305F5C1EE8F4F765FCAA867293B0326636D2E47CDBA5A96583BD4B6F7A4D31EC0BC014FD901B82EE079FA2833FD35BB23D27C277FBC8AE2E8BAA867293B0326636D2E47CDBA5A96583BA9C0B312567BB231DD303D21008E29813377AFFFEAFD269A417C69337E82CC2E827F84554CEF50127C277FBC8AE2E8BA83251EDC214901ED5E8D9A59859A8B6D635BA3ABDB36C18089D37D7C0E48F6C5571747095F342E88FB05168BE4CE3AF X-8FC586DF: 6EFBBC1D9D64D975 X-C1DE0DAB: 0D63561A33F958A569CF181CEE853820B4CF035C5E19483762C99697629E26DFD59269BC5F550898D99A6476B3ADF6B4886A5961035A09600383DAD389E261318FB05168BE4CE3AF X-C8649E89: 4E36BF7865823D7055A7F0CF078B5EC49A30900B95165D3407FE5477D6A8AF08C03344F53FD1F00681923C53CDBDF4938A34F80912DC51AF28B6FBC04D8105F01D7E09C32AA3244C9032D00127375B011B4622F582EB1C6CC3B3ADDA61883BB5927AC6DF5659F194 X-D57D3AED: 3ZO7eAau8CL7WIMRKs4sN3D3tLDjz0dLbV79QFUyzQ2Ujvy7cMT6pYYqY16iZVKkSc3dCLJ7zSJH7+u4VD18S7Vl4ZUrpaVfd2+vE6kuoey4m4VkSEu530nj6fImhcD4MUrOEAnl0W826KZ9Q+tr5ycPtXkTV4k65bRjmOUUP8cvGozZ33TWg5HZplvhhXbhDGzqmQDTd6OAevLeAnq3Ra9uf7zvY2zzsIhlcp/Y7m53TZgf2aB4JOg4gkr2biojRxBGqISLOzCP1ITchhL9Vg== X-Mailru-Sender: 689FA8AB762F739339CABD9B3CA9A7D620DEE3A621BFDBC1E7EF2D5068406ECEA7C8D0F45F857DBFE9F1EFEE2F478337FB559BB5D741EB964C8C2C849690F8E70A04DAD6CC59E3365FEEDEB644C299C0ED14614B50AE0675 X-Mras: Ok Subject: Re: [Tarantool-patches] [PATCH luajit] ci: make GitHub workflows more CMake-ish X-BeenThere: tarantool-patches@dev.tarantool.org X-Mailman-Version: 2.1.34 Precedence: list List-Id: Tarantool development patches List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , From: Igor Munkin via Tarantool-patches Reply-To: Igor Munkin Cc: tarantool-patches@dev.tarantool.org Errors-To: tarantool-patches-bounces@dev.tarantool.org Sender: "Tarantool-patches" Sergey, Thanks for your review! On 02.06.22, Sergey Bronnikov wrote: > Igor, thanks for the patch! > > See my comments below. > > 1. I propose to use long-options for clarity. In particular I mean > long-option for "-j", it is better to replace it with "--parallel", see [1]. Agree, fixed. Diff is below: ================================================================================ diff --git a/.github/workflows/lint.yml b/.github/workflows/lint.yml index ff4b0ac2..c629e5f4 100644 --- a/.github/workflows/lint.yml +++ b/.github/workflows/lint.yml @@ -45,4 +45,4 @@ jobs: - name: configure run: cmake . - name: test - run: cmake --build . -j -t LuaJIT-luacheck + run: cmake --build . --parallel --target LuaJIT-luacheck diff --git a/.github/workflows/linux-aarch64.yml b/.github/workflows/linux-aarch64.yml index 2f62394f..19a6e14a 100644 --- a/.github/workflows/linux-aarch64.yml +++ b/.github/workflows/linux-aarch64.yml @@ -54,6 +54,6 @@ jobs: - name: configure run: cmake . ${{ matrix.CMAKEFLAGS }} -DLUAJIT_ENABLE_GC64=${{ matrix.GC64 }} - name: build - run: cmake --build . -j + run: cmake --build . --parallel - name: test - run: cmake --build . -j -t test + run: cmake --build . --parallel --target test diff --git a/.github/workflows/linux-x86_64.yml b/.github/workflows/linux-x86_64.yml index 4e6e3d9b..6b17d1a3 100644 --- a/.github/workflows/linux-x86_64.yml +++ b/.github/workflows/linux-x86_64.yml @@ -54,6 +54,6 @@ jobs: - name: configure run: cmake . ${{ matrix.CMAKEFLAGS }} -DLUAJIT_ENABLE_GC64=${{ matrix.GC64 }} - name: build - run: cmake --build . -j + run: cmake --build . --parallel - name: test - run: cmake --build . -j -t test + run: cmake --build . --parallel --target test diff --git a/.github/workflows/macos-m1.yml b/.github/workflows/macos-m1.yml index a272e39b..a931495b 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 . -j + run: ${ARCH} cmake --build . --parallel - name: test - run: ${ARCH} cmake --build . -j -t test + run: ${ARCH} cmake --build . --parallel --target test diff --git a/.github/workflows/macos-x86_64.yml b/.github/workflows/macos-x86_64.yml index d639e872..99bd9fe9 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 . -j + run: cmake --build . --parallel - name: test - run: cmake --build . -j -t test + run: cmake --build . --parallel --target test ================================================================================ > > 2. I propose to define a number of threads explicitly, because Ninja and > Make have different meaning for -j without a number of threads: > > Ninja: > >       -j N   run N jobs in parallel (0 means infinity) [default=#CPUs] > > Make: > >        -j [jobs], --jobs[=jobs] >             Specifies the number of jobs (commands) to run > simultaneously.  If >             there is more than one -j option, the last one is > effective.  If >             the -j option is given without an argument, make will not limit >             the number of jobs that can run simultaneously. When make > invokes >             a sub-make, all instances of make will coordinate to run the >             specified number of jobs at a time; see the section > PARALLEL MAKE >             AND THE JOBSERVER for details. > > It is recommended to use a number (CPU cores + 1), so we can explicitly > set "$(nproc) + 1". Also fixed. Diff is below: ================================================================================ diff --git a/.github/workflows/lint.yml b/.github/workflows/lint.yml index c629e5f4..3091c10b 100644 --- a/.github/workflows/lint.yml +++ b/.github/workflows/lint.yml @@ -45,4 +45,4 @@ jobs: - name: configure run: cmake . - name: test - run: cmake --build . --parallel --target LuaJIT-luacheck + run: cmake --build . --parallel $(($(nproc) + 1) --target LuaJIT-luacheck diff --git a/.github/workflows/linux-aarch64.yml b/.github/workflows/linux-aarch64.yml index 19a6e14a..19412b3f 100644 --- a/.github/workflows/linux-aarch64.yml +++ b/.github/workflows/linux-aarch64.yml @@ -54,6 +54,6 @@ jobs: - name: configure run: cmake . ${{ matrix.CMAKEFLAGS }} -DLUAJIT_ENABLE_GC64=${{ matrix.GC64 }} - name: build - run: cmake --build . --parallel + run: cmake --build . --parallel $(($(nproc) + 1)) - name: test - run: cmake --build . --parallel --target test + run: cmake --build . --parallel $(($(nproc) + 1)) --target test diff --git a/.github/workflows/linux-x86_64.yml b/.github/workflows/linux-x86_64.yml index 6b17d1a3..d6434afe 100644 --- a/.github/workflows/linux-x86_64.yml +++ b/.github/workflows/linux-x86_64.yml @@ -54,6 +54,6 @@ jobs: - name: configure run: cmake . ${{ matrix.CMAKEFLAGS }} -DLUAJIT_ENABLE_GC64=${{ matrix.GC64 }} - name: build - run: cmake --build . --parallel + run: cmake --build . --parallel $(($(nproc) + 1)) - name: test - run: cmake --build . --parallel --target test + run: cmake --build . --parallel $(($(nproc) + 1)) --target test diff --git a/.github/workflows/macos-m1.yml b/.github/workflows/macos-m1.yml index a931495b..8387bbcf 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 + run: ${ARCH} cmake --build . --parallel $(($(nproc) + 1)) - name: test - run: ${ARCH} cmake --build . --parallel --target test + run: ${ARCH} cmake --build . --parallel $(($(nproc) + 1)) --target test diff --git a/.github/workflows/macos-x86_64.yml b/.github/workflows/macos-x86_64.yml index 99bd9fe9..2ab2c8d0 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 + run: cmake --build . --parallel $(($(nproc) + 1)) - name: test - run: cmake --build . --parallel --target test + run: cmake --build . --parallel $(($(nproc) + 1)) --target test ================================================================================ Side note: this $(($(..)..)) mess looks a bit ugly, but I can't imagine another way to implement the approach you're proposing. Furthermore, there is nothing similar provided by Github Actions, so I believe your way is the only one. > > There is a CMake option "" [2], but we couldn't use it as it is > available since CMake 3.12, > > when LuaJIT build system has a minimal version only 3.1. Unfortunately, we have to wait until Debian Stretch, Ubuntu Xenial and Ubuntu Bionic reach their EOL to bump CMake minimal version up to 3.12. Looking forward to these days... > > > 1. https://cmake.org/cmake/help/v3.12/manual/cmake.1.html#build-tool-mode > > 2. > https://cmake.org/cmake/help/latest/envvar/CMAKE_BUILD_PARALLEL_LEVEL.html > > Sergey > > On 01.06.2022 18:54, Igor Munkin wrote: -- Best regards, IM