search

dunst-project / docker-images

Docker images for dunst (mainly for CI)
Other
2 stars 3 forks source link

Update CI images - part I #7

Closed zappolowski closed 5 months ago

zappolowski commented 8 months ago

These are just the easy ones ...

Fedora, Alpine, and Archlinux cannot build current master due to -Werror and a lot of warnings there - these have to be fixed before the pipeline images can be updated. See #1232 at the main repo and #8 for a PR here to update the mentioned images.

fwsmit commented 6 months ago

I think it's nice to add some sort of latest version of each distro, along with some older, still supported version. Using the "latest" tag or similar, these will always be up to date.

zappolowski commented 6 months ago

Using the "latest" tag or similar, these will always be up to date.

The problem with this approach is, that now the actual version used depends on when the image was (re)build. When sticking to real version, one can at least be sure about the release being used (plus some potential minor updates).

This obviously doesn't work for rolling release distributions like Arch Linux ... so, there latest is the only option.

fwsmit commented 6 months ago

Using the "latest" tag or similar, these will always be up to date.

The problem with this approach is, that now the actual version used depends on when the image was (re)build. When sticking to real version, one can at least be sure about the release being used (plus some potential minor updates).

This obviously doesn't work for rolling release distributions like Arch Linux ... so, there latest is the only option.

If updating the CI image breaks the building of dunst, that's something we need to know about. Since the distro building dunst will have the same issue. So I think we should actually build the CI images more often and track distro's closely

zappolowski commented 5 months ago

So I think we should actually build the CI images more often and track distro's closely

That's true, but having just latest here doesn't automatically create new images if not actively built and pushed to the repository. As this (at least currently) is a manual step one can also quickly check available versions of the images being built and adjust the files (in most cases it's enough to adjust the first line).

If on the other hand CI images are built automatically (maybe on a schedule) you could run into sudden pipeline failures both here (when latest points to another version and packages were renamed/removed - this happened for me for the alpine image) or in the main repo (new compiler version available with stricter rules - breaking previously seemingly good code). As maintenance is rather low (no offense) this should be avoided.

Given that the current approach of having CI images built in a separate repository, from my point of view manual creation of the CI images (also check a successful build of the current main branch of the main repository) seems the best approach. If the actual CI is fully contained in the main repository this whole approach is void anyhow.

fwsmit commented 5 months ago

Is this mergeable, or does this not run either?

fwsmit commented 5 months ago

I'm fine with the current approach, so if the CI images run, then I'll merge this

zappolowski commented 5 months ago

Is this mergeable, or does this not run either?

  1. The last both merges in the dunst repo in combination broke the pipeline. This should be fixed in #1282.
  2. I've just ran these images on the fixed branch and this time with clang as well (I did just runs with gcc in the past). Turns out that debian-bookworm and ubuntu-jammy are broken due to an incompatible combination of clang and valgrind. Short: clang issues dwarf instructions which valgrind can't understand yet (clang 14 switched to dwarfv5 by default which valgrind 3.19 - shipped in both distributions - doesn't like). I'm looking into possible solutions. See this issue for reference.

Edit: I've fixed the 2nd issue. Please take a look at the last commit.

fwsmit commented 5 months ago

Thanks! I've merged it now.