-
Notifications
You must be signed in to change notification settings - Fork 88
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
Add macos artifact uploading #513
Conversation
I can't check it at the moment, but how does it compare to chocolate-doom-3.1.0.dmg? Since we already have the dsda-launcher, maybe we should bundle it somehow for the macOS release? |
Chocolate has an extremely simplistic launcher for mac, so bundling it in a dmg is fine. Thats what I dont like about bundling source ports as an app bundle. This also makes it possible for the launcher to auto-update the game. |
Oh, I didn't know that. Linux AppImage can still be used as a regular program from the command line, I thought the Mac bundles were the same. |
d92c3ed
to
cb125b7
Compare
Do we need an instructions file about the permissions bundled in as well? |
Yeah added a small instructions file (Quarantine.txt) https://github.com/Pedro-Beirao/dsda-doom/releases/tag/test2-update-script |
It's all ready now |
Maybe |
os: macos-latest | ||
build_type: "Release" | ||
extra_options: "-G Ninja" | ||
shell: bash |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
ecc078a is not correct, do not change the matrix.config
section.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Oh my bad, I used the "Resolve Conflict" button on github. It changed more than what I thought
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Now that Im on my pc, umm whats the problem? Can't I add the macOS x64 Clang
to matrix.config
?
The commit you linked is just github synching stuff, and it is not really part of the PR
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Can't I add the macOS x64 Clang to matrix.config?
You can, but you also changed the syntax to earlier { }
. I've used >-
in MSVC configuration which is not compatible with it.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Fix MSVC build:
diff --git a/.github/workflows/continuous_integration.yml b/.github/workflows/continuous_integration.yml
index e2cea90e6..e28727a7d 100644
--- a/.github/workflows/continuous_integration.yml
+++ b/.github/workflows/continuous_integration.yml
@@ -16,52 +16,49 @@ jobs:
fail-fast: false
matrix:
config:
- - {
- name: "MSVC x64",
- os: windows-latest,
- build_type: "Release",
- extra_options: "-A x64",
- vcpkg_triplet: x64-windows,
- shell: bash,
- }
- - {
- name: "MSYS2 UCRT64",
- os: windows-latest,
- build_type: "Release",
- extra_options: "-G Ninja",
- package_name: "mingw_x64",
- shell: "msys2 {0}",
- msystem: ucrt64,
- msys-env: mingw-w64-ucrt-x86_64,
- artifact-path: build/*.zip,
- }
- - {
- name: "Linux GCC",
- os: ubuntu-latest,
- build_type: "Release",
- extra_options: "-G Ninja -DCMAKE_INSTALL_PREFIX=/usr",
- package_name: "linux_gcc",
- shell: bash,
- artifact-path: build/*.appimage,
- }
- - {
- name: "macOS arm64 Clang",
- os: macos-latest,
- build_type: "Release",
- extra_options: "-G Ninja",
- package_name: "mac_arm64",
- shell: bash,
- artifact-path: build/*.zip,
- }
- - {
- name: "macOS x64 Clang",
- os: macos-13,
- build_type: "Release",
- extra_options: "-G Ninja",
- package_name: "mac_x64",
- shell: bash,
- artifact-path: build/*.zip,
- }
+ - name: "MSVC x64"
+ os: windows-latest,
+ build_type: "Release"
+ vcpkg_triplet: x64-windows
+ shell: pwsh,
+ extra_options: >-
+ -A x64
+ -DCMAKE_TOOLCHAIN_FILE="${env:VCPKG_INSTALLATION_ROOT}\scripts\buildsystems\vcpkg.cmake"
+ -DVCPKG_TARGET_TRIPLET=x64-windows
+
+ - name: "MSYS2 UCRT64"
+ os: windows-latest
+ build_type: "Release"
+ extra_options: "-G Ninja"
+ package_name: "mingw_x64"
+ shell: "msys2 {0}"
+ msystem: ucrt64
+ msys-env: mingw-w64-ucrt-x86_64
+ artifact-path: build/*.zip
+
+ - name: "Linux GCC"
+ os: ubuntu-latest
+ build_type: "Release"
+ extra_options: "-G Ninja -DCMAKE_INSTALL_PREFIX=/usr"
+ package_name: "linux_gcc"
+ shell: bash
+ artifact-path: build/*.appimage
+
+ - name: "macOS arm64 Clang"
+ os: macos-latest
+ build_type: "Release"
+ extra_options: "-G Ninja"
+ package_name: "mac_arm64"
+ shell: bash
+ artifact-path: build/*.zip
+
+ - name: "macOS x64 Clang"
+ os: macos-13
+ build_type: "Release"
+ extra_options: "-G Ninja"
+ package_name: "mac_x64"
+ shell: bash
+ artifact-path: build/*.zip
steps:
- uses: actions/checkout@v4
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I understand now. Done in 21105ca
Sorry about that 😅
rm /usr/local/bin/python3 | ||
rm /usr/local/bin/python3-config | ||
rm /usr/local/bin/python3.12 | ||
rm /usr/local/bin/python3.12-config |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It seems brew install --overwrite python@3.12
is simpler solution. From Homebrew/homebrew-core#173191
All done... sigh Here's also a script I made for auto-updating on mac. Should be easy adaptable for the other OSes |
Thanks 👍 |
https://github.com/Pedro-Beirao/dsda-doom/actions/runs/11133559533
Draft as Im not sure this is the best use of cpack.
I understand that cpack organizes the relevant files as if they were installed, but that isnt really what we need on macos.
Yeah we have App Bundles, but a source port that requires command line parameters on every run shouldnt be packaged that way.
Anyway, since we are not using an App Bundle, and the executable and dylibs are not being codesigned, the users will need to give permissions to every single dylib. So maybe we could include a txt file with a command to do it automatically (
xattr -dr com.apple.quarantine path/to/folder
)Note: with the launcher, the users dont need to give perms like this
CC @rfomin