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

Update dockerfile/build.yml #96

Closed
wants to merge 2 commits into from
Closed

Update dockerfile/build.yml #96

wants to merge 2 commits into from

Conversation

ghost
Copy link

@ghost ghost commented Oct 3, 2023

  • Getting compiler.jar from https


- name: Set up Docker Buildx
uses: docker/setup-buildx-action@v1

- name: Login to GitHub
if: ${{ steps.docker-vars.outputs.has-docker-secret == 'true' }}
uses: docker/login-action@v1
uses: docker/login-action@v2
with:
registry: ghcr.io
username: ${{ github.repository_owner }}
Copy link
Member

Choose a reason for hiding this comment

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

According to the documentation of the login-action step, this should be

Suggested change
username: ${{ github.repository_owner }}
username: ${{ github.actor}}

This would be my guess on how to fix it. The change from GTCR_PAT to GITHUB_TOKEN seems okay.
Also the up to date version of docker/login-action is v3

Copy link
Member

Choose a reason for hiding this comment

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

I tested with these changes and got the following error in the Build and Push section

 Error: buildx failed with: ERROR: failed to solve: failed to push ghcr.io/illarion-ev/illarion-server/base:pr-100: unexpected status from POST request to https://ghcr.io/v2/illarion-ev/illarion-server/base/blobs/uploads/: 403 Forbidden

seen here https://github.com/Illarion-eV/Illarion-Server/actions/runs/7427486427/job/20213407706?pr=100

Not too familiar with github or its workings, so not sure if I should click on "commit suggestion" on here to test things like this for PR reviews? As I wrote in #100 I was told early on to not directly edit things in Illarion-Content due to formatting so not sure if A. this applies to server side stuff too and B. if this applies to PR reviews like this, so I'm treading on the side of caution.

with:
registry: ghcr.io
username: ${{ github.repository_owner }}
password: ${{ steps.docker-vars.outputs.docker-secret }}
password: ${{ steps.docker-vars.outputs.docker-secret }}
Copy link
Member

Choose a reason for hiding this comment

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

Mind the spaces 😛

Suggested change
password: ${{ steps.docker-vars.outputs.docker-secret }}
password: ${{ steps.docker-vars.outputs.docker-secret }}

@@ -13,7 +13,9 @@ jobs:
build:
name: Compile and Test
runs-on: ubuntu-latest

permissions:
Copy link
Member

Choose a reason for hiding this comment

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

This shouldn't be required.

Copy link
Member

Choose a reason for hiding this comment

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

I tried testing it without the permissions: line just now and got the following error

You have an error in your yaml syntax on line 16

where it then got stuck and refused to finish the checks. So it seems like it might be necessary?

Copy link
Member

@mkaring mkaring left a comment

Choose a reason for hiding this comment

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

I left you a few notes on the changes. Let's see if we get this rolling again 😉

@mkaring
Copy link
Member

mkaring commented Jan 6, 2024

@Kerruan: I fixed the develop branch, so all the builds are running fine again. Once you rebase this pull-request and reduce the changes to what needs to be changed initially, everything should work fine.

@ghost ghost closed this by deleting the head repository Apr 17, 2024
This pull request was closed.
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