dunst-project / dunst

Lightweight and customizable notification daemon
https://dunst-project.org
Other
4.56k stars 340 forks source link

Update GitHub actions #1303

Closed bebehei closed 5 months ago

bebehei commented 7 months ago

Fixes dunst-project/docker-images#10

bebehei commented 7 months ago

.. no CI spawning? 🤨

bebehei commented 7 months ago

Suspected first, GH actions did not trigger. Ok, the env was broken. Was only visible, when you checked explicitly the GitHub Actions page.

bebehei commented 7 months ago

Ok, now it's failing because of the typical git "dubious ownership" problem. I expected, this wasn't a problem here at all, since it's everywhere the same user.

Apparently, someone else figured out, that UID 1001 should be used:

https://github.com/actions/runner/issues/2033#issuecomment-1598547465

Also, the reference, why UID 1001 is used is there: https://github.com/actions/runner/blob/8415f13babf8199551f817eb16b718dcdd5b31e2/images/Dockerfile#L38

It feels a bit, that someone just thought it's a good idea to assign another UID.

bebehei commented 7 months ago

Finally, it's our fault, that the pipeline does not run. 🎉

bebehei commented 7 months ago

Ok, this is it now for today. We're not dealing anymore with GH Actions/whatever problems, but with our own build pipeline.

I'll be probably back at tuesday.

zappolowski commented 7 months ago

@bebehei The error is caused but what I described in this comment. I've added this commit to make it work locally. So, something like

diff --git a/.github/workflows/main.yml b/.github/workflows/main.yml
index 56cf410..6d5d389 100644
--- a/.github/workflows/main.yml
+++ b/.github/workflows/main.yml
@@ -37,7 +37,7 @@ jobs:

     env:
       CC: ${{ matrix.CC }}
-      EXTRA_CFLAGS: "-Werror"
+      EXTRA_CFLAGS: "-Werror -gdwarf-4"
     steps:
     - uses: actions/checkout@v2
       with:

could already make it work (it's just needed for debian-bookworm and ubuntu-jammy, but I couldn't find how to set env conditionally ... might have time the coming weekend to have a closer look).

Edit: Maybe something like

diff --git a/.github/workflows/main.yml b/.github/workflows/main.yml
index b4e5a17..f4fb1ff 100644
--- a/.github/workflows/main.yml
+++ b/.github/workflows/main.yml
@@ -37,7 +37,7 @@ jobs:

     env:
       CC: ${{ matrix.CC }}
-      EXTRA_CFLAGS: "-Werror"
+      EXTRA_CFLAGS: "-Werror ${{ (matrix.distro == 'debian-bookworm' || matrix.distro == 'ubuntu-jammy') && '-gdwarf-4' || ' ' }}"
     steps:
     - uses: actions/checkout@v4
       with:

would work.

codecov-commenter commented 7 months ago

Codecov Report

All modified and coverable lines are covered by tests :white_check_mark:

Project coverage is 53.07%. Comparing base (a2af4a8) to head (eba336d).

:exclamation: Your organization needs to install the Codecov GitHub app to enable full functionality.

Additional details and impacted files ```diff @@ Coverage Diff @@ ## master #1303 +/- ## =========================================== - Coverage 65.52% 53.07% -12.45% =========================================== Files 48 48 Lines 7997 8173 +176 =========================================== - Hits 5240 4338 -902 - Misses 2757 3835 +1078 ``` | [Flag](https://app.codecov.io/gh/dunst-project/dunst/pull/1303/flags?src=pr&el=flags&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=dunst-project) | Coverage Δ | | |---|---|---| | [unittests](https://app.codecov.io/gh/dunst-project/dunst/pull/1303/flags?src=pr&el=flag&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=dunst-project) | `53.07% <ø> (-12.45%)` | :arrow_down: | Flags with carried forward coverage won't be shown. [Click here](https://docs.codecov.io/docs/carryforward-flags?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=dunst-project#carryforward-flags-in-the-pull-request-comment) to find out more.

:umbrella: View full report in Codecov by Sentry.
:loudspeaker: Have feedback on the report? Share it here.

bebehei commented 7 months ago

@zappolowski Thanks. I've seen this and had it in my mind. But I also had to kick out the dbus testing suite. I guess this is our other part we have to fix, too.

zappolowski commented 7 months ago

@bebehei I took another look into the latest build failures and it's rather wild.

The Debian bookworm image currently lacks a required package libclang-rt-dev, but even when added it's not really working as then some linker issues arise ... but just when run automatically; when I entry the container in interactive mode, I can successfully run the test-coverage target (which is the one currently failing). Even wilder: it also fails when I manually run /srv/entrypoint, but succeeds, when I adjust the shebang to #!/bin/bash. Using a non-interactive build (like done in the CI) with using #!/bin/bash still fails, though. So, my current guess, is that some setup might be missing.

zappolowski commented 7 months ago

@bebehei I figured it out ... at least I cannot reproduce the failure locally anymore.

  1. Update the debian-bookworm image ... see this PR
  2. add --coverage to CFLAGS, e.g.

    diff --git a/.github/workflows/main.yml b/.github/workflows/main.yml
    index 3d7c38f..25c2229 100644
    --- a/.github/workflows/main.yml
    +++ b/.github/workflows/main.yml
    @@ -37,7 +37,7 @@ jobs:
    
     env:
       CC: ${{ matrix.CC }}
    -      EXTRA_CFLAGS: "-Werror ${{ (matrix.distro == 'debian-bookworm' || matrix.distro == 'ubuntu-jammy') && '-gdwarf-4' || ' '}}"
    +      EXTRA_CFLAGS: "-Werror ${{ matrix.distro == 'debian-bookworm' && '--coverage -gdwarf-4' || ' '}} ${{ matrix.distro == 'ubuntu-jammy' && '-gdwarf-4' || ' ' }}"
     steps:
     - uses: actions/checkout@v4
       with:
zappolowski commented 6 months ago

I've just updated the debian-bookworm image and it seems like it works without addition of --coverage ... for whatever reason.

Currently the pipeline is green, but some tests and alpine is still disabled.

bebehei commented 5 months ago

Thanks for merging the stuff in #1312 and caring about it. I guess, I can close this PR then.