ci: Move linter task to cirrus #20658

pull dhruv wants to merge 2 commits into bitcoin:master from dhruv:linter-on-cirrus changing 5 files +53 −97
  1. dhruv commented at 1:07 am on December 15, 2020: member
    Solves #20467: Move linter to Cirrus-CI as Travis-CI.org is shutting down
  2. fanquake added the label Tests on Dec 15, 2020
  3. in ci/lint/lint_run_all.sh:13 in 52b724efb3 outdated 
     8+
 9+set -o errexit; source ./ci/test/00_setup_env.sh
10+set -o errexit; source ./ci/test/03_before_install.sh
11+set -o errexit; source ./ci/lint/04_install.sh
12+set -o errexit; source ./ci/lint/05_before_script.sh
13+set -o errexit; source ./ci/lint/06_script.sh
    MarcoFalke commented at 7:01 am on December 15, 2020:
    The linter scripts heavily depend on the ci platform because they use $TRAVIS or $CIRRUS env vars, so I think the script here can just be inlined into the platform.yml
  4. in ci/lint/05_before_script.sh:9 in 52b724efb3 outdated 
     5@@ -6,4 +6,4 @@
 6 
 7 export LC_ALL=C
 8 
 9-git fetch --unshallow
10+git fetch
    MarcoFalke commented at 7:02 am on December 15, 2020:
    why?
    dhruv commented at 5:36 pm on December 15, 2020:

    On Cirrus-CI, I received:

    fatal: --unshallow on a complete repository does not make sense

  5. in ci/lint/04_install.sh:16 in 52b724efb3 outdated 
    18-travis_retry pip3 install yq
19-travis_retry pip3 install mypy==0.781
20+pip3 install codespell==1.17.1
21+pip3 install flake8==3.8.3
22+pip3 install yq
23+pip3 install mypy==0.781
    MarcoFalke commented at 7:03 am on December 15, 2020:
    there is ./ci/retry, which can be used instead
  6. in .cirrus.yml:61 in 52b724efb3 outdated 
    53@@ -54,6 +54,15 @@ global_task_template: &GLOBAL_TASK_TEMPLATE
54 #  install_script:
55 #    - choco install python --version=3.7.7 -y
56 
57+lint_task:
58+  name: 'lint'
59+  skip: $CIRRUS_REPO_FULL_NAME == "bitcoin-core/gui" && $CIRRUS_PR == ""  # No need to run on the read-only mirror, unless it is a PR.
60+  container:
61+    image: ubuntu:bionic
    MarcoFalke commented at 7:05 am on December 15, 2020:

    Please re-add the comment here that says

    0    For   python: '3.6' # Oldest supported version according to doc/dependencies.md
  7. in .cirrus.yml:63 in 52b724efb3 outdated 
    58+  name: 'lint'
59+  skip: $CIRRUS_REPO_FULL_NAME == "bitcoin-core/gui" && $CIRRUS_PR == ""  # No need to run on the read-only mirror, unless it is a PR.
60+  container:
61+    image: ubuntu:bionic
62+  pip_cache:
63+    folder: /tmp/pip_cache_dir
    MarcoFalke commented at 7:07 am on December 15, 2020:
    Is this really needed? I somehow supspect that setting up the cache and fetching from it is slower than fetching from the pip servers directly. Or at the least, it is not measurable and we don’t care about a few milliseconds here or there anyway.
  8. in .cirrus.yml:63 in 52b724efb3 outdated 
    53@@ -54,6 +54,15 @@ global_task_template: &GLOBAL_TASK_TEMPLATE
54 #  install_script:
55 #    - choco install python --version=3.7.7 -y
56 
57+lint_task:
58+  name: 'lint'
59+  skip: $CIRRUS_REPO_FULL_NAME == "bitcoin-core/gui" && $CIRRUS_PR == ""  # No need to run on the read-only mirror, unless it is a PR.
    MarcoFalke commented at 7:08 am on December 15, 2020:
    Maybe move this to a GLOBAL_BASE_TEMPLATE to avoid duplication?
  9. MarcoFalke approved
  10. MarcoFalke commented at 7:08 am on December 15, 2020: member
    ACK, just some nits
  11. in .cirrus.yml:64 in 52b724efb3 outdated 
    59+  skip: $CIRRUS_REPO_FULL_NAME == "bitcoin-core/gui" && $CIRRUS_PR == ""  # No need to run on the read-only mirror, unless it is a PR.
60+  container:
61+    image: ubuntu:bionic
62+  pip_cache:
63+    folder: /tmp/pip_cache_dir
64+  script: set -o errexit; source ./ci/lint/lint_run_all.sh
    MarcoFalke commented at 7:10 am on December 15, 2020:

    Could run the linters with compute credits? The most frustrating thing is waiting for the linters to fail (or pass)

    0  # For faster CI feedback, immediately schedule the linters. https://cirrus-ci.org/pricing/#compute-credits
1  use_compute_credits: $CIRRUS_REPO_FULL_NAME == 'bitcoin/bitcoin'
  12. DrahtBot commented at 7:40 am on December 15, 2020: member

    The following sections might be updated with supplementary metadata relevant to reviewers and maintainers.

    Conflicts

    Reviewers, this pull request conflicts with the following ones:

    • #19952 (build, ci: Add file-based logging for individual packages by hebasto)

    If you consider this pull request important, please also help to review the conflicting pull requests. Ideally, start with the one that should be merged first.

  13. dhruv force-pushed on Dec 15, 2020
  14. dhruv commented at 5:37 pm on December 15, 2020: member
    Comments addressed. Ready for re-review.
  15. dhruv force-pushed on Dec 15, 2020
  16. in .cirrus.yml:21 in 38baea179a outdated 
    14@@ -15,11 +15,14 @@ env:
15   CCACHE_SIZE: "200M"
16   CCACHE_DIR: "/tmp/ccache_dir"
17 
18-### Global task template
19-
20+### Base template
21 # https://cirrus-ci.org/guide/tips-and-tricks/#sharing-configuration-between-tasks
22+base_template: &BASE_TEMPLATE
23+  skip: $CIRRUS_REPO_FULL_NAME == "bitcoin-core/gui" && $CIRRUS_PR == "" # No need to run on the read-only mirror, unless it is a PR. https://cirrus-ci.org/guide/writing-tasks/#conditional-task-execution
    MarcoFalke commented at 5:49 pm on December 15, 2020:
    merge_base_script should also be part of the base template
  17. in .cirrus.yml:60 in 38baea179a outdated 
    56@@ -54,6 +57,20 @@ global_task_template: &GLOBAL_TASK_TEMPLATE
57 #  install_script:
58 #    - choco install python --version=3.7.7 -y
59 
60+lint_task:
    MarcoFalke commented at 5:50 pm on December 15, 2020: 
    0task:

    Can be shorter (because you provided a name: )

  18. in .cirrus.yml:64 in 38baea179a outdated 
    56@@ -54,6 +57,20 @@ global_task_template: &GLOBAL_TASK_TEMPLATE
57 #  install_script:
58 #    - choco install python --version=3.7.7 -y
59 
60+lint_task:
61+  name: 'lint'
62+  << : *BASE_TEMPLATE
63+  container:
64+    image: ubuntu:bionic # For python 3.6, oldest supported version according to doc/dependencies.md
    MarcoFalke commented at 5:51 pm on December 15, 2020:
    Can reduce the cpu to 1 to cut the bill in half?
    dhruv commented at 7:09 pm on December 15, 2020: 
    0container:
1   image: ubuntu:bionic # For python 3.6, oldest supported version according to doc/dependencies.md
2   cpu: 1.0 # Cut the bill in half

    Resulted in:

    0Failed to start an instance: INVALID_ARGUMENT: Bad Request.

    Could you share a pointer on how to do this? I found nothing except this in cirrus docs.

    MarcoFalke commented at 6:24 am on December 16, 2020:

    cpu can only be integral, not floating point.

    Also, you might have to move this hunk into the GLOBAL_TASK_TEMPLATE to avoid collisions:

    0timeout_in: 120m  # https://cirrus-ci.org/faq/#instance-timed-out
1container:
2  # https://cirrus-ci.org/faq/#are-there-any-limits
3  # Each project has 16 CPU in total, assign 2 to each container, so that 8 tasks run in parallel
4  cpu: 2
5  memory: 8G  # Set to 8GB to avoid OOM. https://cirrus-ci.org/guide/linux/#linux-containers
6  kvm: true  # Use kvm to avoid spurious CI failures in the default virtualization cluster, see [#20093](/bitcoin-bitcoin/20093/)
    MarcoFalke commented at 6:28 am on December 16, 2020:
    You can also leave it as is for now. Can be addressed later (or in a new commit)
    dhruv commented at 1:55 am on December 17, 2020:
    That seems to have done the trick. I’ve added a commit: 4045a67.
  19. in .cirrus.yml:70 in 38baea179a outdated 
    64+    image: ubuntu:bionic # For python 3.6, oldest supported version according to doc/dependencies.md
65+  # For faster CI feedback, immediately schedule the linters. https://cirrus-ci.org/pricing/#compute-credits
66+  use_compute_credits: $CIRRUS_REPO_FULL_NAME == 'bitcoin/bitcoin'
67+  lint_script:
68+    - set -o errexit; source ./ci/test/00_setup_env.sh
69+    - set -o errexit; source ./ci/test/03_before_install.sh
    MarcoFalke commented at 5:52 pm on December 15, 2020:
    Can each line be a separate script?
  20. in ci/lint/06_script.sh:30 in 38baea179a outdated 
    24@@ -25,8 +25,8 @@ test/lint/check-doc.py
25 test/lint/check-rpc-mappings.py .
26 test/lint/lint-all.sh
27 
28-if [ "$TRAVIS_REPO_SLUG" = "bitcoin/bitcoin" ] && [ "$TRAVIS_EVENT_TYPE" = "cron" ]; then
29+if [ "$CIRRUS_REPO_FULL_NAME" = "bitcoin/bitcoin" ] && [ -n "$CIRRUS_CRON" ]; then
30     git log --merges --before="2 days ago" -1 --format='%H' > ./contrib/verify-commits/trusted-sha512-root-commit
31-    travis_retry gpg --keyserver hkp://keyserver.ubuntu.com:80 --recv-keys $(<contrib/verify-commits/trusted-keys) &&
32+    gpg --keyserver hkp://keyserver.ubuntu.com:80 --recv-keys $(<contrib/verify-commits/trusted-keys) &&
    MarcoFalke commented at 5:56 pm on December 15, 2020:
    the retry missing here?
  21. MarcoFalke approved
  22. MarcoFalke commented at 5:56 pm on December 15, 2020: member
    re-ACK
  23. ci: Move linter task to cirrus 739d39022d
  24. dhruv force-pushed on Dec 15, 2020
  25. hebasto commented at 6:18 pm on December 15, 2020: member
    Concept ACK.
  26. dhruv commented at 7:11 pm on December 15, 2020: member
    Comments addressed. Ready for review.
  27. MarcoFalke commented at 6:26 am on December 16, 2020: member
    ACK 739d39022d2959c1114c14a0065daebf4fe812c1
  28. ci: Use cpu=1 for linter 4045a6722c
  29. dhruv commented at 6:29 am on December 17, 2020: member
    Ready for re-review.
  30. in .cirrus.yml:65 in 4045a6722c 
    61@@ -62,6 +62,7 @@ task:
62   << : *BASE_TEMPLATE
63   container:
64     image: ubuntu:bionic # For python 3.6, oldest supported version according to doc/dependencies.md
65+    cpu: 1 # Cut bill in half for linting
    MarcoFalke commented at 7:46 am on December 17, 2020:

    No need for that comment. Can remove or just say 1 is sufficient

    0    cpu: 1

    0    cpu: 1  # No need for more CPUs
    MarcoFalke commented at 7:47 am on December 17, 2020:

    btw, based on this plot https://cirrus-ci.com/task/4624095184683008

    memory can also be set to 1

  31. MarcoFalke commented at 8:04 am on December 17, 2020: member
    ACK 4045a6722c884be779e86016313061ac6ff80808
  32. MarcoFalke merged this on Dec 17, 2020
  33. MarcoFalke closed this on Dec 17, 2020
  34. hebasto commented at 1:41 pm on December 17, 2020: member

    On my personal account having some “fatal: Not a valid object name master” warnings:

  35. hebasto commented at 1:57 pm on December 17, 2020: member

    Also in the merge commit, https://cirrus-ci.com/task/6086189209878528?command=lint#L50:

    0Skipping shell linting since shellcheck is not installed.
  36. laanwj deleted a comment on Dec 17, 2020
  37. MarcoFalke commented at 2:17 pm on December 17, 2020: member
    Missing packages are installed in #20682
  38. dhruv commented at 3:14 pm on December 17, 2020: member
    Thanks for catching those, @hebasto. I should’ve looked closer.
  39. sidhujag referenced this in commit a0bc41e79f on Dec 17, 2020
  40. sidhujag referenced this in commit 346d54527d on Dec 17, 2020
  41. MarcoFalke referenced this in commit f061da2887 on Dec 21, 2020
  42. sidhujag referenced this in commit 885721f23e on Dec 21, 2020
  43. DrahtBot locked this on Feb 15, 2022


dhruv MarcoFalke DrahtBot hebasto

Labels
Tests

github-metadata-mirror

This is a metadata mirror of the GitHub repository bitcoin/bitcoin. This site is not affiliated with GitHub. Content is generated from a GitHub metadata backup.
generated: 2024-11-16 21:12 UTC
This site is hosted by @0xB10C
More mirrored repositories can be found on mirror.b10c.me