Skip to content

Conversation

@csegarragonz
Copy link
Collaborator

@csegarragonz csegarragonz commented Oct 21, 2022

This is the first of another three-part PR (faabric, cpp, faasm) to fix some issues with our CI/CD pipeline.

The first one is moving all the code-checking-out to use actions/checkout. We were running git commands manually in the actions, and this was causing spurious errors with submodules not being fetched sometimes. There is one caveat to checking out code using actions/checkout: code is checked out under the ${GITHUB_WORKSPACE} directory. Inside a container, this resolves to /w__/faasm/faasm. In most of our actions, we hard-code the directory shell scripts run in, which means that the steps would be running in the version of the code that shipped with the container, not the latest commit. The underlying question is if code can be properly built and used inside the container without being checked out to our well known paths (/code/faabric, /code/cpp, and /usr/local/code/faasm). AFAICT there's no reason why this should not work, only a couple of things:

  • Hardcoded paths in ./bin/workon.sh and ./bin/create_venv.sh need to be updated.
  • If we use actions/checkout across (including faasm/conan-cache-action) then we don't need to set up any directory: variables in any steps (including the conan action).
  • We probably have to build the targets in GHA with the --clean flag. This is because the CMakeCache will point to a different source directory. In practice, this should not increase the tests runtime as, by compiling with sanitisers, we are effectively compiling everything from scratch already.

The second one is using a very neat project I came across. It is a GH action that cancels previously running jobs for the same PR.

The third one is moving away from our bash scripts to format code, and use a python task instead: inv format-code [--check].

The fourth one is that a recent patched vulnerability in git makes it necessary to add the safe-path's for the git command to run under certain combinations of parent-child repo ownership. As a rule of thumb I think we want to add our well-known paths where we check out code inside the containers as safe paths.

@csegarragonz csegarragonz self-assigned this Oct 21, 2022
This was referenced Oct 24, 2022
@csegarragonz csegarragonz merged commit dbaf5a2 into main Oct 25, 2022
@csegarragonz csegarragonz deleted the ci branch October 25, 2022 13:12
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants