[lgrz]

This is a draft. This patch updates a number of submodules, modifies spdlog build config and consolidates some of the build tasks towards a minimal number of Dockerfiles; one for gcc and one clang++.

The trunk.yml and pr.yml "test" tasks should build. Although, I am still in the process of testing other build configurations from the various Pisa options.

Part of #644

[elshize]

@lgrz Did you have a chance to look at the compilation errors? I only had a quick look and it seems like maybe related to catch upgrade, looks as if some standard headers must be included in some tests - but I didn't have much time to have a closer look.

[elshize]

Is there a specific reason to use latest? This will eventually be a different version, and our tests will setup will change and likely break. A good practice is to fix the version, unless we have a compelling reason not to?

[lgrz]

Thanks, no reason to changes this and I will roll this back. I was looking at conflicting docs/examples on gh actions. It also currently triggers a build warning.

[elshize]

Do you know if it's possible to move this to include section, meaning just fully define each pair of compiler/version rather than use the product generation?

The asymmetry is non-intuitive, we have two different ways of defining the same thing.

[lgrz]

I tried logical variations that I thought should make sense, but they were
invalid yaml configurations as far a gh-actions was concerned.

This was rejected by gh-actions...

matrix:
  include:
    - compiler: llvm
      version: 23
    - compiler: gcc
      version: 16

And, also this one...

matrix:
  compiler: []
  version: []
  include:
    - compiler: llvm
      version: 23
    - compiler: gcc
      version: 16

So, now I am beginning to understand that maybe the directory based approach as
the argument to the compiler to build may have made understanding the build
configuration easier.

I agree in that having multiple ways to define the same thing here is ripe for
mis-understanding and error. I will take another and see what can be done for
this. Thanks.

[lgrz]

We can define them all in the matrix and use exclude to remove the invalid combinations. (I prefer whitelist over blacklist though...)

[elshize]

I think bash variables should be in double quotes: -f "$DOCKER_FILE". This should technically not happen here, but it's possible for a variable to evaluate to multiple arguments if there's a space inside. If nothing else, it makes it more difficult to debug when a space is added somewhere by mistake, plus it's a general good practice to do this.

[lgrz]

Thanks, good catch. I will include a fix for this in the next round of changes.

[lgrz]

@lgrz Did you have a chance to look at the compilation errors? I only had a quick look and it seems like maybe related to catch upgrade, looks as if some standard headers must be included in some tests - but I didn't have much time to have a closer look.

Thanks, and sorry I was not very clear on the intention of the status of
the current state of the PR. After submitting the PR I should have added a
detailed comment on the status and intention of the changes.

They are (and were) very much a work-in-progress. For some reason I thought
that getting them onto a PR on this repo would help identify additional
build problems. Sorry for not explaining things properly; and thank you for
taking the time to have a look at the current PR.

Apologies, the PR is half-baked. There are still changes for me to come back and
make another pass on. And, as per some of your review comments, I will wind
back versions of the submodules.

Part of the reason there is a half-baked PR in draft mode, is that there
were quite a few hoops to jump through to get LLVM 23 to build the project.
This started in libfmt, then spdlog, then rapidcheck, then catch2. To
eventually get it working I updated these submodules and then fiddled with
the build config and missing includes.

I have since patched the explicit header includes that are required by recent
versions of LLVM. And, now that I am at this stage, I can see a different
set of changes that can be made to fix the build system while rolling back
the submodules.

I am still working on this and I'm not sure how long it will take and when
I will have blocks of time to get through it. Hopefully in the next week or
two I imagine.

I will take your initial review feedback into account as well, thanks for this.

And, I will try not to trigger any more builds on this PR until it is ready.

[lgrz]

FYI, still working on this one and are closer to having a workable fix for recent versions of clang/llvm.

The work around for getting spdlog to build is to use set(SPDLOG_USE_STD_FORMAT ON). This avoids compile errors in the bundled libfmt that is included with spdlog.

We may need to do something similar for the second version of libfmt which is a direct dependency in pisa.