Cover Linux build by GitHub Actions

[hartwork]

Hi @gkdr,

here's a small PR with a small CI to notify us when things break on Linux, the most recent Ubuntu that GitHub Actions has (while avoiding moving target ubuntu-latest). It targets branch dev, runs every week (in addition to PRs and pushes) so that we'll get notified when the ground shifts below out feet. It also configures GitHub Dependabot to send PRs (like this one) to us when action actions/checkout@v2.4.0 has a more recent release in the future (also avoiding moving Git tag v2).

Before the PR is merged, expected behavior can already be seen over at https://github.com/hartwork/axc/runs/4846368005?check_suite_focus=true .

Looking forward to review, best, Sebastian

[gkdr]

thanks a lot for your help! looks pretty good.

(just as an aside, doesn't have to be now: do you know if it's possible to get build artifacts from github actions? i'm thinking of the built binaries and the test an coverage reports maybe. edit: found it already, that seems easy: https://github.com/marketplace/actions/upload-a-build-artifact)

also, could you also add a line to the changelog? 🙂

[gkdr]

it's not just all because it doesn't cover the "no threads" version, right?

[hartwork]

Yes, that's why I have two targets all build/libaxc-nt.a up there 😄

[gkdr]

if i understand the makefile right, this way the tests only run against the installed dependency, not the libsignal compiled form the submodule. correct?

what do you think about building + testing the static targets without the dependency installed in one step, and then installing it to build and test the shared lib in the next step? ("step" does not necessarily mean a github action step)

[hartwork]

if i understand the makefile right, this way the tests only run against the installed dependency, not the libsignal compiled form the submodule. correct?

I confirm:

# lddtree ./test/test_client.o | fgrep signal
    libsignal-protocol-c.so.2 => /usr/lib64/libsignal-protocol-c.so.2

(Notice the .o suffix in that line. That's where the Makefile stores the ELF executable at the moment.)

what do you think about building + testing the static targets without the dependency installed in one step, and then installing it to build and test the shared lib in the next step? ("step" does not necessarily mean a github action step)

I agree that it would be nice to have the build system be flexible enough to do things like that. There are two reasons why I'd rather not change anything about that yet in this very pull request: The mission of the pull request mostly is detecting breakage in what we already have; re-placing the low-level Makefile with high-level CMake is something that I will investigate further, which would make prior non-critical build system changes a potential waste of time. What do you think?

[gkdr]

oh, i don't mean changing anything about the build system. afaict the test is either run against the static or dynamic build depending on whether the system package is installed:

axc/Makefile

Line 135 in 1d4454e

ifneq ($(REQPKG),)

so my suggestion was:

1. do not install libsignal-protocol-c package
2. build the static targets
3. run tests
4. clean
5. do install the package after
6. build the dynamic target
7. run tests again

i agree that seems a bit silly, and just an easy workaround until there's a proper way to do it.

[hartwork]

@gkdr I see, I did indeed misunderstand you then. Rather than doing all of that in one linear run, I'd split that in two halves an use the GitHub Action Matrix to with a single parameter for system-wide or self-built libsignal-protocol-c. It's not hard to do with regard to GitHub Actions:

diff --git a/.github/workflows/linux.yml b/.github/workflows/linux.yml
index 292f485..e054727 100644
--- a/.github/workflows/linux.yml
+++ b/.github/workflows/linux.yml
@@ -13,11 +13,14 @@ jobs:
   checks:
     name: Build for Linux
     runs-on: ubuntu-20.04
+    strategy:
+      matrix:
+        libsignal: ['system libsignal', 'bundled libsignal']
     steps:
 
     - uses: actions/checkout@v2.4.0
 
-    - name: Install build dependencies
+    - name: Install build dependencies (all but libsignal-protocol-c)
       run: |-
         set -x
         sudo apt-get update
@@ -26,9 +29,19 @@ jobs:
             libcmocka-dev \
             libgcrypt20-dev \
             libglib2.0-dev \
-            libsignal-protocol-c-dev \
             libsqlite3-dev
 
+    - name: Install build dependency libsignal-protocol-c
+      if: ${{ matrix.libsignal == 'system libsignal' }}
+      run: |-
+        sudo apt-get install --yes --no-install-recommends -V \
+            libsignal-protocol-c-dev
+
+    - name: Fetch Git submodule for build dependency libsignal-protocol-c
+      if: ${{ matrix.libsignal == 'bundled libsignal' }}
+      run: |-
+        git submodule update --init --recursive
+
     - name: Build
       run: |-
         set -x

Problem is: The self-build half of things is broken, and so we won't get a green CI before fixing the build system further this way. The symptom (full build log) is…

# make -j 2 all build/libaxc-nt.a
[..]
cc -shared -Wl,-soname,libaxc.so.0 -o build/libaxc.so -fPIC -I/usr/include/glib-2.0 -I/usr/lib/x86_64-linux-gnu/glib-2.0/include   -I./lib/libsignal-protocol-c/src -std=c11 -g -Wall -Wextra -Wpedantic -Wstrict-overflow -fno-strict-aliasing -funsigned-char -fno-builtin-memset -fstack-protector-strong -Wformat -Werror=format-security src/axc.c src/axc_crypto.c src/axc_store.c -pthread -ldl -lglib-2.0 -lsqlite3 -L/usr/lib/x86_64-linux-gnu -lgcrypt ./lib/libsignal-protocol-c/build/src/libsignal-protocol-c.a -lm -D_XOPEN_SOURCE=700 -D_BSD_SOURCE -D_POSIX_SOURCE -D_GNU_SOURCE -D_DEFAULT_SOURCE
cc: error: ./lib/libsignal-protocol-c/build/src/libsignal-protocol-c.a: No such file or directory
make: *** [Makefile:99: build/libaxc.so] Error 1
[..]

… and the cause is that $(AX_PATH) is missing at some places but also needs to be fully conditional, to not interfere with the build against the system-wide library (e.g. when someone does have the Git submodule initialized)… I would personally not want us to fix that in here, because it's not a hard dependency (plus potential upcoming build system re-write).

[hartwork]

@gkdr I guess someone had to fix the build anyway, so I have added the matrix and a dedicated commit to fix the build for the case of self-built libsignal-protocol-c now.

[hartwork]

(just as an aside, doesn't have to be now: do you know if it's possible to get build artifacts from github actions? i'm thinking of the built binaries and the test an coverage reports maybe. edit: found it already, that seems easy: https://github.com/marketplace/actions/upload-a-build-artifact)

I'll go store the coverage files as an artifact now 👍
For Linux, I'm not sure if the built binaries have much worth, do they?

also, could you also add a line to the changelog? slightly_smiling_face

Will do, back in a few minutes 👍

[gkdr]

very nice, thank you!

[gkdr]

before i merge - do you want to keep the individual commits or can i squash the branch into one commit?

[hartwork]

before i merge - do you want to keep the individual commits or can i squash the branch into one commit?

Thanks for asking! Please keep the individual commits separate: They are 3 semantically independent units, and I see value in communicating them as such in Git history. Thank you!