Libelektra: Running reformat-all script

[alexsaber]

Unfortunately, I cannot run the reformat-all script.

Steps to Reproduce the Problem

Build docker container like this

docker build -t buildelektra-stretch-full \
--build-arg JENKINS_USERID=id -u \
--build-arg JENKINS_GROUPID=id -g \
-f scripts/docker/debian/stretch/Dockerfile \
scripts/docker/debian/stretch/

Execute the script inside the container

./reformat-all

Expected Result

The formatting is automatically adapted.

Actual Result

ls: cannot access './reformat-*': No such file or directory

Further Log Files and Output

I tried to execute the reformat-* scripts by hand, but it seems like the dependencies have not been installed. I wander why, since cmake-format is specified in the Dockerfile.

jenkins@5a151fbbc4d8:~/workspace/scripts$ ./reformat-cmake
Please install cmake-format
jenkins@5a151fbbc4d8:~/workspace/scripts$

Further, I tried to execute the scripts with sudo, but I do not know the password of the jenkins user.

[markus2330]

Thank you for reporting this problem!

Execute the script inside the container
./reformat-all

This is a bug. It seems like the script does not work if you are in the scripts folder. Try to run

./scripts/reformat-all

from the top-level source folder.

but it seems like the dependencies have not been installed

Which Dockerfile did you use? With our stretch Dockerfile it works (it is used on our build server).

Further, I tried to execute the scripts with sudo, but I do not know the password of the jenkins user.

Sudo is not needed for reformatting.

[alexsaber]

As always, thank you very much for a quick reply!

Executing the script like ./scripts/reformat-all works indeed.

Which Dockerfile did you use? With our stretch Dockerfile it works (it is used on our build server).

I used this Dockerfile. Just as you suggested it today after the lecture.

scripts/docker/debian/stretch/Dockerfile

I really do not understand why it cannot find the dependencies.

Here is again my output

osboxes@osboxes:~/TU/libelektra$ docker run -it --rm -v "$PWD:/home/jenkins/workspace" -w /home/jenkins/workspace buildelektra-stretch-full:latest
jenkins@994368e0f79f:~/workspace$ ./scripts/reformat-all 
running ./scripts/reformat-cmake...
Please install `cmake-format`
running ./scripts/reformat-markdown...
Please install `prettier` (npm install --global [email protected])
running ./scripts/reformat-shfmt...
running ./scripts/reformat-source...
ClangFormat:   
Version Info:  
Major Version: 
Please install clang-format 6 or clang-format 7jenkins@994368e0f79f:~/workspace$ 

[markus2330]

@kodebach @sanssecours do you have an idea why clang-format is not installed there? Is it on purpose?

@alexsaber in scripts/docker/debian/sid/Dockerfile it is included, so you can try it there

Adding it in stretch should be easy:

• add deb http://ftp.at.debian.org/debian/ stretch-backports main in /etc/apt/sources.list
• then install "clang-format-6.0"

We definitely need some official docker images for:

• trying out Elektra
• developing with Elektra

@kodebach @sanssecours what do you think which docker image should it be?

[sanssecours]

do you have an idea why clang-format is not installed there?

I would assume we do not install clang-format in the Docker image for Debian stretch, since we

• already install it in the Dockerfile for Debian sid, and
• also in various Linux/macOS build jobs on Cirrus and Travis

.

what do you think which docker image should it be?

In the moment I would say the Docker image for Debian sid would be my preferred option, since it does not contain as many outdated packages as Debian stretch. The image for Alpine Linux would be even better than the one for Debian sid, if Alpine Linux would offer better support for Java.

[kodebach]

do you have an idea why clang-format is not installed there?

The Jenkins job that checks formatting uses the sid image, so probably no-one ever noticed that it is missing in stretch.

what do you think which docker image should it be?

Our existing Docker images were written for CI. Therefore they contain everything needed for all the core and all the niche parts of Elektra. Instead of having 2 image, I propose having 3 images. That way new developers won't be scared away by size of the image and the number of dependencies inside the Dockerfile.

1. For quickly trying Elektra: should contain an installed version of Elektra including a small standard set of plugins. So no haskell or java stuff, nothing that has many dependencies or is only useful in very niche cases. If someone wants to try the more niche parts, they should use the third image and build Elektra themselves.
2. For quick and easy development of Elektra: similarly to the first image should contain the dependencies for building a minimal set of plugins/bindings/etc. Should also include all the formatting and checking tools to build and test this core part.
3. A full development and testing image: should contain dependencies for everything. Out of the box it should be possible to clone the git repo and build and test everything.

For the first two images I would suggest Alpine or another small base image, so people can get started quickly and easily. The third image should be the existing Debian sid image. (I also think that stretch is too outdated.)

[alexsaber]

Dear all! Thank you for your support, but I still cannot run the reformatting scripts. Because of this, I cannot finish resolving issue #2541. The tests are still, of course, failing because of the wrong formattings. What can you recommend me to do?

[sanssecours]

What can you recommend me to do?

Did you try the Docker image for Debian sid? After building the image with

docker build -t buildelektra-sid \
    --build-arg JENKINS_USERID=$(id -u) \
    --build-arg JENKINS_GROUPID=$(id -g) \
    -f scripts/docker/debian/sid/Dockerfile \
    scripts/docker/debian/sid/

and running it using

docker run -it --rm \
    -v "$PWD:/home/jenkins/workspace" \
    -w /home/jenkins/workspace \
    buildelektra-sid

the script reformat-all:

scripts/reformat-all

worked successfully on my machine.

Another option is to use the output of the build server to fix the problem. Every time testscr_check_formatting fails, it will print output that you can apply locally with the command diff to fix the formatting problems.

[kodebach]

I also created a minimal docker image containing clang-format 7.0.1, since Fedora 30 only has packages for clang-format 8 (which sadly formats our code slightly differently because of a few bugfixes).

You can find the image here: https://hub.docker.com/r/kodebach/clang-format
You can also find information on how use it there. Just make sure to use the kodebach/clang-format:7.0.1 image, otherwise you will end up with version 8.0.0.

If you want to use the docker image together with the formatting script, you can create a file called clang-format-7 on your $PATH (e.g. ~/bin/clang-format-7), with the following content:

#!/bin/sh

docker run --user "$(id -u):$(id -g)" -it -v "$(pwd):/workdir" -w /workdir "kodebach/clang-format:7.0.1" "$@"

or with podman:

#!/bin/sh

podman run -it -v "$(pwd):/workdir:z" -w /workdir "kodebach/clang-format:7.0.1" "$@"

Note: there is a slight difference between the docker image and normal clang-format. The docker image invokes clang-format --help, if no arguments are given.

[alexsaber]

@sanssecours Thank you very much! The proposed Docker solution worked for me! I close the issue.