abarichello / godot-ci

Docker image to export Godot Engine games. Templates for Gitlab CI and GitHub Actions to deploy to GitLab Pages/GitHub Pages/Itch.io.
https://hub.docker.com/r/barichello/godot-ci
MIT License
759 stars 133 forks source link

chore(git): version for LFS dependency #44

Closed lallmon closed 4 years ago

lallmon commented 4 years ago

So regarding #43 and my last pull request adding git lfs to the dependencies; to actually be able to use LFS in the actions pipeline, you need a minimum of git version 2.18.

Ubuntu Bionic defaults to 2.17.1 :frowning:

There are two solutions here, to use ubuntu: focal as the image (the current LTS build)

Or add the PPA repository while installing the other dependencies

sudo add-apt-repository ppa:git-core/ppa
sudo apt update
sudo apt install git

Not sure which one falls better in line with the strategy of Godot-CI? However, I'm not sure how that might affect the Mono Dockerfile, since I can't really find what that image is based on.

lallmon commented 4 years ago

Also I did a test using these version of Ubuntu change proposed here, and it works https://hub.docker.com/repository/docker/lalmon/godot-ci-lfs/general

However I'd prefer to contribute to this project rather than fork it just for LFS support.

I can also update the Readme if you like, because the 'Checkout" action (using V2 in my action btw) requires LFS to be enabled.

steps:
      - name: Checkout
        uses: actions/checkout@v2
        with:
          lfs: true
abarichello commented 4 years ago

However I'd prefer to contribute to this project rather than fork it just for LFS support.

I can also update the Readme if you like, because the 'Checkout" action (using V2 in my action btw) requires LFS to be enabled.

Great! Please edit the workflow file to include the lfs flags then.

realkotob commented 4 years ago

I agree with upgrading to ubuntu:focal if it's the current LTS release.

The mono docker is based on debian buster (10) slim -- https://github.com/mono/docker/blob/master/6.10.0.104/slim/Dockerfile

Looking at the git package on buster, it seems it has git 2.20 so we should be fine. -- https://packages.debian.org/buster/git

lallmon commented 4 years ago

Alright @aBARICHELLO @asheraryam I rebased my branch off current master and added the changes to the action file, let me know if anything else is needed. And Resolves #43

abarichello commented 4 years ago

Pipelines working: https://github.com/aBARICHELLO/game-off/runs/1393783390 https://gitlab.com/BARICHELLO/game-off/-/pipelines/215528982

Thanks!

OverloadedOrama commented 4 years ago

I am having issues after this PR got merged with Pixelorama's GitHub Actions Windows export workflow. The "Setup WINE and rcedit" step gets stuck because it waits for user input, and the workflow keeps running for hours. I Googled a bit, and found out that a solution could be to add this line to the Dockerfile. ENV DEBIAN_FRONTEND=noninteractive

https://askubuntu.com/questions/909277/avoiding-user-interaction-with-tzdata-when-installing-certbot-in-a-docker-contai

abarichello commented 4 years ago

Ok, I'll update in a few hours and rebuild the images

abarichello commented 4 years ago

I think ARG instead of ENV should be enough right?

OverloadedOrama commented 4 years ago

I haven't tested it myself (hence why I did not open a pull request), but I took a look at #37 and it seems to be using ENV. I'm not sure if there's a difference though.

abarichello commented 4 years ago

I've seen that env on other ubuntu images, I think I should have included it from the start

OverloadedOrama commented 4 years ago

I tried re-running the jobs, but the same problem still persists. image

The workflow gets stuck there and does not do anything.

abarichello commented 4 years ago

If this prompt is showing up during the job then i think we need to use ENV instead of ARG to expose the variable beyond build time, please test this and let me know if it works.

OverloadedOrama commented 4 years ago

I made a copy of the Dockerfile and tested it with ENV instead of ARG and it seems to be working, so I assume that should solve the issue.

abarichello commented 4 years ago

My mistake then, I'll update in a few hours. Thanks!