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

ci: Replace broken repoman with pkgcheck, add manifest checks, drop emerge all #512

Closed
wants to merge 6 commits into from

Conversation

NexAdn
Copy link
Member

@NexAdn NexAdn commented Dec 9, 2022

This PR drops repoman from the CI pipeline as it's no longer available in ::gentoo and deprecated. It is replaced with its successor pkgcheck. Furthermore, manifest checks are added which verify the validity of manifest files.
Third, jobs to emerge all ebuilds are dropped. ago's tinderbox does a good job at finding problems by emerging ebuilds with various USE flag combinations and varying system setups.
I suggest we ask ago to enable the tinderbox for our overlay and remove the jobs to emerge all ebuilds.

Lastly, a major change is that the new jobs use a Docker runner as all the checks are performed in Docker anyway and the specialized CI image already contains an up-to-date portage tree and as such no longer requires mounting a portage tree image into the Docker container.

We can leverage ago's Gentoo Tinderbox for this, which also allows for
broader testing with different USE flag combinations and build
environments.
repoman is no longer the linter used in portage and is no longer
available in ::gentoo. Its replacement is pkgcheck.
@NexAdn NexAdn changed the title ci: Replace broken repoman with pkgcheck, add manifest checks, drop emerge all WIP: ci: Replace broken repoman with pkgcheck, add manifest checks, drop emerge all Dec 9, 2022
@NexAdn NexAdn changed the title WIP: ci: Replace broken repoman with pkgcheck, add manifest checks, drop emerge all ci: Replace broken repoman with pkgcheck, add manifest checks, drop emerge all Dec 9, 2022
These checks ensure no broken manifests are commited to master and
manifests already on master are still valid and their sources still
exist.
Just a few extra blank lines for better readability.
These scripts are no longer used by the CI pipeline and are now obsolete.
These jobs don't need so many resources since they run on docker now.
The previous jobs had to run emerge jobs, possibly compiling stuff. The
new jobs only run a few downloads and static analysis, which shouldn't
be as resource intensive.
@audiodef
Copy link
Contributor

This makes sense and it gets my vote.

@NexAdn NexAdn requested a review from audiodef February 5, 2023 19:35
@NexAdn NexAdn mentioned this pull request Feb 5, 2023
@simonvanderveldt
Copy link
Member

simonvanderveldt commented Feb 5, 2023

Lastly, a major change is that the new jobs use a Docker runner as all the checks are performed in Docker anyway and the specialized CI image already contains an up-to-date portage tree and as such no longer requires mounting a portage tree image into the Docker container.

The problem with this is that now you don't have an (easy) way to run these jobs locally anymore, since AFAIK there's no way to run any of the CircleCI (or any CI service for that matter) steps locally.


pkgcheck all:
docker:
- image: registry.gitlab.fem-net.de/gentoo/fem-overlay-ci-image:master
Copy link
Member

Choose a reason for hiding this comment

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

Not sure if we/you should depend on a (for me random/unvetted) third-party Docker image.
You can just use the upstream gentoo image with a volume container with the portage image to get the portage tree.

Copy link
Member Author

Choose a reason for hiding this comment

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

Well, that image is built with a GitLab CI pipeline in a public repository. This is way more trustworthy than all the images on Docker Hub which don't link to their repo and where you can't be sure the image published is actually built from the Dockerfile linked in the README. And yet people blindly accept these kinds of Docker images as well (not saying you do – you obviously used the official images which can be considered trusted).
Also, you can have a look at the commit history and you'll see that the Dockerfile in this repo is written by me (since I didn't find any image on Docker Hub which satisfied my requirements).

The solution you proposed still has the issue of requiring to install the required tools during each CI run. This will cause problems after version bumps of important Gentoo packages like Perl which require rebuilding a bunch of stuff on the system, since it will blow up CI run times and maybe even timeout the jobs. That stuff is one of the reasons I do all the work during the build of the Docker image.

I suppose I could switch back to the old approach. This will, however, introduce the aforementioned problems without bringing (at least in my opinion) any benefit, since I consider the trust-issue with the fem-overlay-ci-image non-existent, given that the entire build process can be seen and monitored publicly.

@NexAdn
Copy link
Member Author

NexAdn commented Feb 6, 2023

The problem with this is that now you don't have an (easy) way to run these jobs locally anymore, since AFAIK there's no way to run any of the CircleCI (or any CI service for that matter) steps locally.

Well, the steps don't really need to be run locally anyway. The only available tools for Gentoo development I know of are pkgdev and pkgcheck from pkgcore. repoman no longer exists. Thus, anyone who wants to change stuff on an ebuild needs to have those tools installed anyway and working with them instead of using plain git even runs those checks performed by the CI automatically before committing, ensuring the Manifest matches the staged files and no important QA issues are introduced into the package.

To sum up, if you are following the Gentoo development workflow, you will never have to run the CI checks locally. And even if you do, you will probably have the tools installed already, as they are needed for writing ebuilds anyway.

@NexAdn NexAdn marked this pull request as draft February 6, 2023 11:17
@NexAdn
Copy link
Member Author

NexAdn commented Feb 6, 2023

I'll switch back to the approach used by simon for this PR. The overhead was probably not that big in the past and I'd rather have a slow pipeline that one that doesn't work. I also don't want to step on anyone's toes by merging this new approach without consent. The benefits I see from my new design are also not worth prolonging this discussion, given that simon doesn't have that much free time to respond quickly. We can always revisit this topic at a later time. For now, having a working CI pipeline at all is far more important.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Development

Successfully merging this pull request may close these issues.

3 participants