From d3394905b413125189d2d73bf46d44a0cd60678e Mon Sep 17 00:00:00 2001 From: Mikhail Yakshin Date: Fri, 2 Jun 2023 09:57:41 +0100 Subject: [PATCH 01/21] Added build & run unittest scripts for Linux --- .build/build | 8 ++++++++ .build/run-unittest | 6 ++++++ 2 files changed, 14 insertions(+) create mode 100755 .build/build create mode 100755 .build/run-unittest diff --git a/.build/build b/.build/build new file mode 100755 index 0000000..9fa3b94 --- /dev/null +++ b/.build/build @@ -0,0 +1,8 @@ +#!/bin/sh -ef + +cd $(dirname "$0")/.. + +mkdir -p build +cd build +cmake .. +cmake --build . diff --git a/.build/run-unittest b/.build/run-unittest new file mode 100755 index 0000000..c8e4525 --- /dev/null +++ b/.build/run-unittest @@ -0,0 +1,6 @@ +#!/bin/sh -ef + +cd $(dirname "$0")/.. + +cd build/tests +./unittest From 6e3dc81b8f9a3b46f2124cad278ec1740e448f8a Mon Sep 17 00:00:00 2001 From: Mikhail Yakshin Date: Fri, 2 Jun 2023 09:58:09 +0100 Subject: [PATCH 02/21] Added GitHub actions flow for building & testing on Linux --- .github/workflows/build.yml | 20 ++++++++++++++++++++ 1 file changed, 20 insertions(+) create mode 100644 .github/workflows/build.yml diff --git a/.github/workflows/build.yml b/.github/workflows/build.yml new file mode 100644 index 0000000..2406fab --- /dev/null +++ b/.github/workflows/build.yml @@ -0,0 +1,20 @@ +name: Unit tests + +on: + push: + branches: + - master + pull_request: {} + +jobs: + linux: + runs-on: ubuntu-latest + steps: + - uses: actions/checkout@v1 + - name: restore + run: | + apt-get install -y libgtest-dev + - name: build + run: .build/build + - name: unittest + run: .build/run-unittest From dc519a3ada29df4bbbf125dcd19113af48ee881c Mon Sep 17 00:00:00 2001 From: Mikhail Yakshin Date: Fri, 2 Jun 2023 10:00:42 +0100 Subject: [PATCH 03/21] Try with sudo --- .github/workflows/build.yml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/.github/workflows/build.yml b/.github/workflows/build.yml index 2406fab..f5d2965 100644 --- a/.github/workflows/build.yml +++ b/.github/workflows/build.yml @@ -13,7 +13,7 @@ jobs: - uses: actions/checkout@v1 - name: restore run: | - apt-get install -y libgtest-dev + sudo apt-get install -y libgtest-dev - name: build run: .build/build - name: unittest From af16c09bf7d908a61fae3e21e2cc1b95a821ce8e Mon Sep 17 00:00:00 2001 From: Michael Yakshin Date: Fri, 2 Jun 2023 10:23:16 +0100 Subject: [PATCH 04/21] Added CI part for Windows --- .build/build.ps1 | 28 ++++++++++++++++++++++++++++ .build/run-unittest.ps1 | 19 +++++++++++++++++++ .github/workflows/build.yml | 14 ++++++++++++++ 3 files changed, 61 insertions(+) create mode 100644 .build/build.ps1 create mode 100644 .build/run-unittest.ps1 diff --git a/.build/build.ps1 b/.build/build.ps1 new file mode 100644 index 0000000..121195c --- /dev/null +++ b/.build/build.ps1 @@ -0,0 +1,28 @@ +<# +.DESCRIPTION +Builds Kaitai Struct C++ runtime library and unit tests + +Requires: +- MSVC native tools installed and available in the command prompt +- cmake/ctest available (normally installed with MSVC native tools) +- GTest installed, path passed in `-GTestPath` +#> + +[CmdletBinding()] +param ( + [string] $GTestPath +) + +# Standard boilerplate +Set-StrictMode -Version Latest +$ErrorActionPreference = "Stop" +$PSDefaultParameterValues['*:ErrorAction']='Stop' + +Push-Location ..\build + +cmake -DCMAKE_PREFIX_PATH="$GTestPath" -DSTRING_ENCODING_TYPE=NONE .. +cmake --build . --config Debug +cp $GTestPath\debug\bin\*.dll tests\Debug +cp Debug\kaitai_struct_cpp_stl_runtime.dll tests\Debug + +Pop-Location diff --git a/.build/run-unittest.ps1 b/.build/run-unittest.ps1 new file mode 100644 index 0000000..814d4e2 --- /dev/null +++ b/.build/run-unittest.ps1 @@ -0,0 +1,19 @@ +<# +.DESCRIPTION +Runs unit tests on Windows +#> + +# Standard boilerplate +Set-StrictMode -Version Latest +$ErrorActionPreference = "Stop" +$PSDefaultParameterValues['*:ErrorAction']='Stop' + +Push-Location ..\build + +# Use ctest +#ctest -C Debug --output-on-failure + +# Run gtest-generated binary directly, produces more detailed output +./tests/Debug/unittest.exe + +Pop-Location diff --git a/.github/workflows/build.yml b/.github/workflows/build.yml index f5d2965..d16f35c 100644 --- a/.github/workflows/build.yml +++ b/.github/workflows/build.yml @@ -18,3 +18,17 @@ jobs: run: .build/build - name: unittest run: .build/run-unittest + + windows: + runs-on: windows-latest + # NB: see https://github.com/actions/runner-images/blob/main/images/win/Windows2022-Readme.md for what's available in this image + steps: + - uses: actions/checkout@v1 + - name: restore + run: | + C:\vcpkg\bootstrap-vcpkg.bat + C:\vcpkg\vcpkg install gtest + - name: build + run: .build\build.ps1 -GTestPath C:\vcpkg\installed\x64-windows + - name: unittest + run: .build\run-unittest.ps1 From 91d5269a3e777f321940788228db2e21fb3a5f59 Mon Sep 17 00:00:00 2001 From: Mikhail Yakshin Date: Fri, 2 Jun 2023 10:24:17 +0100 Subject: [PATCH 05/21] Update actions version as per PR comments MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Co-authored-by: Petr Pučil --- .github/workflows/build.yml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/.github/workflows/build.yml b/.github/workflows/build.yml index d16f35c..0d3d59e 100644 --- a/.github/workflows/build.yml +++ b/.github/workflows/build.yml @@ -10,7 +10,7 @@ jobs: linux: runs-on: ubuntu-latest steps: - - uses: actions/checkout@v1 + - uses: actions/checkout@v3 - name: restore run: | sudo apt-get install -y libgtest-dev From 76610d3b5352a50524ce4586ef1c510397b57ac1 Mon Sep 17 00:00:00 2001 From: Michael Yakshin Date: Fri, 2 Jun 2023 10:34:32 +0100 Subject: [PATCH 06/21] Ensure build/ dir exists --- .build/build.ps1 | 1 + 1 file changed, 1 insertion(+) diff --git a/.build/build.ps1 b/.build/build.ps1 index 121195c..00aa064 100644 --- a/.build/build.ps1 +++ b/.build/build.ps1 @@ -18,6 +18,7 @@ Set-StrictMode -Version Latest $ErrorActionPreference = "Stop" $PSDefaultParameterValues['*:ErrorAction']='Stop' +$null = New-Item ..\build -ItemType Directory -Force Push-Location ..\build cmake -DCMAKE_PREFIX_PATH="$GTestPath" -DSTRING_ENCODING_TYPE=NONE .. From 3c3092fc5967229508c464520da3f18a7c3623dc Mon Sep 17 00:00:00 2001 From: Michael Yakshin Date: Fri, 2 Jun 2023 10:34:49 +0100 Subject: [PATCH 07/21] Only build/install gtest for x64-windows --- .github/workflows/build.yml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/.github/workflows/build.yml b/.github/workflows/build.yml index d16f35c..d48f725 100644 --- a/.github/workflows/build.yml +++ b/.github/workflows/build.yml @@ -27,7 +27,7 @@ jobs: - name: restore run: | C:\vcpkg\bootstrap-vcpkg.bat - C:\vcpkg\vcpkg install gtest + C:\vcpkg\vcpkg install gtest:x64-windows - name: build run: .build\build.ps1 -GTestPath C:\vcpkg\installed\x64-windows - name: unittest From 2439a0b98c5addfc216eafe4661ab9ac80e61f55 Mon Sep 17 00:00:00 2001 From: Michael Yakshin Date: Fri, 2 Jun 2023 10:57:17 +0100 Subject: [PATCH 08/21] Update Windows checkout@v3 as well --- .github/workflows/build.yml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/.github/workflows/build.yml b/.github/workflows/build.yml index 69aa31b..07529fc 100644 --- a/.github/workflows/build.yml +++ b/.github/workflows/build.yml @@ -23,7 +23,7 @@ jobs: runs-on: windows-latest # NB: see https://github.com/actions/runner-images/blob/main/images/win/Windows2022-Readme.md for what's available in this image steps: - - uses: actions/checkout@v1 + - uses: actions/checkout@v3 - name: restore run: | C:\vcpkg\bootstrap-vcpkg.bat From d0136fc66d58436ce981a8d22dfa7822c7ceff2d Mon Sep 17 00:00:00 2001 From: Michael Yakshin Date: Fri, 2 Jun 2023 10:57:45 +0100 Subject: [PATCH 09/21] Made Windows .build agnostic to which dir they are invoked from --- .build/build.ps1 | 9 +++++++-- .build/run-unittest.ps1 | 6 +++++- 2 files changed, 12 insertions(+), 3 deletions(-) diff --git a/.build/build.ps1 b/.build/build.ps1 index 00aa064..c3b0025 100644 --- a/.build/build.ps1 +++ b/.build/build.ps1 @@ -10,6 +10,7 @@ Requires: [CmdletBinding()] param ( + [Parameter(Mandatory=$true)] [string] $GTestPath ) @@ -18,8 +19,12 @@ Set-StrictMode -Version Latest $ErrorActionPreference = "Stop" $PSDefaultParameterValues['*:ErrorAction']='Stop' -$null = New-Item ..\build -ItemType Directory -Force -Push-Location ..\build +# Go to repo root +$repoRoot = (Resolve-Path "$PSScriptRoot\..").Path +Push-Location $repoRoot + +$null = New-Item build -ItemType Directory -Force +cd build cmake -DCMAKE_PREFIX_PATH="$GTestPath" -DSTRING_ENCODING_TYPE=NONE .. cmake --build . --config Debug diff --git a/.build/run-unittest.ps1 b/.build/run-unittest.ps1 index 814d4e2..aed5f21 100644 --- a/.build/run-unittest.ps1 +++ b/.build/run-unittest.ps1 @@ -8,7 +8,11 @@ Set-StrictMode -Version Latest $ErrorActionPreference = "Stop" $PSDefaultParameterValues['*:ErrorAction']='Stop' -Push-Location ..\build +# Go to repo root +$repoRoot = (Resolve-Path "$PSScriptRoot\..").Path +Push-Location $repoRoot + +cd build # Use ctest #ctest -C Debug --output-on-failure From a26e9907363441b6c238dcf12e188fd7c462fed8 Mon Sep 17 00:00:00 2001 From: Michael Yakshin Date: Fri, 2 Jun 2023 11:08:17 +0100 Subject: [PATCH 10/21] Fix weird cast which resulted in compiler warning; in reality, this should never ever work as uint64, unary minus would have always typecasted this implicitly to signed anyway --- kaitai/kaitaistream.h | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/kaitai/kaitaistream.h b/kaitai/kaitaistream.h index 3a25a7d..0d9e3f0 100644 --- a/kaitai/kaitaistream.h +++ b/kaitai/kaitaistream.h @@ -246,7 +246,7 @@ class kstream { if (val < 0) { buf[0] = '-'; // get absolute value without undefined behavior (https://stackoverflow.com/a/12231604) - unsigned_to_decimal(-static_cast(val), &buf[1]); + unsigned_to_decimal(-static_cast(val), &buf[1]); } else { unsigned_to_decimal(val, buf); } From de9b3cd8d8400739072659c487b0bd9209b20842 Mon Sep 17 00:00:00 2001 From: Mikhail Yakshin Date: Fri, 16 Jun 2023 10:45:28 +0100 Subject: [PATCH 11/21] Replaced middle part of to_string() implementation with one liner with a lot of comments from Petr --- kaitai/kaitaistream.h | 36 ++++++++++++++++++++++++++++++++++-- 1 file changed, 34 insertions(+), 2 deletions(-) diff --git a/kaitai/kaitaistream.h b/kaitai/kaitaistream.h index 0d9e3f0..668787d 100644 --- a/kaitai/kaitaistream.h +++ b/kaitai/kaitaistream.h @@ -245,8 +245,40 @@ class kstream { char buf[std::numeric_limits::digits10 + 5]; if (val < 0) { buf[0] = '-'; - // get absolute value without undefined behavior (https://stackoverflow.com/a/12231604) - unsigned_to_decimal(-static_cast(val), &buf[1]); + + // NB: `val` is negative and we need to get its absolute value (i.e. minus `val`). However, since + // `int64_t` uses two's complement representation, its range is `[-2**63, 2**63 - 1] = + // [-0x8000_0000_0000_0000, 0x7fff_ffff_ffff_ffff]` (both ends inclusive) and thus the naive + // `-val` operation will overflow for `val = std::numeric_limits::min() = + // -0x8000_0000_0000_0000` (because the result of `-val` is mathematically + // `-(-0x8000_0000_0000_0000) = 0x8000_0000_0000_0000`, but the `int64_t` type can represent at + // most `0x7fff_ffff_ffff_ffff`). And signed integer overflow is undefined behavior in C++. + // + // To avoid undefined behavior for `val = -0x8000_0000_0000_0000 = -2**63`, we do the following + // steps for all negative `val`s: + // + // 1. Convert the signed (and negative) `val` to an unsigned `uint64_t` type. This is a + // well-defined operation in C++: the resulting `uint64_t` value will be `val mod 2**64` (`mod` + // is modulo). The maximum `val` we can have here is `-1` (because `val < 0`), a theoretical + // minimum we are able to support would be `-2**64 + 1 = -0xffff_ffff_ffff_ffff` (even though + // in practice the widest standard type is `int64_t` with the minimum of `-2**63`): + // + // * `static_cast(-1) = -1 mod 2**64 = 2**64 + (-1) = 0xffff_ffff_ffff_ffff = 2**64 - 1` + // * `static_cast(-2**64 + 1) = (-2**64 + 1) mod 2**64 = 2**64 + (-2**64 + 1) = 1` + // + // 2. Subtract `static_cast(val)` from `2**64 - 1 = 0xffff_ffff_ffff_ffff`. Since + // `static_cast(val)` is in range `[1, 2**64 - 1]` (see step 1), the result of this + // subtraction will be mathematically in range `[0, (2**64 - 1) - 1] = [0, 2**64 - 2]`. So the + // mathematical result cannot be negative, hence this unsigned integer subtraction can never + // wrap around (which wouldn't be a good thing to rely upon because it confuses programmers and + // code analysis tools). + // + // 3. Since we did mathematically `(2**64 - 1) - (2**64 + val) = -val - 1` so far (and we wanted + // to do `-val`), we add `1` to correct that. From step 2 we know that the result of `-val - 1` + // is in range `[0, 2**64 - 2]`, so adding `1` will not wrap (at most we could get `2**64 - 1 = + // 0xffff_ffff_ffff_ffff`, which is still in the valid range of `uint64_t`). + + unsigned_to_decimal((std::numeric_limits::max() - static_cast(val)) + 1, &buf[1]); } else { unsigned_to_decimal(val, buf); } From 7ada9b8cc84d43f57837cf419500fd7c9ab07bf6 Mon Sep 17 00:00:00 2001 From: Mikhail Yakshin Date: Fri, 16 Jun 2023 17:03:47 +0100 Subject: [PATCH 12/21] Fix shell quoting bug around dirname MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Co-authored-by: Petr Pučil --- .build/build | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/.build/build b/.build/build index 9fa3b94..e1109b4 100755 --- a/.build/build +++ b/.build/build @@ -1,6 +1,6 @@ #!/bin/sh -ef -cd $(dirname "$0")/.. +cd "$(dirname "$0")"/.. mkdir -p build cd build From 0c165f4c2c4fd693c8910d1da8394bfbff852d21 Mon Sep 17 00:00:00 2001 From: Mikhail Yakshin Date: Fri, 16 Jun 2023 17:04:03 +0100 Subject: [PATCH 13/21] Fix shell quoting bug around dirname MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Co-authored-by: Petr Pučil --- .build/run-unittest | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/.build/run-unittest b/.build/run-unittest index c8e4525..0ffa351 100755 --- a/.build/run-unittest +++ b/.build/run-unittest @@ -1,6 +1,6 @@ #!/bin/sh -ef -cd $(dirname "$0")/.. +cd "$(dirname "$0")"/.. cd build/tests ./unittest From 13d315710e894fefba606b6b305106051da5b82c Mon Sep 17 00:00:00 2001 From: Mikhail Yakshin Date: Fri, 16 Jun 2023 17:27:41 +0100 Subject: [PATCH 14/21] Add Mingw CI support --- .build/build-mingw.ps1 | 34 ++++++++++++++++++++++++++++ .build/{build.ps1 => build-msvc.ps1} | 2 +- .github/workflows/build.yml | 20 ++++++++++++++-- 3 files changed, 53 insertions(+), 3 deletions(-) create mode 100644 .build/build-mingw.ps1 rename .build/{build.ps1 => build-msvc.ps1} (89%) diff --git a/.build/build-mingw.ps1 b/.build/build-mingw.ps1 new file mode 100644 index 0000000..70e7c92 --- /dev/null +++ b/.build/build-mingw.ps1 @@ -0,0 +1,34 @@ +<# +.DESCRIPTION +Builds Kaitai Struct C++ runtime library and unit tests on Windows using Mingw + +Requires: +- Mingw installed and available in the command prompt +- cmake/ctest available +- GTest installed, path passed in `-GTestPath` +#> + +[CmdletBinding()] +param ( + [Parameter(Mandatory=$true)] + [string] $GTestPath +) + +# Standard boilerplate +Set-StrictMode -Version Latest +$ErrorActionPreference = "Stop" +$PSDefaultParameterValues['*:ErrorAction']='Stop' + +# Go to repo root +$repoRoot = (Resolve-Path "$PSScriptRoot\..").Path +Push-Location $repoRoot + +$null = New-Item build -ItemType Directory -Force +cd build + +cmake -DCMAKE_PREFIX_PATH="$GTestPath" -DSTRING_ENCODING_TYPE=NONE -G "MinGW Makefiles" .. +cmake --build . --config Debug +cp $GTestPath\debug\bin\*.dll tests\Debug +cp Debug\kaitai_struct_cpp_stl_runtime.dll tests\Debug + +Pop-Location diff --git a/.build/build.ps1 b/.build/build-msvc.ps1 similarity index 89% rename from .build/build.ps1 rename to .build/build-msvc.ps1 index c3b0025..fdab4ff 100644 --- a/.build/build.ps1 +++ b/.build/build-msvc.ps1 @@ -1,6 +1,6 @@ <# .DESCRIPTION -Builds Kaitai Struct C++ runtime library and unit tests +Builds Kaitai Struct C++ runtime library and unit tests on Windows using MS Visual C++ (MSVC) Requires: - MSVC native tools installed and available in the command prompt diff --git a/.github/workflows/build.yml b/.github/workflows/build.yml index 07529fc..2678903 100644 --- a/.github/workflows/build.yml +++ b/.github/workflows/build.yml @@ -19,7 +19,7 @@ jobs: - name: unittest run: .build/run-unittest - windows: + windows-msvc: runs-on: windows-latest # NB: see https://github.com/actions/runner-images/blob/main/images/win/Windows2022-Readme.md for what's available in this image steps: @@ -29,6 +29,22 @@ jobs: C:\vcpkg\bootstrap-vcpkg.bat C:\vcpkg\vcpkg install gtest:x64-windows - name: build - run: .build\build.ps1 -GTestPath C:\vcpkg\installed\x64-windows + run: .build\build-msvc.ps1 -GTestPath C:\vcpkg\installed\x64-windows + - name: unittest + run: .build\run-unittest.ps1 + + windows-mingw64: + runs-on: windows-latest + steps: + - uses: actions/checkout@v3 + - uses: egor-tensin/setup-mingw@v2 + with: + platform: x64 + - name: restore + run: | + C:\vcpkg\bootstrap-vcpkg.bat + C:\vcpkg\vcpkg install gtest:x64-windows + - name: build + run: .build\build-mingw.ps1 -GTestPath C:\vcpkg\installed\x64-windows - name: unittest run: .build\run-unittest.ps1 From 12a4eb3873d804cf29ad80eb19a9f3ee1e1ad367 Mon Sep 17 00:00:00 2001 From: Mikhail Yakshin Date: Fri, 16 Jun 2023 17:31:01 +0100 Subject: [PATCH 15/21] No need to install mingw, default GH Actions image have some; added vcpkg instructions to build Mingw-specific build --- .github/workflows/build.yml | 7 ++----- 1 file changed, 2 insertions(+), 5 deletions(-) diff --git a/.github/workflows/build.yml b/.github/workflows/build.yml index 2678903..631d556 100644 --- a/.github/workflows/build.yml +++ b/.github/workflows/build.yml @@ -37,14 +37,11 @@ jobs: runs-on: windows-latest steps: - uses: actions/checkout@v3 - - uses: egor-tensin/setup-mingw@v2 - with: - platform: x64 - name: restore run: | C:\vcpkg\bootstrap-vcpkg.bat - C:\vcpkg\vcpkg install gtest:x64-windows + C:\vcpkg\vcpkg install gtest:x64-mingw-dynamic - name: build - run: .build\build-mingw.ps1 -GTestPath C:\vcpkg\installed\x64-windows + run: .build\build-mingw.ps1 -GTestPath C:\vcpkg\installed\x64-mingw-dynamic - name: unittest run: .build\run-unittest.ps1 From 17c8d124f90047ffdf873fca0465547277fad868 Mon Sep 17 00:00:00 2001 From: Mikhail Yakshin Date: Fri, 16 Jun 2023 17:54:57 +0100 Subject: [PATCH 16/21] Adding MINGW32 define check, assuming it will fix the endian.h problem on Mingw --- kaitai/kaitaistream.cpp | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/kaitai/kaitaistream.cpp b/kaitai/kaitaistream.cpp index 6e9a142..092104d 100644 --- a/kaitai/kaitaistream.cpp +++ b/kaitai/kaitaistream.cpp @@ -9,7 +9,7 @@ #define __BYTE_ORDER BYTE_ORDER #define __BIG_ENDIAN BIG_ENDIAN #define __LITTLE_ENDIAN LITTLE_ENDIAN -#elif defined(_MSC_VER) // !__APPLE__ +#elif defined(_MSC_VER) || defined(MINGW32) #include #define __LITTLE_ENDIAN 1234 #define __BIG_ENDIAN 4321 @@ -17,7 +17,7 @@ #define bswap_16(x) _byteswap_ushort(x) #define bswap_32(x) _byteswap_ulong(x) #define bswap_64(x) _byteswap_uint64(x) -#elif defined(__QNX__) // __QNX__ +#elif defined(__QNX__) #include #include #define bswap_16(x) ENDIAN_RET16(x) @@ -26,7 +26,7 @@ #define __BYTE_ORDER BYTE_ORDER #define __BIG_ENDIAN BIG_ENDIAN #define __LITTLE_ENDIAN LITTLE_ENDIAN -#else // !__APPLE__ or !_MSC_VER or !__QNX__ +#else // !__APPLE__ or !_MSC_VER or !MINGW32 or !__QNX__ #include #include #endif From 88eac90af57c518221a4e43ec6838edebf870118 Mon Sep 17 00:00:00 2001 From: Mikhail Yakshin Date: Fri, 16 Jun 2023 18:00:00 +0100 Subject: [PATCH 17/21] Try another define: __MINGW32__ --- kaitai/kaitaistream.cpp | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/kaitai/kaitaistream.cpp b/kaitai/kaitaistream.cpp index 092104d..eed380f 100644 --- a/kaitai/kaitaistream.cpp +++ b/kaitai/kaitaistream.cpp @@ -9,7 +9,7 @@ #define __BYTE_ORDER BYTE_ORDER #define __BIG_ENDIAN BIG_ENDIAN #define __LITTLE_ENDIAN LITTLE_ENDIAN -#elif defined(_MSC_VER) || defined(MINGW32) +#elif defined(_MSC_VER) || defined(__MINGW32__) #include #define __LITTLE_ENDIAN 1234 #define __BIG_ENDIAN 4321 @@ -26,7 +26,7 @@ #define __BYTE_ORDER BYTE_ORDER #define __BIG_ENDIAN BIG_ENDIAN #define __LITTLE_ENDIAN LITTLE_ENDIAN -#else // !__APPLE__ or !_MSC_VER or !MINGW32 or !__QNX__ +#else // !__APPLE__ or !_MSC_VER or !__MINGW32__ or !__QNX__ #include #include #endif From d3c529f9076d7f72976e5541f84a471ca6b89934 Mon Sep 17 00:00:00 2001 From: Mikhail Yakshin Date: Fri, 16 Jun 2023 18:56:20 +0100 Subject: [PATCH 18/21] Silence compiler warning about src_enc in kaitai::kstream::bytes_to_str() being unused if KS_STR_ENCODING_NONE --- kaitai/kaitaistream.cpp | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/kaitai/kaitaistream.cpp b/kaitai/kaitaistream.cpp index eed380f..a5b69b8 100644 --- a/kaitai/kaitaistream.cpp +++ b/kaitai/kaitaistream.cpp @@ -692,6 +692,10 @@ std::string kaitai::kstream::bytes_to_str(std::string src, std::string src_enc) } #elif defined(KS_STR_ENCODING_NONE) std::string kaitai::kstream::bytes_to_str(std::string src, std::string src_enc) { + // Silence the compiler warning about src_enc being unused - https://stackoverflow.com/a/1486931 + // Sure, it's being unused in this version of this function, but it is obviously used in other implementations + (void) src_enc; + return src; } #else From 0c6e31804d6861abe9e5d2641f60381ba92fdc18 Mon Sep 17 00:00:00 2001 From: Mikhail Yakshin Date: Fri, 16 Jun 2023 19:04:05 +0100 Subject: [PATCH 19/21] Temporarily remove copying DLLs, add debug print to figure out which files mingw make generates --- .build/build-mingw.ps1 | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/.build/build-mingw.ps1 b/.build/build-mingw.ps1 index 70e7c92..5969767 100644 --- a/.build/build-mingw.ps1 +++ b/.build/build-mingw.ps1 @@ -28,7 +28,7 @@ cd build cmake -DCMAKE_PREFIX_PATH="$GTestPath" -DSTRING_ENCODING_TYPE=NONE -G "MinGW Makefiles" .. cmake --build . --config Debug -cp $GTestPath\debug\bin\*.dll tests\Debug -cp Debug\kaitai_struct_cpp_stl_runtime.dll tests\Debug + +Get-ChildItem -Recurse Pop-Location From 9c089323764ca80e13b3ae0fd1f5f7e5405d927e Mon Sep 17 00:00:00 2001 From: Mikhail Yakshin Date: Fri, 16 Jun 2023 19:17:11 +0100 Subject: [PATCH 20/21] Fix MinGW DLL copying + add searching for test executable in 2 different places in run-unittest.ps1 --- .build/build-mingw.ps1 | 16 +++++++++++++--- .build/run-unittest.ps1 | 9 ++++++++- 2 files changed, 21 insertions(+), 4 deletions(-) diff --git a/.build/build-mingw.ps1 b/.build/build-mingw.ps1 index 5969767..842584b 100644 --- a/.build/build-mingw.ps1 +++ b/.build/build-mingw.ps1 @@ -1,9 +1,9 @@ <# .DESCRIPTION -Builds Kaitai Struct C++ runtime library and unit tests on Windows using Mingw +Builds Kaitai Struct C++ runtime library and unit tests on Windows using MinGW Requires: -- Mingw installed and available in the command prompt +- MinGW installed and available in the command prompt - cmake/ctest available - GTest installed, path passed in `-GTestPath` #> @@ -29,6 +29,16 @@ cd build cmake -DCMAKE_PREFIX_PATH="$GTestPath" -DSTRING_ENCODING_TYPE=NONE -G "MinGW Makefiles" .. cmake --build . --config Debug -Get-ChildItem -Recurse +# MinGW Makefiles build generates the following output: +# +# - build/libkaitai_struct_cpp_stl_runtime.dll +# - build/libkaitai_struct_cpp_stl_runtime.dll.a +# - build/tests/unittest.exe +# +# unittest.exe links dynamically against GTest DLLs and `libkaitai_struct_cpp_stl_runtime.dll`, and it +# will need all of them in same directory as .exe to run it, so we copy it. + +cp $GTestPath\debug\bin\*.dll tests +cp libkaitai_struct_cpp_stl_runtime.dll tests\unittest.exe Pop-Location diff --git a/.build/run-unittest.ps1 b/.build/run-unittest.ps1 index aed5f21..47fbc6b 100644 --- a/.build/run-unittest.ps1 +++ b/.build/run-unittest.ps1 @@ -18,6 +18,13 @@ cd build #ctest -C Debug --output-on-failure # Run gtest-generated binary directly, produces more detailed output -./tests/Debug/unittest.exe +if (Test-Path './tests/Debug/unittest.exe') { + ./tests/Debug/unittest.exe +} elseif (Test-Path './tests/unittest.exe') { + ./tests/unittest.exe +} else { + Write-Error "Unable to find the unittest executable." + exit 1 +} Pop-Location From 4633379778fa73d8eca6c03a7fc1ec5e636100db Mon Sep 17 00:00:00 2001 From: Mikhail Yakshin Date: Fri, 16 Jun 2023 23:36:55 +0100 Subject: [PATCH 21/21] Add uploading of build/ as artifacts after completed build for extra manual validation --- .github/workflows/build.yml | 15 +++++++++++++++ 1 file changed, 15 insertions(+) diff --git a/.github/workflows/build.yml b/.github/workflows/build.yml index 631d556..bd80e15 100644 --- a/.github/workflows/build.yml +++ b/.github/workflows/build.yml @@ -16,6 +16,11 @@ jobs: sudo apt-get install -y libgtest-dev - name: build run: .build/build + - uses: actions/upload-artifact@v3 + with: + name: build + path: | + build - name: unittest run: .build/run-unittest @@ -30,6 +35,11 @@ jobs: C:\vcpkg\vcpkg install gtest:x64-windows - name: build run: .build\build-msvc.ps1 -GTestPath C:\vcpkg\installed\x64-windows + - uses: actions/upload-artifact@v3 + with: + name: build + path: | + build - name: unittest run: .build\run-unittest.ps1 @@ -43,5 +53,10 @@ jobs: C:\vcpkg\vcpkg install gtest:x64-mingw-dynamic - name: build run: .build\build-mingw.ps1 -GTestPath C:\vcpkg\installed\x64-mingw-dynamic + - uses: actions/upload-artifact@v3 + with: + name: build + path: | + build - name: unittest run: .build\run-unittest.ps1