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

Support macOS frameworks; update CI workflows for alr v2(.0.1). #102

Merged
merged 2 commits into from
May 13, 2024

Conversation

simonjwright
Copy link
Contributor

These changes allow users to import SDL2 using one of Homebrew, MacPorts or the framework from libsdl.org, in that order of preference.

For the source-level framework changes, there are two issues:

  • the way that GCC is built means that it doesn't search for frameworks in /Library/Frameworks, so we have to tell it to.
  • the linker options can't have spaces, because they need to be passed to ld as separate arguments. See -Wl,option and -Xlinker option in https://gcc.gnu.org/onlinedocs/gcc/Link-Options.html.

For the CI changes:

  • the version of alr used needs to be updated to v2(.0.1), because v1.2 doesn't understand the new os & distribution choices introduced for macOS.
  • the macOS workflow needs to run on macos-12; macos-latest now runs on aarch64, and macos-13 uses Xcode 15, with consequent linking problems.

These changes allow users to import SDL2 using one of Homebrew,
MacPorts or the framework from libsdl.org, in that order of
preference.

For the framework changes, there are two issues:
* the way that GCC is built means that it doesn't search for frameworks in
  /Library/Frameworks, so we have to tell it to.
* the linker options can't have spaces, because they need to be passed to
  ld as separate arguments. See -Wl,option and -Xlinker option in
  https://gcc.gnu.org/onlinedocs/gcc/Link-Options.html.

For the CI changes:
* the version of alr used needs to be updated to v2(.0.1), because v1.2
  doesn't understand the new os & distribution choices introduced for
  macOS.
* the macOS workflow needs to run on macos-12; macos-latest now runs on
  aarch64, and macos-13 uses Xcode 15, with consequent linking problems.

  * alire.toml (gpr-set-externals): if the OS is macos, set SDL_PLATFORM
      o if 'brew' is on the PATH, to macos_homebrew
      o if not, but 'port' is, to macos_ports
      o if neither, to macosx
  * src/link/macosx/sdl_linker.ads (Options): use the "-Wl," style to add
      3 parameters:
      o -F/Library/Frameworks
      o -framework
      o -SDL2

  * .github/workflows/ci-linux.yml
    (setup-alire): use setup-alire@v3.
    (Install toolchain): 'config' is deprecated, use 'settings'.
      Remove the 'toolchain --install' lines (option not available).
  * .github/workflows/ci-windows.yaml: likewise.
  * .github/workflows/ci-macos.yml: likewise.
    (runs-on): change to macos-12.
@Lucretia
Copy link
Collaborator

Lucretia commented May 3, 2024

I would expect that there fore 3 different workflow files for macos, one for each way of building, mp, hb or fw.

@simonjwright
Copy link
Contributor Author

That’s a good plan. I’ve started on it, hb/mp no problem, can get fw to work; I do see some issues.

In build/gnat/sdlada.gpr

  • there’s -DREENTRANT. I don’t see that in sdl2-config; it’s not in the hb version (I haven’t tried mp), and I just built from source (2.30.3); it’s not there either. What there is is -D_THREAD_SAFE.
  • -DSDL_HOMEBREW, -DSDL_MACPORTS don’t seem to be used anywhere.
  • build/gnat/sdlada_common.gpr hasn’t been updated for recent changes, and is only used in build/gnat/tests_mixer.gpr (which, likewise).

You can see where I’ve got to here - there are 9 commits on top of the PR you’ve already seen, which I’d merge-squash before submitting. The CI runs, for just this set of changes, are here.


Would you prefer to merge this PR, and add the new CI changes in another PR? or, include the CI changes in this PR.


I feel that the CI builds would be more useful if they actually did a link. Wouldn’t need to do much, perhaps just print the version info.

@simonjwright
Copy link
Contributor Author

Actually, this is a better link to see the work-in-progress.

@Lucretia
Copy link
Collaborator

Lucretia commented May 5, 2024

I feel that the CI builds would be more useful if they actually did a link.

That's why there's the separated out test progs repo.

@Lucretia
Copy link
Collaborator

Lucretia commented May 5, 2024

Actually, this is a better link to see the work-in-progress.

Is there a reason why you're not using the github sdl2 framework actions?

@simonjwright
Copy link
Contributor Author

That's why there's the separated out test progs repo.

Looking at it again, it’d be easy to run the unit tests. Anyway, it’s your call.

@simonjwright
Copy link
Contributor Author

Actually, this is a better link to see the work-in-progress.

Is there a reason why you're not using the github sdl2 framework actions?

I wasn’t aware of it. It works just fine (so long as you don’t spell 'frameworks' 'framewworks', anyway). The only slight difficulty is it installs in ~/Library/Frameworks, not our expected /Library/Frameworks. I suppose we could add both paths? That would give users more freedom.

@Lucretia
Copy link
Collaborator

Lucretia commented May 5, 2024

That's why there's the separated out test progs repo.

Looking at it again, it’d be easy to run the unit tests. Anyway, it’s your call.

Well, I separated them out just to make things a bit easier. The base libs all have workflows in them to test building normally. But the test progs should really be used to test that linking works on all platforms.

@Lucretia
Copy link
Collaborator

Lucretia commented May 5, 2024

Actually, this is a better link to see the work-in-progress.

Is there a reason why you're not using the github sdl2 framework actions?

I wasn’t aware of it. It works just fine (so long as you don’t spell 'frameworks' 'framewworks', anyway). The only slight difficulty is it installs in ~/Library/Frameworks, not our expected /Library/Frameworks. I suppose we could add both paths? That would give users more freedom.

You mean that action installs to ~? Yeah, it's default is /Users/runner/Library/Frameworks, but that can be changed.

The macOS workflows use actions/checkout@v4, alire-project/setup-alire@v3.

The Macports CI disables Homebrew and installs Macports.

The Frameworks CI disables Homebrew. It uses the
BrettDong/setup-sdl2-frameworks action to retrieve the latest SDL2.framework,
and copies it to /Library/Frameworks, where this crate expects it.

  * .github/workflows/ci-macos-hb.yml: renamed from ci-macos.yml.
      Uses Homebrew.
  * .github/workflows/ci-macos-mp.yml: new. Uses MacPorts.
  * .github/workflows/ci-macos.yml: renamed to ci-macos-hb.yml.
  * .github/workflows/ci-macos-fw.yml: new. Uses SDL2.framework.
@simonjwright
Copy link
Contributor Author

Is there a reason why you're not using the github sdl2 framework actions?

I wasn’t aware of it. It works just fine (so long as you don’t spell 'frameworks' 'framewworks', anyway). The only slight difficulty is it installs in ~/Library/Frameworks, not our expected /Library/Frameworks. I suppose we could add both paths? That would give users more freedom.

Turns out that adding both paths can’t be done with the current design of sdl_linker.ads - we’d have to include $HOME somehow, I don’t think that can be done while the package is Pure.

I’m about to add a commit with the hb, mp, and fw CI scripts.

@Lucretia
Copy link
Collaborator

Lucretia commented May 6, 2024

Is there a reason why you're not using the github sdl2 framework actions?

I wasn’t aware of it. It works just fine (so long as you don’t spell 'frameworks' 'framewworks', anyway). The only slight difficulty is it installs in ~/Library/Frameworks, not our expected /Library/Frameworks. I suppose we could add both paths? That would give users more freedom.

Turns out that adding both paths can’t be done with the current design of sdl_linker.ads - we’d have to include $HOME somehow, I don’t think that can be done while the package is Pure.

I don't know what you mean? link/macos is for framework, link/nix is *nix-like which should cover hb and mp.

I’m about to add a commit with the hb, mp, and fw CI scripts.

@simonjwright
Copy link
Contributor Author

simonjwright commented May 6, 2024

I don't know what you mean? link/macos is for framework, link/nix is *nix-like which should cover hb and mp.

I should have said, I’m talking about the macOS version.

sdl.ads says pragma Linker_Options (SDL_Linker.Options); which forces a single string; it’d have to be something like

with Ada.Environment_Variables;

package SDL_Linker is
   pragma Pure;
   
   HOME : constant String := 
     Ada.Environment_Variables.Value ("HOME");                  -- impure
   
   Options : constant String := 
     "-Wl,"
     & "-F/Library/Frameworks,"
     & "-F" & HOME & "/Library/Frameworks,"
     & "-framework,SDL2";
end SDL_Linker;

which would probably be OK if it wasn’t for pragma Pure;, which we have to retain because sdl.ads is marked Pure.

I did wonder about preprocessing, but clang won’t accept the necessary switch: I got it to work with HOME : constant String := $HOME; and this unfriendly command line

alr build -- -cargs:ada -gnateDHOME=\"$HOME\" -cargs:c -F$HOME/Library/Frameworks

but on the whole I’d prefer to just insist on /Library/Frameworks.

@Lucretia
Copy link
Collaborator

Lucretia commented May 6, 2024

Can't we just force it use the frameworks installed in a system directory? The action can install it to a system dir, I think, well you can set the location. IIRC, the SDL docs tell you to install it into the system directory.

@simonjwright
Copy link
Contributor Author

There are other framework directories (e.g. /System/Library/Frameworks) but /Library/Frameworks is the nearest we have to a system directory accessible to us punters.

(I think that /System/Library/Frameworks is in fact a link to somewhere in the SDK, so writing in there would get lost on SDK update)

@Lucretia Lucretia merged commit 48c775c into ada-game-framework:master May 13, 2024
5 checks passed
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.

2 participants