ReproNim / containers

Containers "distribution" for reproducible neuroimaging
Apache License 2.0
26 stars 14 forks source link

Switch from Travis to GitHub Actions #99

Closed jwodder closed 5 months ago

jwodder commented 1 year ago

Closes #98.

jwodder commented 1 year ago

@yarikoptic The tests are failing on macOS because the dummy Docker script isn't being passed the arguments it expects during the script tests. Instead of a command line ending with:

--pwd /singularity 'foo bar' blah 45.5 /dir 'bar;' 'foo&' '\${foo}'

it's getting:

--pwd '/tmp/tmp dir.XXXXXX.cieII6oF' /Users/runner/work/containers/containers/scripts/tests/arg-test.simg sh -c 'find /tmp /var/tmp && cat "/tmp/tmp dir.XXXXXX.cieII6oF/file"'

Also, shellcheck is printing out a bunch of messages, but it's still exiting 0, so no one will ever check to see them.

jwodder commented 1 year ago

@yarikoptic I've fixed the shellcheck issue, though I had to skip checking of scripts/utils/get-rid-of-datalad-remote.sh, which is a zsh script, which shellcheck does not support.

yarikoptic commented 1 year ago

Awesome, thank you! And that script was one trick pony anyways

yarikoptic commented 1 year ago

ok, one problem as identified by tests

line 130: mapfile: command not found

happening on OSX, according to https://github.com/jitterbit/get-changed-files/issues/15#issuecomment-724909010 :+1:

The problem is that macOS has bash 3 and mapfile was introduced in bash 4. I found a solution, add this step in front in your workflow:

- if: contains( matrix.os, 'macos')
  name: Install bash 5.0 under macOS for mapfile
  run: |
    brew install bash
    sudo mv /usr/local/bin/bash /bin/bash

but I would like to avoid that since then OSX folks might be left behind... will think about it...

jwodder commented 10 months ago

@yarikoptic The mapfile thing is addressed by #105. The remaining, bigger problem is still https://github.com/ReproNim/containers/pull/99#issuecomment-1563453156.

yarikoptic commented 7 months ago

rebased, force pushed.

yarikoptic commented 5 months ago

ok, green now (after we just started to skip here too ;) ) -- so let's merge and consider "DONE" ;)