From cde0c8ea3f2809b6a953d9e1785d06ed6e607487 Mon Sep 17 00:00:00 2001 From: Joshua Newton Date: Tue, 25 Jun 2024 14:35:07 -0400 Subject: [PATCH 1/8] Move non-data steps into separate workflows This will make it so that simple errors won't stop other checks, while also making it easier for me to focus on the "core" changes. --- .github/workflows/check-line-endings.yml | 44 ++++++++++++++++++++++++ .github/workflows/lint-participants.yml | 28 +++++++++++++++ .github/workflows/validator.yml | 22 ------------ 3 files changed, 72 insertions(+), 22 deletions(-) create mode 100644 .github/workflows/check-line-endings.yml create mode 100644 .github/workflows/lint-participants.yml diff --git a/.github/workflows/check-line-endings.yml b/.github/workflows/check-line-endings.yml new file mode 100644 index 0000000000..51ffa1f0aa --- /dev/null +++ b/.github/workflows/check-line-endings.yml @@ -0,0 +1,44 @@ + +name: Check for Windows line endings +on: + push: + branches: [ master ] + pull_request: + branches: [ master ] + +jobs: + check-line-endings: + runs-on: ubuntu-latest + + steps: + + - name: Checkout + uses: actions/checkout@v2 + with: + # make sure to download directly from the PR's repo, whether that is this repo or a fork + # By default github generates a merge commit for each PR in this repo, but only for the one branch under test + # but `git-annex` needs access to *two* branches: the current branch and `git-annex` + # this might be subtly buggy since it is testing the remote version, not the merged version + # + # *if* this is not a pull request, this will fall back to its default behaviour. + repository: ${{github.event.pull_request.head.repo.full_name}} + ref: ${{ github.event.pull_request.head.ref }} + + - name: Install dos2unix + run: | + # the easy way to do this: convert everything to unix and ask git if that changed anything + sudo apt-get install -y dos2unix + + - name: Check line endings + run: | + # the easy way to do this: convert everything to unix and ask git if that changed anything + + find ./ -path ./.git -prune -o -type f -print0 | xargs -0 dos2unix -q + # this version is safer, but more maintenance: + #find ./ -path ./.git -prune -o -type f \( ! -name "*.nii" -a ! -name "*.nii.gz" \) -print0 | xargs -0 dos2unix -q + + git diff --stat --exit-code + if [ "$?" -ne 0 ]; then + echo "error: Windows line endings found." + exit 1 + fi diff --git a/.github/workflows/lint-participants.yml b/.github/workflows/lint-participants.yml new file mode 100644 index 0000000000..50365930b6 --- /dev/null +++ b/.github/workflows/lint-participants.yml @@ -0,0 +1,28 @@ + +name: Lint participants.tsv file +on: + push: + branches: [ master ] + pull_request: + branches: [ master ] + +jobs: + lint-participants: + runs-on: ubuntu-latest + + steps: + + - name: Checkout + uses: actions/checkout@v2 + with: + # make sure to download directly from the PR's repo, whether that is this repo or a fork + # By default github generates a merge commit for each PR in this repo, but only for the one branch under test + # but `git-annex` needs access to *two* branches: the current branch and `git-annex` + # this might be subtly buggy since it is testing the remote version, not the merged version + # + # *if* this is not a pull request, this will fall back to its default behaviour. + repository: ${{github.event.pull_request.head.repo.full_name}} + ref: ${{ github.event.pull_request.head.ref }} + + - name: Lint participants.tsv + run: .github/workflows/lint-tsv participants.tsv diff --git a/.github/workflows/validator.yml b/.github/workflows/validator.yml index 982915084f..253c639a0b 100644 --- a/.github/workflows/validator.yml +++ b/.github/workflows/validator.yml @@ -24,28 +24,6 @@ jobs: repository: ${{github.event.pull_request.head.repo.full_name}} ref: ${{ github.event.pull_request.head.ref }} - - name: Install dos2unix - run: | - # the easy way to do this: convert everything to unix and ask git if that changed anything - sudo apt-get install -y dos2unix - - - name: Check line endings - run: | - # the easy way to do this: convert everything to unix and ask git if that changed anything - - find ./ -path ./.git -prune -o -type f -print0 | xargs -0 dos2unix -q - # this version is safer, but more maintenance: - #find ./ -path ./.git -prune -o -type f \( ! -name "*.nii" -a ! -name "*.nii.gz" \) -print0 | xargs -0 dos2unix -q - - git diff --stat --exit-code - if [ "$?" -ne 0 ]; then - echo "error: Windows line endings found." - exit 1 - fi - - - name: Lint participants.tsv - run: .github/workflows/lint-tsv participants.tsv - #- name: Update software # run: | # # do we want to do this? it's helpful to avoid testing against surprise out-of-date software, but also so slow. From 18f52f062166d16f2d6cbc87e95b58fcf32a2c6b Mon Sep 17 00:00:00 2001 From: Joshua Newton Date: Tue, 25 Jun 2024 14:38:36 -0400 Subject: [PATCH 2/8] `validator.yml`: Use action to free up space --- .github/workflows/validator.yml | 18 ++++++++++-------- 1 file changed, 10 insertions(+), 8 deletions(-) diff --git a/.github/workflows/validator.yml b/.github/workflows/validator.yml index 253c639a0b..337c3355ef 100644 --- a/.github/workflows/validator.yml +++ b/.github/workflows/validator.yml @@ -12,6 +12,16 @@ jobs: steps: + - name: Gain disk space by remapping temp disk (`/dev/sdb1`) + uses: easimon/maximize-build-space@master + # We use the above action instead of removing unwanted packages because it only takes about 2s; + # by comparison, removing lots of small file is very time-consuming. + with: + # After freeing up space we could have as much as 87GB available, which is overkill. + # So, we set root == 10GB to save room for installing dependencies (leaving us with ~77GB) + root-reserve-mb: 10240 + swap-size-mb: 1024 + - name: Checkout uses: actions/checkout@v2 with: @@ -51,14 +61,6 @@ jobs: - name: Install spine-generic for acquisition parameters check run: pip install spinegeneric@git+https://github.com/spine-generic/spine-generic.git@master - #- name: Increase free space - # run: | - # # this takes about 2 minutes but saves about 30GB, which is space we might need for git-annex. - # # annex.thin saves a lot of space, but if the dataset grows beyond what Github can handle - # # try enabling this. - # # ref: https://github.com/actions/virtual-environments/issues/2606#issuecomment-772683150 - # sudo rm -rf /usr/local/lib/android /usr/share/dotnet - - name: git config run: | # this is needed by git-annex so that it can write to the local git-annex branch From 0e4a454a2bbe5a0fc0808dc7428500118cdea765 Mon Sep 17 00:00:00 2001 From: Joshua Newton Date: Tue, 25 Jun 2024 16:24:06 -0400 Subject: [PATCH 3/8] `validator.yml`: Add retry setting for flaky `-J8` downloads --- .github/workflows/validator.yml | 1 + 1 file changed, 1 insertion(+) diff --git a/.github/workflows/validator.yml b/.github/workflows/validator.yml index 337c3355ef..6cd08f7a6e 100644 --- a/.github/workflows/validator.yml +++ b/.github/workflows/validator.yml @@ -45,6 +45,7 @@ jobs: run: | sudo apt-get install -y git-annex git config --global annex.thin true + git config --global annex.retry 2 - name: Install bids-validator run: | From 20f50d688f6764cffc3a3e1efab0f5899781ba88 Mon Sep 17 00:00:00 2001 From: Joshua Newton Date: Tue, 25 Jun 2024 16:25:56 -0400 Subject: [PATCH 4/8] `validator.yml`: Move NodeJS install to separate step --- .github/workflows/validator.yml | 9 ++++----- 1 file changed, 4 insertions(+), 5 deletions(-) diff --git a/.github/workflows/validator.yml b/.github/workflows/validator.yml index 6cd08f7a6e..4833afaee2 100644 --- a/.github/workflows/validator.yml +++ b/.github/workflows/validator.yml @@ -47,12 +47,11 @@ jobs: git config --global annex.thin true git config --global annex.retry 2 + - name: Setup NodeJS (for bids-validator) + uses: actions/setup-node@v4 + - name: Install bids-validator - run: | - # install proper NodeJS for bids-validator - curl -sL https://deb.nodesource.com/setup_current.x | sudo -E bash - - sudo apt-get install -y nodejs - sudo npm install -g bids-validator + run: sudo npm install -g bids-validator - name: Set up Python uses: actions/setup-python@v2 From 2a14341351f570bb7f4d67d8402d2cf9e1062ba5 Mon Sep 17 00:00:00 2001 From: Joshua Newton Date: Wed, 26 Jun 2024 21:52:21 -0400 Subject: [PATCH 5/8] `validator.yml`: Update to Py3.11 --- .github/workflows/validator.yml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/.github/workflows/validator.yml b/.github/workflows/validator.yml index 4833afaee2..38f71dcd7b 100644 --- a/.github/workflows/validator.yml +++ b/.github/workflows/validator.yml @@ -56,7 +56,7 @@ jobs: - name: Set up Python uses: actions/setup-python@v2 with: - python-version: 3.7 + python-version: 3.11 - name: Install spine-generic for acquisition parameters check run: pip install spinegeneric@git+https://github.com/spine-generic/spine-generic.git@master From 2a5e432af322d6541897fbd63ca2fbe231206592 Mon Sep 17 00:00:00 2001 From: Joshua Newton Date: Wed, 26 Jun 2024 23:58:35 -0400 Subject: [PATCH 6/8] `validator.yml`: Fail on warnings, but run all steps too --- .github/workflows/validator.yml | 22 +++++++++++++++++++--- 1 file changed, 19 insertions(+), 3 deletions(-) diff --git a/.github/workflows/validator.yml b/.github/workflows/validator.yml index 38f71dcd7b..e8cb649953 100644 --- a/.github/workflows/validator.yml +++ b/.github/workflows/validator.yml @@ -78,8 +78,24 @@ jobs: git annex get -J8 - name: Checking BIDS compliance - run: bids-validator --verbose ./ + if: always() + run: | + bids-validator --verbose ./ | tee bids.txt + printf "\nShort summary:\n\n" + # FIXME: This should be replaced with bids-validator settings: A) throw error on warning or B) warning ignores + ! grep "\[WARN\]\|\[ERR\]" bids.txt # ! -> Reverse error code so "found == error thrown" + - name: Checking acquisition parameters - run: sg_params_checker -path-in ./ + if: always() + run: | + sg_params_checker -path-in ./ | tee params.txt + # FIXME: Checking the output like this should be replaced with SG throwing actual errors + ! grep "WARNING" params.txt # ! -> Reverse error code so "found == error thrown" + - name: Checking data consistency - run: sg_check_data_consistency -path-in ./ + if: always() + run: | + sg_check_data_consistency -path-in ./ | tee consistency.txt + # FIXME: Checking the output like this should be replaced with SG throwing actual errors + ! grep "Warning\|Missing" consistency.txt + grep "all good" consistency.txt From 9a19675a5a582ad26ee13af0bc5dd2ca63a29d57 Mon Sep 17 00:00:00 2001 From: Joshua Newton Date: Thu, 27 Jun 2024 00:19:10 -0400 Subject: [PATCH 7/8] `validator.yml`: Update Ubuntu packages --- .github/workflows/validator.yml | 12 ++++++------ 1 file changed, 6 insertions(+), 6 deletions(-) diff --git a/.github/workflows/validator.yml b/.github/workflows/validator.yml index e8cb649953..60a186432f 100644 --- a/.github/workflows/validator.yml +++ b/.github/workflows/validator.yml @@ -34,12 +34,12 @@ jobs: repository: ${{github.event.pull_request.head.repo.full_name}} ref: ${{ github.event.pull_request.head.ref }} - #- name: Update software - # run: | - # # do we want to do this? it's helpful to avoid testing against surprise out-of-date software, but also so slow. - # sudo apt-get update && - # sudo DEBIAN_FRONTEND=noninteractive apt-get -y dist-upgrade && - # sudo DEBIAN_FRONTEND=noninteractive apt-get autoremove + - name: Update apt packages + run: | + # do we want to do this? it's helpful to avoid testing against surprise out-of-date software, but also so slow. + sudo apt-get update && + sudo DEBIAN_FRONTEND=noninteractive apt-get -y dist-upgrade && + sudo DEBIAN_FRONTEND=noninteractive apt-get autoremove - name: Install git-annex run: | From 9ca106d72cf3ab45f43f72ea8d17368c18347e8d Mon Sep 17 00:00:00 2001 From: Joshua Newton Date: Thu, 27 Jun 2024 00:28:28 -0400 Subject: [PATCH 8/8] `validator.yml`: Update outdated NodeJS actions --- .github/workflows/validator.yml | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/.github/workflows/validator.yml b/.github/workflows/validator.yml index 60a186432f..e9c438299f 100644 --- a/.github/workflows/validator.yml +++ b/.github/workflows/validator.yml @@ -23,7 +23,7 @@ jobs: swap-size-mb: 1024 - name: Checkout - uses: actions/checkout@v2 + uses: actions/checkout@v4 with: # make sure to download directly from the PR's repo, whether that is this repo or a fork # By default github generates a merge commit for each PR in this repo, but only for the one branch under test @@ -54,7 +54,7 @@ jobs: run: sudo npm install -g bids-validator - name: Set up Python - uses: actions/setup-python@v2 + uses: actions/setup-python@v5 with: python-version: 3.11 @@ -73,7 +73,7 @@ jobs: - name: Download dataset run: | - git fetch --depth=1 origin git-annex:git-annex # actions/checkout@v2 does a shallow checkout, so it is missing this important branch + git fetch --depth=1 origin git-annex:git-annex # actions/checkout@v4 does a shallow checkout, so it is missing this important branch git annex init git annex get -J8