Add Github action to build NONMATCHING rom #309

abaresk · 2024-03-12T05:01:08Z

No description provided.

red031000 · 2024-03-12T10:59:15Z

if this isn't ready pls mark as draft

abaresk · 2024-03-14T13:50:53Z

Should be ready for review now

red031000

I have several questions

also the required status check of build is not going to pass, given that you're removed it, so for merging we'll have to switch over what's required before it can be merged

red031000 · 2024-03-14T15:43:17Z

.github/workflows/build.yml

+    if: |
+      always() &&
+      (github.event_name == 'push') &&
+      contains(join(needs.*.result, ','), 'success')


Safeguard to make sure this only runs if all the build-X jobs succeed.

actions/runner#1251 (comment)

red031000 · 2024-03-14T15:44:03Z

.github/workflows/build-rom/action.yml

+      run: make -j${nproc}
+
+    - name: Archive build artifacts
+      if: ${{ always() && env.NONMATCHING == 0 }}


There was a step before called "Post error archive", which uploaded an archive of the build. If we still want that, we need always() in case "Build ROM" fails.

The "Post-merge" step is a bit messier if you do it for nonmatching, so I just excluded it.

red031000 · 2024-03-14T15:44:13Z

.github/workflows/build-rom/action.yml

+        find . -maxdepth 2 -type d \( -name build -or -name files \) -exec tar -czvhf ${GAME_VERSION}_build.tar.gz {} +
+
+    - name: Upload build artifacts
+      if: ${{ always() && env.NONMATCHING == 0 }}


red031000 · 2024-03-14T15:46:00Z

.github/workflows/build.yml

+
+      - name: Build ROM
+        env:
+          GAME_VERSION: HEARTGOLD


this only builds non-matching HG? not SS?

We can add SS -- it wouldn't cost any more time. But the cases where it adds coverage seemed pretty narrow.

I think you'd only get a build error if you don't define a SS-specific data variable. I could only find one in the game: ov74_0223C968

please do add it, it could be useful to have I guess

red031000 · 2024-03-14T15:47:09Z

.github/workflows/build.yml


      - name: Webhook
-        if: ${{ github.event_name == 'push' }}


not needed cause this requires push anyway, right??
did you test whether this works locally?

This whole job is guarded now by (github.event_name == 'push').

abaresk

Yeah we'll have to change the PR policy before this can be merged

abaresk · 2024-03-14T17:21:14Z

.github/workflows/build-rom/action.yml

+      run: make -j${nproc}
+
+    - name: Archive build artifacts
+      if: ${{ always() && env.NONMATCHING == 0 }}


There was a step before called "Post error archive", which uploaded an archive of the build. If we still want that, we need always() in case "Build ROM" fails.

The "Post-merge" step is a bit messier if you do it for nonmatching, so I just excluded it.

abaresk · 2024-03-14T17:21:23Z

.github/workflows/build-rom/action.yml

+        find . -maxdepth 2 -type d \( -name build -or -name files \) -exec tar -czvhf ${GAME_VERSION}_build.tar.gz {} +
+
+    - name: Upload build artifacts
+      if: ${{ always() && env.NONMATCHING == 0 }}


abaresk · 2024-03-14T17:28:46Z

.github/workflows/build.yml

+
+      - name: Build ROM
+        env:
+          GAME_VERSION: HEARTGOLD


We can add SS -- it wouldn't cost any more time. But the cases where it adds coverage seemed pretty narrow.

I think you'd only get a build error if you don't define a SS-specific data variable. I could only find one in the game: ov74_0223C968

abaresk · 2024-03-14T17:31:17Z

.github/workflows/build.yml

+    if: |
+      always() &&
+      (github.event_name == 'push') &&
+      contains(join(needs.*.result, ','), 'success')


Safeguard to make sure this only runs if all the build-X jobs succeed.

actions/runner#1251 (comment)

abaresk · 2024-03-14T17:32:00Z

.github/workflows/build.yml


      - name: Webhook
-        if: ${{ github.event_name == 'push' }}


This whole job is guarded now by (github.event_name == 'push').

Makes it harder to debug build errors

PikalaxALT

lgtm, you clearly have a more thorough understanding of gh actions than the rest of us

PikalaxALT · 2024-04-19T22:40:08Z

if you're wondering why this hasn't been merged yet:

merge conflicts
github actions seems to have screwed you over, preventing merge

red031000 · 2024-04-21T17:06:58Z

if you're wondering why this hasn't been merged yet:

merge conflicts

github actions seems to have screwed you over, preventing merge

I'm still blocking waiting for non-matching SS btw

abaresk · 2024-04-28T14:22:00Z

I added non-matching Soul Silver. Is it possible to change the settings to merge this, or do I need to restore the build job and delete it after this gets merged?

PikalaxALT · 2024-05-24T23:36:50Z

Please resolve conflicts with master

red031000

this is fine (sorry didn't realise I was blocking it) but the conflicts need to be fixed

abaresk force-pushed the gh-nonmatching branch 6 times, most recently from 437cd6c to cf6888a Compare March 12, 2024 05:22

Parallelize Github actions and add NONMATCHING job

4aa608e

abaresk force-pushed the gh-nonmatching branch 3 times, most recently from 80d428c to 200640d Compare March 12, 2024 06:17

Test calcrom

64d78e2

abaresk force-pushed the gh-nonmatching branch from 200640d to 64d78e2 Compare March 12, 2024 06:31

abaresk marked this pull request as draft March 12, 2024 13:09

abaresk added 2 commits March 12, 2024 13:30

Test nonmatching

ce7e9e5

Calcrom sub/ binary

5cd49fa

abaresk force-pushed the gh-nonmatching branch 14 times, most recently from fa61bf0 to 906e2c4 Compare March 14, 2024 04:12

abaresk force-pushed the gh-nonmatching branch 2 times, most recently from 5b9d60e to effdc13 Compare March 14, 2024 06:07

More tests

d8c2d49

abaresk force-pushed the gh-nonmatching branch from effdc13 to d8c2d49 Compare March 14, 2024 06:22

abaresk added 3 commits March 14, 2024 13:36

Cleanup

019228f

Merge branch 'master' into gh-nonmatching

431582a

Cleanup

84f5104

abaresk marked this pull request as ready for review March 14, 2024 13:50

red031000 requested changes Mar 14, 2024

View reviewed changes

abaresk commented Mar 14, 2024

View reviewed changes

Remove verbose output while tarring

dc741d6

Makes it harder to debug build errors

PikalaxALT previously approved these changes Mar 14, 2024

View reviewed changes

Merge branch 'master' into gh-nonmatching

07653d2

abaresk dismissed PikalaxALT’s stale review via 07653d2 April 28, 2024 13:07

Fix nonmatching issue

a229626

abaresk force-pushed the gh-nonmatching branch from b754db3 to fdf3208 Compare April 28, 2024 13:43

Add nonmatching soul silver

5103079

abaresk force-pushed the gh-nonmatching branch from fdf3208 to 5103079 Compare April 28, 2024 14:08

PikalaxALT previously approved these changes May 13, 2024

View reviewed changes

PikalaxALT added the CI/CD label May 22, 2024

red031000 previously approved these changes Jun 1, 2024

View reviewed changes

Merge branch 'master' into gh-nonmatching

910b488

abaresk dismissed stale reviews from red031000 and PikalaxALT via 910b488 June 5, 2024 15:47

red031000 approved these changes Jun 5, 2024

View reviewed changes

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Add Github action to build NONMATCHING rom #309

Add Github action to build NONMATCHING rom #309

abaresk commented Mar 12, 2024 •

edited

red031000 commented Mar 12, 2024

abaresk commented Mar 14, 2024

red031000 left a comment

red031000 Mar 14, 2024

abaresk Mar 14, 2024

red031000 Mar 14, 2024

abaresk Mar 14, 2024

red031000 Mar 14, 2024

abaresk Mar 14, 2024

red031000 Mar 14, 2024

abaresk Mar 14, 2024

red031000 Mar 30, 2024

red031000 Mar 14, 2024

abaresk Mar 14, 2024

abaresk left a comment

abaresk Mar 14, 2024

abaresk Mar 14, 2024

abaresk Mar 14, 2024

abaresk Mar 14, 2024

abaresk Mar 14, 2024

PikalaxALT left a comment

PikalaxALT commented Apr 19, 2024

red031000 commented Apr 21, 2024

abaresk commented Apr 28, 2024

PikalaxALT commented May 24, 2024

red031000 left a comment

Add Github action to build NONMATCHING rom #309

Are you sure you want to change the base?

Add Github action to build NONMATCHING rom #309

Conversation

abaresk commented Mar 12, 2024 • edited

red031000 commented Mar 12, 2024

abaresk commented Mar 14, 2024

red031000 left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

abaresk left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

PikalaxALT left a comment

Choose a reason for hiding this comment

PikalaxALT commented Apr 19, 2024

red031000 commented Apr 21, 2024

abaresk commented Apr 28, 2024

PikalaxALT commented May 24, 2024

red031000 left a comment

Choose a reason for hiding this comment

abaresk commented Mar 12, 2024 •

edited