Skip to content

Conversation

@mabruzzo
Copy link
Collaborator

This PR fixes a significant bug in GrackleConfig.cmake that causes applications with CMake-build systems that expect to link against free-standing installations of Grackle (i.e. Grackle is pre-built) to break when using the newest versions of CMake (version 4.0 or newer).

Background

As part of the installation-process, the CMake build-system installs a file called GrackleConfig.cmake alongside the Grackle headers and libraries. This is a special kind CMake script that a build-system searches for and reads when you want to building a downstream application that links against a free-standing Grackle installation. Basically, the script contains queryable metadata about how Grackle was configured and the necessary linker/compilation flags.

While we require version 3.16 of CMake or newer to actually build/install the core Grackle library, I followed standard convention and tried to make it possible for the GrackleConfig.cmake file to be used by CMake 3.3 (basically as early a version of CMake as possible).

  • Essentially, the idea is that the version used to compile/package a version of Grackle may exceed the version of CMake currently installed on somebody's machine. (This isn't a significant consideration for us right now, but it's worth thinking about for the future)
  • To put it another way, since the GrackleConfig.cmake file is a really simple script, it wasn't hard to use a subset of the language that was compatible with older versions of the lanugage.

The Problem

I mistakenly used the cmake_minimum_version command within GrackleConfig.cmake to ensure that the file wasn't read by a version of CMake older than 3.3.

Everything seemed to work fine at first. I only realized this was an issue today. It turns out that with the release of CMake 4.0 (released at the end of March), that calling cmake_minimum_version to try enforce a minimum version before version 3.5 now causes CMake to abort with an error.

Consequently, people can use CMake 4.0 (or newer) to perform a full standalone installation of Grackle. However, if they then turn around and try to link against that version of Grackle (with that same CMake version), CMake will abort with an error. It's worth noting that doing an embedded install of Grackle (e.g. compiling Grackle as part of the downstream application) using CMake won't cause any problems, but the scenario I highlighted is an extremely plausible way of using CMake.

The Solution

Fixing this is easy. We simply delete the call to
cmake_minimum_version in the GrackleConfig.cmake file. For whatever reason I had previously written a separate redundant check enforcing the minimum CMake version that we can leave in place.

Ramifications

I really think we should consider making a 3.4.1 release of grackle with this bugfix. This is only going to be a bigger problem as time goes on (and people start using newer versions of CMake)

This PR fixes a significant bug in **GrackleConfig.cmake** that causes
applications with CMake-build systems that expect to link against
free-standing installations of Grackle (i.e. Grackle is pre-built) to
break when using the newest versions of CMake (version 4.0 or newer).

Background
----------
As part of the installation-process, the CMake build-system installs a
file called **GrackleConfig.cmake** alongside the Grackle headers and
libraries. This is a special kind CMake script that a build-system
searches for and reads when you want to building a downstream
application that links against a free-standing Grackle installation.
Basically, the script contains queryable metadata about how Grackle was
configured and the necessary linker/compilation flags.

While we require version 3.16 of CMake or newer to actually
build/install the core Grackle library, I followed standard convention
and tried to make it possible for the GrackleConfig.cmake file to be
used by CMake 3.3 (basically as early a version of CMake as possible).
- Essentially, the idea is that the version used to compile/package a
  version of Grackle may exceed the version of CMake currently installed
  on somebody's machine. (This isn't a significant consideration for us
  right now, but it's worth thinking about for the future)
- To put it another way, since the GrackleConfig.cmake file is a really
  simple script, it wasn't hard to use a subset of the language that was
  compatible with older versions of the lanugage.

The Problem
-----------
I mistakenly used the `cmake_minimum_version` command within
**GrackleConfig.cmake** to ensure that the file wasn't read by a version
of CMake older than 3.3.

Everything seemed to work fine at first. I only realized this was an
issue today. It turns out that with the release of CMake 4.0 (released
at the end of March), that calling `cmake_minimum_version` to try
enforce a minimum version before version 3.5 now causes CMake to abort
with an error.

Consequently, people can use CMake 4.0 (or newer) to perform a full
standalone installation of Grackle. However, if they then turn around
and try to link against that version of Grackle (with that same CMake
version), CMake will abort with an error. It's worth noting that doing
an embedded install of Grackle (e.g. compiling Grackle as part of the
downstream application) using CMake won't cause any problems, but the
scenario I highlighted is an extremely plausible way of using CMake.

The Solution
------------
Fixing this is easy. We simply delete the call to
`cmake_minimum_version` in the **GrackleConfig.cmake** file. For
whatever reason I had previously written a separate redundant check
enforcing the minimum CMake version that we can leave in place.

Ramifications
-------------
I really think we should consider making a 3.4.1 release of grackle with
this bugfix. This is only going to be a bigger problem as time goes on
(and people start using newer versions of CMake)
@mabruzzo mabruzzo added bug Something isn't working build-system Related to the build-system labels May 17, 2025
@brittonsmith
Copy link
Contributor

I'll merge this now. I agree we should probably issue a quick bugfix release.

@brittonsmith brittonsmith merged commit 32a5cd3 into grackle-project:main May 19, 2025
5 checks passed
@mabruzzo mabruzzo deleted the cmake-minimum_version-bugfix branch May 23, 2025 17:35
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

bug Something isn't working build-system Related to the build-system

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants