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

Github Actions Automated Builds #55

Closed
wants to merge 26 commits into from
Closed

Github Actions Automated Builds #55

wants to merge 26 commits into from

Conversation

WingofaGriffin
Copy link

Draft of adding automated builds via Github actions. Linux should work as expected, but Windows is having issues completing the build with the current readme instructions:

Src/OSD/Windows/DirectInputSystem.h:36:10: fatal error: SDL.h: No such file or directory

@trzy are the current instructions up to date still? Looks like SDL is not being properly found even with the path being added. Then again, I am not a Windows dev at all, so this is all anecdotal.

For reference, this will trigger a new build that will be hosted in Github actions as an artifact every time a commit or PR is made to the master branch. You can change that frequency, or even move them to releases instead with https://github.com/softprops/action-gh-release

Reference to: #52 (comment)

@trzy
Copy link
Owner

trzy commented Dec 29, 2022

Thanks, let me find some time this weekend to take a closer look. Re: Windows, the instructions haven't changed, but the issue is probably that the SDL header files are not in the expected location (a hard-coded path in the Makefile). This forum post has some more information: http://supermodel3.com/Forum/viewtopic.php?f=3&t=2203&p=20684#p20676

@WingofaGriffin
Copy link
Author

Checking in to see if you had a chance to take a look at this yet. I think it probably makes sense to update the Makefile so the windows build can work.

@trzy
Copy link
Owner

trzy commented Jan 10, 2023

Hey sorry I've been absolutely swamped and may not be able to look at it this weekend until Sunday night or Monday. But I would like to get to this. What's a good way to test this?

@WingofaGriffin
Copy link
Author

Probably to make a temporary fork tbh and testing it there and deleting when you're done. I did it by making a fork I plan on deleting once this is merged in, and it seems to be the simplest way to test without clogging up the main repo. It's completely free, so shouldn't be an issue.

@trzy
Copy link
Owner

trzy commented Jan 23, 2023

Thanks for bearing with me on this. I appreciate the work you've put into this. I've just been incredibly busy :( However, I would like to get this over the finish line soon. A couple of things will be necessary to figure out:

  1. Is there a way to readily fetch the artifacts and turn them into release packages on the project page?
  2. Is there a way to readily fetch the artifacts via a script to post them on the Supermodel download page? Eventually, the download page may become superfluous. But for now, I can continue running a task on one of my Windows PCs to pull the latest artifacts and post them.
  3. Need to bundle the builds appropriately, the way the release script currently does. That means packaging the README.txt and various sub-directories, etc. This is probably quite simple to do and would just involve adding a few more commands to run.

Now, re: the Windows build, I see what the issue is. I did a test and had it execute sdl2-config to figure out what was going on:

Run echo "/mingw64/bin" >> $GITHUB_PATH
-ID:/a/_temp/msys64/mingw64/include/SDL2 -Dmain=SDL_main
-LD:/a/_temp/msys64/mingw64/lib -lmingw32 -mwindows -lSDL2main -lSDL2

What is the best way to handle this? I think it may be possible to write a shell command to extract the -I part. Then, an option can be added specifically to Makefile.Win32 that if detected, will run sdl2-config and extract the include directory. Likewise for the library directory.

What do you think? I can probably figure this out myself but would like your thoughts.

@trzy
Copy link
Owner

trzy commented Jan 23, 2023

I tested the adjustments and they work. Makefile.Win32 needs to be changed to allow the SDL2 include and lib paths to be overridden:

#
# Path to SDL2
#
ifndef SDL2_INCLUDE_DIR
  SDL2_INCLUDE_DIR = \msys64\mingw64\include\SDL2
endif
ifndef SDL2_LIB_DIR
  SDL2_LIB_DIR = \msys64\mingw64\lib
endif

Then, the Windows action should look like this:

name: make-windows

on:
  push:
    branches: [ "master" ]
  pull_request:
    branches: [ "master" ]

jobs:
  build:

    runs-on: windows-latest

    steps:
    - uses: actions/checkout@v3
    - uses: msys2/setup-msys2@v2
      with:
        update: true
        install: >-
          mingw64/mingw-w64-x86_64-gcc
          mingw64/mingw-w64-x86_64-make
          mingw64/mingw-w64-x86_64-SDL2
          mingw64/mingw-w64-x86_64-SDL2_net
    - name: Update Path
      shell: msys2 {0}
      run: |
          echo "/mingw64/bin" >> $GITHUB_PATH
    - name: make
      shell: msys2 {0}
      run: |
         #
          # We need to run sdl2-config to extract the SDL2 include and library paths. We cannot use
          # the output of sdl2-config directly because we do not want to replace main w/ SDL_main.
          #
          # In bash, (x) interprets the space-delimited string x as an array. Parameter expansion,
          # ${var:N:L} extracts L characters from offset N in var. If L is ommitted, all characters
          # up until the end of the sequence are retained.
          #
          # The below code defines SDL2_INCLUDE_DIR and SDL2_LIB_DIR and these can be passed to the
          # Win32 Makefile.
          #
          SDL2_CONFIG_CFLAGS=(`sdl2-config --cflags`)
          SDL2_CONFIG_LIBS=(`sdl2-config --libs`)
          SDL2_INCLUDE_DIR=${SDL2_CONFIG_CFLAGS[0]:2}
          SDL2_LIB_DIR=${SDL2_CONFIG_LIBS[0]:2}
          echo $SDL2_INCLUDE_DIR
          echo $SDL2_LIB_DIR
          mingw32-make -f Makefiles/Makefile.Win32 SDL2_INCLUDE_DIR=$SDL2_INCLUDE_DIR SDL2_LIB_DIR=$SDL2_LIB_DIR
    - uses: actions/upload-artifact@v3
      with:
        name: supermodel
        path: bin64/supermodel

Still more work to do. Most importantly:

  1. Add commands to produce a release package .zip file (I can do this) both here and in the Linux version.
  2. Figure out how to at least turn these into proper packages that are released?

@WingofaGriffin
Copy link
Author

Feel free to push those to this PR, as you should have final say on how the build process works.

RE: Github releases, that can be done with a helper function, such as https://github.com/softprops/action-gh-release (there are a bunch of others if this doesn't work)

So, either along with or instead of upload-artifact, we can create a release. Now to get that on your website, we can either just direct people to the github releases (and also save in the amount of files you need to host), or just add in more tools to automate that such as a GCP/AWS/Azure/Etc addon.

@trzy
Copy link
Owner

trzy commented Feb 11, 2023

Sorry this is taking so long. Haven't had any time to look at it (juggling 4 other projects, so context switches are difficult). But I intend to look at it early next week again!

If GitHub releases end up working well, we can indeed just have the download page link here. That's a solid idea.

@WingofaGriffin
Copy link
Author

Not an issue. You will probably want to tweak the activation logic (unless you are only pushing to Main once things are ready for release), but besides that it shouldn't be much more of a push to get it across the finish line. Let me know if you need further help!

@WingofaGriffin
Copy link
Author

Hi @trzy !

The flatpak somebody made for this project seems to be pretty broken making me encouraged to bring this back up for automated builds. Any time to try to get this done?

@trzy
Copy link
Owner

trzy commented Jan 29, 2024 via email

@WingofaGriffin
Copy link
Author

Apologies for the delay, I have added you.

@WingofaGriffin
Copy link
Author

Duplicated by #75

Blocked by #55

@WingofaGriffin
Copy link
Author

Closing in favor of #75

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