[pulseaudio-discuss] [PATCH] gitlab: Add support for GitLab CI

[Arun Raghavan]

On Thu, 23 Aug 2018, at 6:29 PM, Felipe Sateler wrote:
> On Wed, Aug 22, 2018 at 11:36 PM Arun Raghavan <arun at arunraghavan.net>
> wrote:
> 
> > This adds a Dockerfile to generate a Docker image with the required
> > dependencies on top of the standard Ubuntu 18.04 image. The Gitlab CI
> > then runs the PulseAudio build within this image.
> > ---
> >  .gitlab-ci.yml     | 19 +++++++++++++++++
> >  scripts/Dockerfile | 53 ++++++++++++++++++++++++++++++++++++++++++++++
> >  2 files changed, 72 insertions(+)
> >  create mode 100644 .gitlab-ci.yml
> >  create mode 100644 scripts/Dockerfile
> >
> > diff --git a/.gitlab-ci.yml b/.gitlab-ci.yml
> > new file mode 100644
> > index 000000000..e9c983075
> > --- /dev/null
> > +++ b/.gitlab-ci.yml
> > @@ -0,0 +1,19 @@
> > +image: registry.freedesktop.org/pulseaudio/pulseaudio/ubuntu:18.04
> > +
> > +build:
> > +  stage: build
> > +  script:
> > +    - export MAKEFLAGS="-j4"
> >
> 
> This seems to be inappropriate for a CI build. How do you know the
> available cores in the runner? I would expect to have at most
> MAKEFLAGS=$(nproc)

Sure, can do.

> > +    - NOCONFIGURE=1 ./bootstrap.sh
> > +    - mkdir build
> > +    - cd build
> > +    - ../configure --localstatedir=/var
> >
> 
> I think for CI builds options should be explicitly selected, and have the
> build fail if any of the expected options cannot be enabled. This may help
> catch some configure.ac/Makefile.am bugs.

I'm okay to add that in if you have suggestions for a sensible list (imo this can come as a second step).

> > +    - make
> > +    - make check
> > +    - make check-daemon
> > +    - make distcheck
> >
> 
> This effectively runs make and make check again. I think it would be better
> to split this to a separate job so it can run in parallel.

Jobs require artifacts to be passed around, so it's a bit of a pain. I think the additional work is worth it for now, as distcheck caught the bug that I fixed in the last commit.

> > +    - make install DESTDIR=`mktemp -d`
> > +    - make dist
> > +  artifacts:
> > +    paths:
> > +      - build/
> > diff --git a/scripts/Dockerfile b/scripts/Dockerfile
> > new file mode 100644
> > index 000000000..ed7063212
> > --- /dev/null
> > +++ b/scripts/Dockerfile
> > @@ -0,0 +1,53 @@
> > +# Start with current Ubuntu LTS
> > +FROM ubuntu:18.04
> > +
> > +# Add a PulseAudio's dependencies
> > +RUN apt-get update && apt-get install -y \
> > +    autoconf \
> > +    automake \
> > +    autopoint \
> > +    bash-completion \
> > +    check \
> > +    dbus-x11 \
> > +    g++ \
> > +    gcc \
> >
> 
> build-essential should get you a basic build system in place. Otherwise,
> you need to add make to the list (make might stop being a transitive
> dependency in the future).

I'll just add make in for now.

> > +    gettext \
> >
> 
> Isn't intltool necessary too?(If not I could drop that from the debian
> package)

It got dropped as a dep with 57e3ccaf51f714eec8ca29005c3cc4fde456e84e.
 
> > +    git-core \
> > +    libasound2-dev \
> > +    libasyncns-dev \
> > +    libatomic-ops-dev \
> >
> 
> This is not necessary because gcc has atomic builtins.

Fair enough.

> > +    libavahi-client-dev \
> > +    libbluetooth-dev \
> > +    libcap-dev \
> > +    libfftw3-dev \
> > +    libglib2.0-dev \
> > +    libgtk-3-dev \
> > +    libice-dev \
> > +    libjack-dev \
> > +    liblircclient-dev \
> > +    libltdl-dev \
> > +    liborc-0.4-dev \
> > +    libsbc-dev \
> > +    libsndfile1-dev \
> > +    libsoxr-dev \
> > +    libspeexdsp-dev \
> > +    libssl-dev \
> > +    libtdb-dev \
> > +    libudev-dev \
> > +    libwebrtc-audio-processing-dev \
> > +    libwrap0-dev \
> > +    libx11-xcb-dev \
> > +    libxcb1-dev \
> > +    libxml-parser-perl \
> > +    libxtst-dev \
> > +    systemd
> >
> 
> libsystemd-dev is missing.

We don't depend on libsystemd, just systemd.

> > +
> > +# Add a user and set as default for the build. This is safer, in general,
> > and
> > +# allows us to avoid having to explicitly allow running as root in the
> > +# check-daemon stage.
> > +RUN groupadd -g 1000 a_group && \
> > +    useradd a_user -u 1000 -g a_group -m
> >
> 
> Ubuntu has adduser that creates both user and group at the same time.

Shouldn't matter as it's about the same (and more clear in some sense).

> > +USER a_user:a_group
> > +
> > +# And make sure subsequent commands are run in the user's home directory
> > +WORKDIR /home/a_user
> > --
> > 2.17.1
> >

Thanks for the review. I'll update the MR based on this and let's continue there.

Cheers,
Arun