Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Provide packaging routines / CMake target #135

Open
wants to merge 6 commits into
base: dev
Choose a base branch
from

Conversation

nickelpro
Copy link

@nickelpro nickelpro commented Jun 21, 2023

I was trying to add centurion to my vcpkg registry and was somewhat surprised that it didn't generate INSTALL targets.

So this patch is a first pass at packaging centurion in the "modern" tradition.

Notes:

  • We generate a centurion.pc for anyone still attached to pkg-config
  • We use file sets to ensure the headers are installed and linked correctly. This dodges a minor issue of integration as a subproject vs imported target, as file sets always generate the correct path information.
  • Using file sets bumps the minimum required CMake version to 2.23
  • We provide an alias for centurion::centurion so that subproject/imported target usage is the same
  • Tests and examples use the new target, for demonstration purposes

Possible Future Work:

  • Separate targets that don't require mixer/ttf/image
  • Deprecate the set() vars? They're considered an anti-pattern in some CMake circles

A working portfile using this patch is here (commit).

If you want to test this out, the following vcpkg.json will do it:

{
  "$schema": "https://raw.githubusercontent.com/microsoft/vcpkg-tool/main/docs/vcpkg.schema.json",
  "name": "centurion-test",
  "version": "1.0.0",
  "dependencies": [
    "centurion"
  ],
  "vcpkg-configuration": {
    "default-registry": {
      "kind": "git",
      "baseline": "f6a5d4e8eb7476b8d7fc12a56dff300c1c986131",
      "repository": "https://github.com/microsoft/vcpkg.git"
    },
    "registries": [
      {
        "kind": "git",
        "baseline": "14ff6907873864ae70dd145097894c84da43a292",
        "repository": "https://github.com/nickelpro/vito-reg.git",
        "packages": [
          "centurion"
        ]
      }
    ]
  }
}

Default is shared linkage, also provide -static and -headers-only (bring
your own SDL) targets.

Also ditch the FindModules hanging out in the CMake folder. SDL ships
with appropriate config mode module discovery and CMake ships with
FindModules for SDL components.
@nickelpro
Copy link
Author

Second commit differentiates targets:

  • centurion::centurion-headers-only: Bring your own SDL
  • centurion::centurion: Use shared libraries if they exist, otherwise static
  • centurion::centurion-static: Use static libraries, only available if static libraries exist

This allows the "headers-only" target to be installed even if SDL wasn't
found during configuration, enabling a "bring your own SDL" style of
development for devs who vendor specific SDL versions or binaries in
their codebase.
@nickelpro
Copy link
Author

Third commit makes centurion::centurion and centurion::centurion-static optional, completely removing the find_package(REQUIRED) for SDL. It's possible to configure centurion as a pure headers lib and link in SDL manually from wherever

@nickelpro
Copy link
Author

nickelpro commented Jun 21, 2023

Fourth commit corrects a minor oversight about needing to link sdl2main

Fifth commit contains changes necessary for CI to work:

  • Update Ubuntu hosts before installing dependencies, surprising you didn't run into this before. Upstream dev fails CI for me right now because of this
  • Properly convert env paths to CMake native
  • Drop Windows SDL versions which don't ship a config file. These are annoying but not impossible to test, but better to just test them on *Nix/MacOS. If it's highly desired to support this I'll add a hack.
  • Pass the SDL2*DIR env args to find_package if available

* Update Ubuntu hosts before installing packages
* Convert env paths to CMake native
* Drop SDL versions which don't ship configs on Windows
* Pass locally downloaded SDL packages to find_package(PATHS)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

1 participant