ddev / github-action-setup-ddev

Set up your GitHub Actions workflow with DDEV
GNU General Public License v3.0
33 stars 7 forks source link

Running DDEV GitHub Action with PHP 8.3 #30

Closed tomasnorre closed 4 months ago

tomasnorre commented 4 months ago

When I ran the DDEV GitHub Action with PHP 8.3 I got the following error https://github.com/tomasnorre/crawler/actions/runs/8900718640/job/24442912503#step:7:90

E: The repository 'http://ppa.launchpad.net/ondrej/php/ubuntu jammy InRelease' is not signed.

I know it was already mentioned in #27, but I think it's right to split them up as they are not related, just present at the same information in console.

Will see if I can find a way to reproduce this and perhaps come up with a fix.

rfay commented 4 months ago

That's a result of having an old DDEV version most likely. They let their key expire:

https://github.com/ddev/ddev/issues/5795

It can also be a result of internet trouble.

rfay commented 4 months ago

Oh, I see - it has the old key. The new key has to be installed.

rfay commented 4 months ago

The repository 'http://ppa.launchpad.net/ondrej/php/ubuntu jammy InRelease' is not signed

But why are the deb.sury.org versions being installed on Ubuntu? No PHP installations are required to use ddev...

tomasnorre commented 4 months ago

I cannot find where this is done in the GHA.

The repository 'http://ppa.launchpad.net/ondrej/php/ubuntu jammy InRelease' is not signed

But why are the deb.sury.org versions being installed on Ubuntu? No PHP installations are required to use ddev...

I cannot find where this is done in the GHA. Have searched for ondrej no matches. The only possibility that I see is that it's include in the base docker image used, but I don't see where that's declared.

tomasnorre commented 4 months ago

It's not a nice nor the right fix, but small shell line, could "fix" it

sudo add-apt-repository --remove ppa:ondrej/php

If it's not needed, it can be assured it's not present, but I would rather figure out where it's coming from.

rfay commented 4 months ago

I don't know how a PPA is being used anywhere for any reason. and don't find "ondrej" anywhere. I can't understand why a PPA would be installed.

I do see that tests are being run with DDEV v1.22.3, which would have the expired key, but that would be a debian error and now show an Ubuntu PPA as being involved.

tomasnorre commented 4 months ago

It only happens in my TYPO3 Crawler workflow with PHP 8.3, not with 8.1 and 8.2.

I don't see where it is coming from either. Perhaps @jonaseberle who started this action can point us into the direction?

stasadev commented 4 months ago
E: The repository 'http://ppa.launchpad.net/ondrej/php/ubuntu jammy InRelease' is not signed.

I think the error has nothing to do with this action, it comes from the ubuntu-22.04, which is used downstream:

jobs:
  Acceptance:
    runs-on: ubuntu-22.04

This could either be a temporary issue or something wrong with ubuntu-22.04.


The only "problem" is that this action runs sudo apt-get update, which causes some upstream issues.

stasadev commented 4 months ago

The fix could be the same as we have in the main ddev/ddev repository:

sudo apt-get update || true

https://github.com/ddev/github-action-setup-ddev/blob/9dc222980594cecae55d7c69cd1ba56fda984983/lib/main.js#L75


-cmd = `sudo apt-get update && sudo apt-get install -y ${ddevPackage} && mkcert -install`;
+cmd = `(sudo apt-get update || true) && sudo apt-get install -y ${ddevPackage} && mkcert -install`;
tomasnorre commented 4 months ago

I can provide a PR next week somewhen if you want.

rfay commented 4 months ago

Here's where the problem is coming from, it's in the Ubuntu 20.04 runner: https://github.com/actions/runner-images/blob/877807370206567253a2fbc0b76f95b5e69d85cc/images/ubuntu/scripts/build/install-php.sh#L14-L17

Why don't we just stop using Ubuntu 20.04? I don't think it offers any value going forward.

It does look like they have failed to maintain that I guess, not sure. Mostly we should probably be testing this on Ubuntu 22.04 and upcoming 24.04 IMO. So just remove 20.04 from the matrix?

tomasnorre commented 4 months ago

I would say, skipping the 20.04 LTS sounds like a good idea and focus on to two latest LTS releases of Ubuntu. But on the other side, Ubuntu 20.04 has support till April 2025, so many will not update unless they are "forced" to.

But your call. I can still create the PR somewhen next week if wanted.

rfay commented 4 months ago

In general, Ubuntu is so very solid that it rarely makes any difference what version you use. My opinion would be to just test on ubuntu-latest and let it be.

stasadev commented 4 months ago

And apt-get update || true is still worth adding, because these ubuntu-* still have many different PPAs https://github.com/tomasnorre/crawler/actions/runs/8900718640/job/24442912503#step:7:90 that may be temporarily unavailable at a given moment.

rfay commented 4 months ago

apt-get update || true works for me! It's not a real error if the apt update fails, and if there's a real error it will happen on actually trying to install something.

jonaseberle commented 4 months ago

Hmm, I am not sure about || true. We want apt update because we just added a new .deb repo (in the Github runner, to install the DDEV from there). If we let it fail the apt install does not produce what we want. If it fails it is a real error IMHO.

I can't reproduce in the tests from https://github.com/ddev/github-action-setup-ddev/pull/32

stasadev commented 4 months ago

This is an example of why we need || true here, many people were stuck because of an inaccessible repo that they didn't even use:

See how many repos apt update is reaching here: https://github.com/ddev/github-action-setup-ddev/actions/runs/8920768962/job/24499486305?pr=32#step:3:66 It would be annoying to see a test fail if some of them were temporarily unavailable.

We want apt update because we just added a new .deb repo (in the Github runner, to install the DDEV from there). If we let it fail the apt install does not produce what we want.

In this case apt install will fail with a different error like ddev not found and you will still see an apt update error details when scrolling up.

jonaseberle commented 4 months ago

Ok. Let's get that in :)

tomasnorre commented 4 months ago

I can confirm that the issue is fixed downstream in the TYPO3 Crawler.