Re: [PATCH v2 0/5] ci: add support for macOS to GitLab CI

[Phillip Wood <phillip.wood123@gmail.com>]

Hi Patrick

On 18/01/2024 10:22, Patrick Steinhardt wrote:
> Hi,
> 
> this is the second version of my patch series that adds a macOS job to
> GitLab CI. Changes compared to v1:
> 
>    - Added a fix for a flaky test in t7527 that caused the pipeline to
>      fail in ~50% of all runs.
> 
>    - Improved some commit messages.
> 
>    - Tests now write test data into a RAMDisk. This speeds up tests and
>      fixes some hung pipelines I was seeing.
> 
> Thanks for your reviews so far!

I've read though all the patches and they seem sensible to me though I'm 
hardly a macOS expert. I did wonder about the use of pushd/popd in the 
fourth patch as they are bashisms but that matches what we're doing on 
Ubuntu already. It's nice to see the GitLab CI running on macOS as well 
as Linux now.

Best Wishes

Phillip

> Patrick
> 
> Patrick Steinhardt (5):
>    t7527: decrease likelihood of racing with fsmonitor daemon
>    Makefile: detect new Homebrew location for ARM-based Macs
>    ci: handle TEST_OUTPUT_DIRECTORY when printing test failures
>    ci: make p4 setup on macOS more robust
>    ci: add macOS jobs to GitLab CI
> 
>   .gitlab-ci.yml               | 34 +++++++++++++++++++++++++++++++++-
>   ci/install-dependencies.sh   | 10 ++++------
>   ci/lib.sh                    | 12 +++++++++++-
>   ci/print-test-failures.sh    |  2 +-
>   config.mak.uname             | 13 +++++++++++++
>   t/t7527-builtin-fsmonitor.sh |  2 +-
>   6 files changed, 63 insertions(+), 10 deletions(-)
> 
> Range-diff against v1:
> -:  ---------- > 1:  554b1c8546 t7527: decrease likelihood of racing with fsmonitor daemon
> 2:  3adb0b7ae8 = 2:  32d8bd1d78 Makefile: detect new Homebrew location for ARM-based Macs
> -:  ---------- > 3:  d55da77747 ci: handle TEST_OUTPUT_DIRECTORY when printing test failures
> 1:  a5d725bea7 ! 4:  1ed6e68650 ci: make p4 setup on macOS more robust
>      @@ Commit message
>           into a separate directory which we then manually append to our PATH.
>           This matches what we do on Linux-based jobs.
>       
>      +    Note that it may seem like we already did append "$HOME/bin" to PATH
>      +    because we're actually removing the lines that adapt PATH. But we only
>      +    ever adapted the PATH variable in "ci/install-dependencies.sh", and
>      +    didn't adapt it when running "ci/run-build-and-test.sh". Consequently,
>      +    the required binaries wouldn't be found during the test run unless the
>      +    CI platform already had the "$HOME/bin" in PATH right from the start.
>      +
>           Signed-off-by: Patrick Steinhardt <ps@pks.im>
>       
>        ## ci/install-dependencies.sh ##
> 3:  d196cfd9d0 ! 5:  c5ed38f0a6 ci: add macOS jobs to GitLab CI
>      @@ Metadata
>        ## Commit message ##
>           ci: add macOS jobs to GitLab CI
>       
>      -    Add two macOS-based jobs to GitLab CI, one for Clang and one for GCC.
>      -    This matches equivalent jobs we have for GitHub Workflows, except that
>      -    we use macOS 14 instead of macOS 13.
>      +    Add a job to GitLab CI which runs tests on macOS, which matches the
>      +    equivalent "osx-clang" job that we have for GitHub Workflows. One
>      +    significant difference though is that this new job runs on Apple M1
>      +    machines and thus uses the "arm64" architecture. As GCC does not yet
>      +    support this comparatively new architecture we cannot easily include an
>      +    equivalent for the "osx-gcc" job that exists in GitHub Workflows.
>       
>           Note that one test marked as `test_must_fail` is surprisingly passing:
>       
>             t7815-grep-binary.sh                             (Wstat: 0 Tests: 22 Failed: 0)
>               TODO passed:   12
>       
>      -    This seems to boil down to an unexpected difference in how regcomp(1)
>      +    This seems to boil down to an unexpected difference in how regcomp(3P)
>           works when matching NUL bytes. Cross-checking with the respective GitHub
>      -    job shows though that this is not an issue unique to the GitLab CI job
>      -    as it passes in the same way there.
>      -
>      -    Further note that we do not include the equivalent for the "osx-gcc" job
>      -    that we use with GitHub Workflows. This is because the runner for macOS
>      -    on GitLab is running on Apple M1 machines and thus uses the "arm64"
>      -    architecture. GCC does not support this platform yet.
>      +    job shows that this is not an issue unique to the GitLab CI job as it
>      +    passes in the same way there.
>       
>           Signed-off-by: Patrick Steinhardt <ps@pks.im>
>       
>      @@ .gitlab-ci.yml: test:
>       +  image: $image
>       +  tags:
>       +    - saas-macos-medium-m1
>      ++  variables:
>      ++    TEST_OUTPUT_DIRECTORY: "/Volumes/RAMDisk"
>       +  before_script:
>      ++    # Create a 4GB RAM disk that we use to store test output on. This small hack
>      ++    # significantly speeds up tests by more than a factor of 2 because the
>      ++    # macOS runners use network-attached storage as disks, which is _really_
>      ++    # slow with the many small writes that our tests do.
>      ++    - sudo diskutil apfs create $(hdiutil attach -nomount ram://8192000) RAMDisk
>       +    - ./ci/install-dependencies.sh
>       +  script:
>       +    - ./ci/run-build-and-tests.sh
>      @@ .gitlab-ci.yml: test:
>       +      if test "$CI_JOB_STATUS" != 'success'
>       +      then
>       +        ./ci/print-test-failures.sh
>      ++        mv "$TEST_OUTPUT_DIRECTORY"/failed-test-artifacts t/
>       +      fi
>       +  parallel:
>       +    matrix:
> 
> base-commit: cd69c635a1a62b0c8bfdbf221778be8a512ad048