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 CI Port #16082

Open
wants to merge 13 commits into
base: master
Choose a base branch
from
Open

GitHub Actions CI Port #16082

wants to merge 13 commits into from

Conversation

illusion0001
Copy link
Contributor

@illusion0001 illusion0001 commented Sep 15, 2024

Cache is supported on the penguin
Cold: https://github.com/illusion0001/rpcs3/actions/runs/10871089443/attempts/1
Warm: https://github.com/illusion0001/rpcs3/actions/runs/10871089443

Plans for later, build in clang and gcc on Windows and OSX

@Megamouse Megamouse added the Build and CI Anything related to the build process and continuous integration label Sep 19, 2024
@illusion0001 illusion0001 marked this pull request as draft September 23, 2024 10:17
@hcorion
Copy link
Member

hcorion commented Sep 23, 2024

What's the motivation for porting to GitHub Actions? Doesn't it have the exact same restrictions as Azure?

@illusion0001 illusion0001 force-pushed the patch-1 branch 3 times, most recently from 1cdde6d to 1b3a7cd Compare September 28, 2024 08:52
@illusion0001
Copy link
Contributor Author

illusion0001 commented Sep 28, 2024

What's the motivation for porting to GitHub Actions? Doesn't it have the exact same restrictions as Azure?

so users don't have to sign up to extra services to run auto builds, and yeah i think

@q4a
Copy link
Contributor

q4a commented Jan 19, 2025

What's the motivation for porting to GitHub Actions? Doesn't it have the exact same restrictions as Azure?

I think that GitHub Actions is useful now because of free arm64 runners. I improved illusion0001's version a bit:
https://github.com/q4a/rpcs3/commits/arm-ci/
But don't understand how to pass $COMPILER env to docker?
Check fail on github actions:
https://github.com/q4a/rpcs3/actions/runs/12852962562/job/35835578225
vs sucess on Cirrus CI:
https://cirrus-ci.com/task/6366890357948416

@illusion0001
Copy link
Contributor Author

$COMPILER is used to distinguish between GCC and clang builds

@q4a
Copy link
Contributor

q4a commented Jan 19, 2025

But don't understand how to pass $COMPILER env to docker?

I know that)
Cirrus CI has this lines in log:
https://cirrus-ci.com/task/6366890357948416

+ export CC=clang-18
+ export CXX=clang++-18

Actions x86_64 has this lines in log:
https://github.com/q4a/rpcs3/actions/runs/12852962562/job/35835578464

+ export CC=clang-18
+ export CXX=clang++-18

But Actions arm64 has only this lines in log:
https://github.com/q4a/rpcs3/actions/runs/12852962562/job/35835578225

+ export CC=
+ export CXX=

@q4a
Copy link
Contributor

q4a commented Jan 19, 2025

Ok. I fixed arm64 build with simple chmod +x ‎.ci/build-linux-aarch64.sh.
@illusion0001 can you cherry-pick or just copy my changes to this PR?
https://github.com/q4a/rpcs3/commits/arm-ci/
Here is build result:
https://github.com/q4a/rpcs3/actions/runs/12853858021

@Darkhost1999
Copy link
Contributor

Darkhost1999 commented Jan 19, 2025

Screenshot.jpg

Maybe instead of matrix: Linux Build it can be matrix: Linux Aarch 64 and matrix: Linux X64 by creating new name categories.
I was looking into splitting the files and then just using uses ./.workflows/linux_aarch64.yml
That way there's a main file for rpcs3.yml and then maintainable smaller jobs. Especially after adding MacOS then Windows Arm.

@illusion0001
Copy link
Contributor Author

Ok. I fixed arm64 build with simple chmod +x ‎.ci/build-linux-aarch64.sh. @illusion0001 can you cherry-pick or just copy my changes to this PR? https://github.com/q4a/rpcs3/commits/arm-ci/ Here is build result: https://github.com/q4a/rpcs3/actions/runs/12853858021

Pushed

@illusion0001 illusion0001 marked this pull request as ready for review January 19, 2025 16:03
@AniLeo AniLeo requested a review from hcorion January 19, 2025 17:03
@illusion0001
Copy link
Contributor Author

illusion0001 commented Jan 20, 2025

Arm gcc build is clang, needs fixing
https://github.com/illusion0001/rpcs3/actions/runs/12855079318/job/35840192563

@q4a
Copy link
Contributor

q4a commented Jan 20, 2025

Arm gcc build is clang, needs fixing https://github.com/illusion0001/rpcs3/actions/runs/12855079318/job/35840192563

I tested few changes and this one works pretty good:
q4a@3e2d9f7
CI result:
https://github.com/q4a/rpcs3/actions/runs/12875043123

@illusion0001
Copy link
Contributor Author

Arm gcc build is clang, needs fixing https://github.com/illusion0001/rpcs3/actions/runs/12855079318/job/35840192563

I tested few changes and this one works pretty good: q4a@3e2d9f7 CI result: https://github.com/q4a/rpcs3/actions/runs/12875043123

Updated. Also fixed windows artifact uploading build files

@illusion0001 illusion0001 force-pushed the patch-1 branch 2 times, most recently from f198291 to 11a5643 Compare January 21, 2025 05:20
@Jack54guythecoder
Copy link

What's the motivation for porting to GitHub Actions? Doesn't it have the exact same restrictions as Azure?

You can use nightly.link with it.

matrix:
compiler: [clang, gcc]
cfg:
- os: ubuntu-24.04-arm
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

To save on CI time, we only want to run the clang compiler on arm, since we're already testing both compilers on the x86 build. For arm64 just use the compiler we want to publish binaries with (clang).

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Should be fixed

- name: Append Compiler version to appimage
run: |
file=".ci/deploy-linux.sh"
sed -i 's/RPCS3_APPIMAGE="rpcs3-v${COMM_TAG}-${COMM_COUNT}-${COMM_HASH}_linux64.AppImage"/RPCS3_APPIMAGE="rpcs3-v${COMM_TAG}-${COMM_COUNT}-${COMM_HASH}-${COMPILER}_linux64.AppImage"/g' $file
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why are you changing the output filename here? If we want to change the output filename just change the deploy script. I'm not sure why you're changing it in the first place though.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It's to separate the artifacts from being overwritten, i.e clang finishes before gcc, when GCC is uploaded, clang appimage is overwritten. I didn't want to modify script to keep compatibility for other ci

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Well, now that I had a second look at this, it's not needed after all because artifact have compiler name in it

${{ matrix.cfg.build_sh }}
- name: Upload artifacts
uses: actions/upload-artifact@main
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm not familiar with actions/upload-artifact@main, is this going to upload binaries to RPCS3/rpcs3-binaries-win/linux/linux-arm64/etc ? I don't see where we set any variables related to the upload repository, so I don't think this will work how we want it to.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Please also only run this step with the same restrictions we use in Azure:
condition: and(ne(variables['Build.Reason'], 'PullRequest'), eq(variables['Build.Repository.Name'], 'RPCS3/rpcs3'), eq(variables['Build.SourceBranch'], 'refs/heads/master'), eq(variables['COMPILER'], 'clang'))

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Added

        if: |
          github.event_name != 'pull_request' &&
          github.repository == 'RPCS3/rpcs3' &&
          github.ref == 'refs/heads/master' &&
          matrix.compiler == 'clang'

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm not familiar with actions/upload-artifact@main, is this going to upload binaries to RPCS3/rpcs3-binaries-win/linux/linux-arm64/etc ? I don't see where we set any variables related to the upload repository, so I don't think this will work how we want it to.

Nope, there will have to be a separate step for it. Using appropriate tokens like azure to upload it

DEPLOY_APPIMAGE: true
APPDIR: "/rpcs3/build/appdir"
ARTDIR: "/root/artifacts"
RELEASE_MESSAGE: "/rpcs3/GitHubReleaseMessage-${{ matrix.compiler }}.txt"
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We shouldn't need to have release messages per-compiler, since we only upload the binaries from one compiler to the GitHub Release.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Removed

uses: actions/cache@main
with:
path: ${{ env.CACHE_DIR }}
key: ${{ runner.os }}-${{ env.COMPILER }}-${{ env.QT_VER }}-${{ env.VULKAN_SDK_SHA }}-llvm.lock-glslang.lock
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm wondering if the llvm.lock and glslang.lock keys are working? In Azure providing the filename means it will hash the file for the key. However, in https://github.com/actions/cache#creating-a-cache-key I see that they use this function: hashFiles('**/lockfiles') to hash files.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Will check

shell: bash
run: .ci/setup-windows.sh

- name: Export Variables
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This should should be in a script file. Also, is it possible to do this step in bash?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It can be done in bash, as for making it a file, I'm afraid that github.env won't be resolved there, hence why it's a step.

- name: Pack up build artifacts
shell: bash
run: |
mkdir -p build
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This should mkdir $BUILD_ARTIFACTSTAGINGDIRECTORY

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Should be sorted

path: bin2
if-no-files-found: error

- name: Upload artifacts (7z)
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Same problems with the linux version of this step

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Should be sorted

run: |
mkdir -p build
.ci/deploy-windows.sh
mkdir -p bin2/${{ env.BUILD_RAW }}
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why are we using this bin2 folder instead of just using the existing bin folder for the zip artifact upload.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Bin2 is really only for zipped version artifact to not be double compressed. I'll remove it in favor of 7z

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Zip removed

mkdir -p bin2/${{ env.BUILD_RAW }}
mv bin/* bin2/${{ env.BUILD_RAW }}
- name: Upload artifacts (zip)
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

If I understand correctly, we upload this seperate .zip because the .7z step gets double compressed? Perhaps we only deploy the 7z artifact in the case of a master build for upload.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'll remove zip

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Zip removed

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Build and CI Anything related to the build process and continuous integration
Projects
None yet
Development

Successfully merging this pull request may close these issues.

7 participants