-
Notifications
You must be signed in to change notification settings - Fork 219
1. Fix linux + mac builds #79
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
Open
culix-7
wants to merge
22
commits into
SourMesen:master
Choose a base branch
from
culix-7:test-build-master
base: master
Could not load branches
Branch not found: {{ refName }}
Loading
Could not load tags
Nothing to show
Loading
Are you sure you want to change the base?
Some commits from the old base branch may be removed from the timeline,
and old review comments may become outdated.
Open
+172
−17
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
use path to refer to project without cd saves us from cd errors later
052d5bf to
961571b
Compare
a445a2b to
36d0ef4
Compare
it's easier to build and view in a separate script no functional changes
macOS 13 images have been officially turned off by GitHub actions/runner-images#13046
flag should work by itself; is not expecting a value the current version works fine on a local command line, but not in the cloud when parsed through 'make'
We don't want any PRs trying to sign themselves as an official, merged Mesen build but they would all fail anyway because PRs don't have access to the signing keys (only the main repo has that, after PRs are merged)
more flexible if the path changes
some mac builds are passing with the new find command, but others are failing bc of the `UNAME` value ``` (clang, macos-14, arm64) works: >Found app at: bin/osx-arm64/Release/osx-arm64/publish/Mesen.app (clang_aot, macos-14, arm64) works: >Found app at: bin/osx-arm64/Release/osx-arm64/publish/Mesen.app (clang, macos-14, x64) failed: >find: bin/osx-x64: No such file or directory (clang_aot, macos-14, x64) failed: >find: bin/osx-x64: No such file or directory ``` add a make test so we can see what is happening and fix/enforce it currently: ``` Checking Makefile architecture detection logic... ------------------------------------------------ Input: Linux-x86_64 | Expected Platform: linux-x64 | Expected Arch Flag: -m64 Input: Linux-aarch64 | Expected Platform: linux-arm64 | Expected Arch Flag: Input: Darwin-x86_64 | Expected Platform: osx-x64 | Expected Arch Flag: -m64 Input: Darwin-arm64 | Expected Platform: osx-arm64 | Expected Arch Flag: -m64 Input: Darwin-aarch64 | Expected Platform: osx-arm64 | Expected Arch Flag: -m64 ------------------------------------------------ ``` Right now this just prints them. need to refactor the makefile a bit to actually test it that will be in the next commit
use ?= for setting uname and machine, so we can test our own makefile now it gives: ``` $ make test-all-configs Validating makefile architecture detection: ---------------------------------------------------------------------------------------------------------- INPUT | EXPECTED (PLAT/FLAGS) | ACTUAL (PLAT/FLAGS) | RESULT ---------------------------------------------------------------------------------------------------------- Linux-x86_64 | linux-x64 -m64 | linux-x64 -m64 | PASS Linux-aarch64 | linux-arm64 | linux-arm64 | PASS Darwin-x86_64 | osx-x64 -m64 | osx-x64 -m64 | PASS Darwin-arm64 | osx-arm64 -m64 | osx-arm64 -m64 | PASS Darwin-aarch64 | osx-arm64 -m64 | osx-arm64 -m64 | PASS ---------------------------------------------------------------------------------------------------------- ``` manual test: I can prove this works by changing any of the `MESENPLATFORM :=` lines e.g. `MESENPLATFORM := $(MESENOS)-bad!` gives ``` ---------------------------------------------------------------------------------------------------------- INPUT | EXPECTED (PLAT/FLAGS) | ACTUAL (PLAT/FLAGS) | RESULT ---------------------------------------------------------------------------------------------------------- Linux-x86_64 | linux-x64 -m64 | linux-bad! -m64 | FAIL ```
1. verify-all-env: you can run this in your shell to test and check that the current Makefile logic works or not ``` ---------------------------------------------------------------------------------------------------------- INPUT | EXPECTED (PLAT/FLAGS) | ACTUAL (PLAT/FLAGS) | RESULT ---------------------------------------------------------------------------------------------------------- Linux-x86_64 | linux-x64 -m64 | linux-x64 -m64 | PASS Linux-aarch64 | linux-arm64 | linux-arm64 | PASS Darwin-x86_64 | osx-x64 -m64 | osx-x64 -m64 | PASS Darwin-arm64 | osx-arm64 -m64 | osx-arm64 -m64 | PASS Darwin-aarch64 | osx-arm64 -m64 | osx-arm64 -m64 | PASS ---------------------------------------------------------------------------------------------------------- ``` I can manually test and verify: if I change a `MESENPLATFORM` line, it breaks and prints `FAIL` instead ``` Linux-x86_64 | linux-x64 -m64 | linux-x64-bad! -m64 | FAIL ``` 2. verify-env: we will hook this up in the build.yml shortly so we can prove that we are doing the right thing in the build
test what we are actually building on
the tests now correctly detect this failure happening on macos ``` ---------------------------------------------------------------------------------------------------------- INPUT | EXPECTED (PLAT/FLAGS) | ACTUAL (PLAT/FLAGS) | RESULT ---------------------------------------------------------------------------------------------------------- Linux-x86_64 | linux-x64 -m64 | linux-x64 -m64 | PASS Linux-aarch64 | linux-arm64 | linux-arm64 | PASS Darwin-x86_64 | osx-x64 -m64 | osx-x64 -m64 | PASS Darwin-arm64 | osx-arm64 -m64 | osx-arm64 -m64 | PASS Linux-x86_64 | linux-arm64 | linux-x64 -m64 | FAIL make[1]: *** [makefile:309: test-env-row] Error 1 Darwin-arm64 | osx-x64 -m64 | osx-arm64 -m64 | FAIL make[1]: *** [makefile:309: test-env-row] Error 1 Linux-x86_64 | linux-arm64 | linux-x64 -m64 | FAIL make[1]: *** [makefile:309: test-env-row] Error 1 ---------------------------------------------------------------------------------------------------------- ```
with this change, the tests now pass ``` ---------------------------------------------------------------------------------------------------------- INPUT | EXPECTED (PLAT/FLAGS) | ACTUAL (PLAT/FLAGS) | RESULT ---------------------------------------------------------------------------------------------------------- Linux-x86_64 | linux-x64 -m64 | linux-x64 -m64 | PASS Linux-aarch64 | linux-arm64 | linux-arm64 | PASS Darwin-x86_64 | osx-x64 -m64 | osx-x64 -m64 | PASS Darwin-arm64 | osx-arm64 -m64 | osx-arm64 -m64 | PASS Linux-aarch64 | linux-arm64 | linux-arm64 | PASS Darwin-x86_64 | osx-x64 -m64 | osx-x64 -m64 | PASS Linux-aarch64 | linux-arm64 | linux-arm64 | PASS ---------------------------------------------------------------------------------------------------------- Verification Complete. All tests PASSED. ```
keep all of the args for each test in the same order, so it is easier to visually see: UNAME MARCHINE E_PLAT ARCH makefiles are already ugly enough. do what we can to keep it clean verify-env still passes: ``` make verify-all-env Validating Makefile Architecture Detection Logic: ---------------------------------------------------------------------------------------------------------- INPUT | EXPECTED (PLAT/FLAGS) | ACTUAL (PLAT/FLAGS) | RESULT ---------------------------------------------------------------------------------------------------------- Linux-x86_64 | linux-x64 -m64 | linux-x64 -m64 | PASS Linux-aarch64 | linux-arm64 | linux-arm64 | PASS Darwin-x86_64 | osx-x64 -m64 | osx-x64 -m64 | PASS Darwin-arm64 | osx-arm64 -m64 | osx-arm64 -m64 | PASS Linux-aarch64 | linux-arm64 | linux-arm64 | PASS Darwin-x86_64 | osx-x64 -m64 | osx-x64 -m64 | PASS Linux-aarch64 | linux-arm64 | linux-arm64 | PASS ---------------------------------------------------------------------------------------------------------- ``
the new tests now pass
don't try to do fancy sub-folder guessing. if we get an architecture env wrong or make a mistake, it fails to zip. just search in the main directory and let the 'find' command handle it
currently both macos (clang_aot, macos-14, x64) macos (clang, macos-14, x64) try to use the same artifact name. this causes a failure. add the compiler type so they are distinct
36d0ef4 to
c4fe530
Compare
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Hello, it appears that on the lastest
masterbranch commit, fabc9a6 , some of the current mac and linux builds are broken in GitHub cloud builders.But with just a few small edits - it is easy to fix them and make them pass again.
I created a test PR in my own fork of the repo so I could run the GitHub runners and fix.
You can see here that several of the builds fail - https://github.com/culix-7/Mesen2/actions/runs/20806315078
I have made several edits in this PR to fix the build. You can see with this that all of the builds now pass! - https://github.com/culix-7/Mesen2/actions/runs/20828681806
I realize that reading makefiles can be messy, and this might be a large number of changes. I have tried to clean up the commits to make them clear and easy to follow.
Essentially:
UIproject out of the Visual Studio project file and into aUI/prebuild_linux_mac.shfile. Hopefully this makes it easier to view and edit for the future.verify-all-env:checks all of theUNAME_S,MACHINE, and platform/os flags that we use during building.make verify-all-envon the command line to test this and verify that the makefile logic does the right thing.verify-envtarget to test just one platform.With that - everything passes! :D
You can see a passing run here - https://github.com/culix-7/Mesen2/actions/runs/20828681806
I hope that helps.
..
Before:
After: