From d07deb4cbbaea4c9e70fe4dd59de30fd09f31733 Mon Sep 17 00:00:00 2001
From: Petr Pucil <petr.pucil@seznam.cz>
Date: Mon, 11 Mar 2024 17:16:33 +0100
Subject: [PATCH] cpp_builder: remove workaround of Boost 1.62 bug, drop JUnit
 support

This drops support for Boost 1.62, but I don't see much value in
supporting this broken version specifically, given that none of the
`cpp_stl` targets use it at the moment. As of March 2024, we're using
these versions:

* Boost 1.54.0 - in `{clang3.4,gcc4.8}-linux-x86_64` targets, since they
  both run on Ubuntu 14.04 (see
  https://github.com/kaitai-io/kaitai_struct_docker_images/blob/ef0ad6e3/src/cpp_stl/gcc4.8-linux-x86_64/Dockerfile#L1),
  which comes with this version of Boost:
  https://launchpad.net/ubuntu/trusty/+source/boost-defaults
* Boost 1.71.0 - in `msvc141-windows-x64`
  (https://ci.appveyor.com/project/kaitai-io/ci-targets/builds/49319128/job/282j67vaxhklm2o4?fullLog=true#L120)
* Boost 1.74.0 - in `{clang11,gcc11}-linux-x86_64`, since they both run
  on Ubuntu 22.04 (see
  https://github.com/kaitai-io/kaitai_struct_docker_images/blob/ef0ad6e3/src/cpp_stl/gcc11-linux-x86_64/Dockerfile#L1),
  which comes with this version of Boost:
  https://launchpad.net/ubuntu/jammy/+source/boost-defaults
* Boost 1.84.0 - in `clang14-macos-x86_64`

On top of that, the documented workaround of the bug (`--log_sink`
broken in Boost v1.62) in the code inadverently changed the log format
to JUnit (`--logger=JUNIT,...`), which is apparently why additional code
had to be added in 5d2125c50197de2b32727bdaf26fb23f72ac3fb6 to support
JUnit test result format in addition to the Boost-specific XML format.

This part is definitely unnecessary - there's no reason why we should
support two log formats, we should choose one. Even in Boost 1.62 the
XML format was definitely available as well, though it would have to be
specified as `--logger=XML,...` instead of `--log_format=XML` (see
https://web.archive.org/web/20221205203459/https://svn.boost.org/trac10/ticket/12507).
It looks like the reason the workaround uses JUnit is that the
`--logger=JUNIT,...` option was copied from the [Stack Overflow
answer](https://stackoverflow.com/a/39999085/487064) mentioned in the
code comment without thinking.

Fun fact: apparently, there hasn't been a single run using Boost 1.62
since December 2018, because there isn't any occurrence of `<testcase`
(typical for JUnit format) in the
https://github.com/kaitai-io/ci_artifacts repo under
`test_out/cpp_stl{_98,_11}/`. You can check this by cloning
https://github.com/kaitai-io/ci_artifacts locally and running this
command:

```
git log --stat -G '<testcase' --all -- test_out/cpp_stl{_98,_11}/
```

This yields no results, meaning that this workaround predates the
migration of C++ to the "new" modular CI at ci.kaitai.io
(https://github.com/kaitai-io/kaitai_struct/issues/62) and has never
been used since then.
---
 aggregate/boost_test_parser.rb | 47 ----------------------------------
 builder/cpp_builder.rb         | 20 +--------------
 2 files changed, 1 insertion(+), 66 deletions(-)

diff --git a/aggregate/boost_test_parser.rb b/aggregate/boost_test_parser.rb
index 1aee49f7f..e605bf6b4 100644
--- a/aggregate/boost_test_parser.rb
+++ b/aggregate/boost_test_parser.rb
@@ -8,14 +8,6 @@ def initialize(fn)
   end
 
   def each_test
-    # Boost actually uses two slightly different formats: old and
-    # new. Instead of trying to detect which one we have, we actually
-    # just try both.
-    each_from_old_boost_format { |tr| yield tr }
-    each_from_new_boost_format { |tr| yield tr }
-  end
-
-  def each_from_old_boost_format
     @doc.root.elements.each('TestSuite') { |ts|
       ts.elements.each('TestCase') { |tc|
         name = tc.attribute('name').value
@@ -54,43 +46,4 @@ def each_from_old_boost_format
       }
     }
   end
-
-  def each_from_new_boost_format
-    @doc.root.elements.each('testcase') { |tc|
-      name = tc.attribute('name').value
-
-      raise "Unable to parse name: \"#{name}\"" unless name =~ /^test_(.*?)$/
-      name = underscore_to_ucamelcase($1)
-
-      failures = []
-      tc.elements.each('failure') { |err|
-        failures << TestResult::Failure.new(
-          nil, # filename
-          nil, # line
-          err.text,
-          nil
-        )
-      }
-
-      tc.elements.each('error') { |err|
-        failures << TestResult::Failure.new(
-          nil, # filename
-          nil, # line
-          err.text,
-          nil
-        )
-      }
-
-      if failures.empty?
-        status = :passed
-        failure = nil
-      else
-        status = :failed
-        failure = failures[0]
-      end
-
-      tr = TestResult.new(name, status, tc.attribute('time').value.to_f, failure)
-      yield tr
-    }
-  end
 end
diff --git a/builder/cpp_builder.rb b/builder/cpp_builder.rb
index 6054b3508..d0feafc16 100644
--- a/builder/cpp_builder.rb
+++ b/builder/cpp_builder.rb
@@ -306,24 +306,6 @@ def file_to_test(filename)
   def run_tests
     xml_log = "#{@abs_cpp_test_out_dir}/results.xml"
 
-    # Work around boost v1.62 bug: https://svn.boost.org/trac10/ticket/12507
-    # --log_sink is broken in boost v1.62, using workaround, as per
-    # https://stackoverflow.com/a/39999085/487064
-    #
-    # However, Travis has boost v1.54, which has problems with it, so we
-    # won't use the workaround on anything except exactly v1.62
-
-    boost_log_option = "--log_sink=#{xml_log}"
-
-    begin
-      if File.read('/usr/include/boost/version.hpp') =~ /BOOST_VERSION 106200/
-        # Boost v1.62 detected, enabling workaround
-        boost_log_option = "--logger=JUNIT,test_suite,#{xml_log}"
-      end
-    rescue Errno::ENOENT => e
-      # ignore
-    end
-
     # Choose test executable
     case @mode
     when :msbuild_windows
@@ -339,7 +321,7 @@ def run_tests
     tests_cli = [
       ks_tests_bin,
       '--log_format=XML',
-      boost_log_option,
+      "--log_sink=#{xml_log}",
       '--log_level=all',
       '--report_level=detailed',
     ]