investigate Tricium support for CrOS repos |
||||
Issue descriptionthe http://go/tricium project is bringing support for linting type checks to gerrit codereviews directly. this lets you get integrated linter support as native comments so reviewers can see them directly. it's in dogfooding stages where i think we should be able to play around with it a bit for CrOS integration. our goals would be: - figure out what we can enable for CrOS repos - understand limitations of the system (both current and long term design) - understand how the linters can be configured - enable some linters for CrOS build/infra repos - write documentation so other CrOS developers can turn it on for their repos too as they want we don't plan on "owning" the work to enable it everywhere. we want to manage it for the build/infra repos and leave it to the rest of the team to expand.
,
Jul 10
shellcheck analyzer: crrev.com/c/1132389
,
Jul 10
The following revision refers to this bug: https://chromium.googlesource.com/infra/infra/+/290b71a4e4be614ff304e8b7eff5f325bcdfe6ea commit 290b71a4e4be614ff304e8b7eff5f325bcdfe6ea Author: Lann Martin <lannm@google.com> Date: Tue Jul 10 23:24:05 2018 [tricium] Add shellcheck analyzer Bug: 856733 Change-Id: I3fd5aa4f8155eb59b48b4379b5f3f8b826f7cf71 Reviewed-on: https://chromium-review.googlesource.com/1132389 Commit-Queue: Lann Martin <lannm@chromium.org> Reviewed-by: Quinten Yearsley <qyearsley@chromium.org> [add] https://crrev.com/290b71a4e4be614ff304e8b7eff5f325bcdfe6ea/go/src/infra/tricium/functions/shellcheck/.gitignore [add] https://crrev.com/290b71a4e4be614ff304e8b7eff5f325bcdfe6ea/go/src/infra/tricium/functions/shellcheck/Makefile [add] https://crrev.com/290b71a4e4be614ff304e8b7eff5f325bcdfe6ea/go/src/infra/tricium/functions/shellcheck/fetch_shellcheck.sh [add] https://crrev.com/290b71a4e4be614ff304e8b7eff5f325bcdfe6ea/go/src/infra/tricium/functions/shellcheck/main_test.go [add] https://crrev.com/290b71a4e4be614ff304e8b7eff5f325bcdfe6ea/go/src/infra/tricium/functions/shellcheck/testdata/good.sh [add] https://crrev.com/290b71a4e4be614ff304e8b7eff5f325bcdfe6ea/go/src/infra/tricium/functions/shellcheck/testdata/bad.sh [add] https://crrev.com/290b71a4e4be614ff304e8b7eff5f325bcdfe6ea/go/src/infra/tricium/functions/shellcheck/shellcheck-v0.4.7.linux.x86_64.tar.xz.sum [add] https://crrev.com/290b71a4e4be614ff304e8b7eff5f325bcdfe6ea/go/src/infra/tricium/functions/shellcheck/cipd.yaml [add] https://crrev.com/290b71a4e4be614ff304e8b7eff5f325bcdfe6ea/go/src/infra/tricium/functions/shellcheck/testdata/tricium/data/files.json [add] https://crrev.com/290b71a4e4be614ff304e8b7eff5f325bcdfe6ea/go/src/infra/tricium/functions/shellcheck/main.go [add] https://crrev.com/290b71a4e4be614ff304e8b7eff5f325bcdfe6ea/go/src/infra/tricium/functions/shellcheck/shellcheck.infra_testing [add] https://crrev.com/290b71a4e4be614ff304e8b7eff5f325bcdfe6ea/go/src/infra/tricium/functions/shellcheck/shellcheck_parser.go [add] https://crrev.com/290b71a4e4be614ff304e8b7eff5f325bcdfe6ea/go/src/infra/tricium/functions/shellcheck/README.md
,
Jul 11
I made some updates to support passthrough of --exclude and --shell: https://crrev.com/c/1133436 With those, I have ebuild-specific linting working: https://crrev.com/c/1132381/13/chromeos-config-0.0.1.ebuild#21 Should we turn this on for chromiumos-overlay?
,
Jul 11
sure, lets send a PSA and see how it goes. we'll want to turn it on for all overlays eventually. we know ebuilds won't pass all of the checks, and that's OK, so we need to make sure we're only surfacing checks we actually expect people to fix.
,
Jul 11
you could also turn it on for apps/libapps/ ... that has fewer devs and not as much shell code, but the shell that is in there should be shellcheck clean
,
Jul 11
Can you exclude: 1) seccomp policy files (*.policy) 2) metadata.xml files for ebuilds 3) patch files (*.patch) if you do not exclude these, then please tune the parameters so Tricium doesn't get caught up on the diff syntax. See Tricium comments on https://chromium-review.googlesource.com/c/chromiumos/overlays/chromiumos-overlay/+/1114119 for context.
,
Jul 11
allenwebb@: Sorry, that should not be happening.
,
Jul 11
The following revision refers to this bug: https://chromium.googlesource.com/infra/infra/+/b8ab318528067c55a78033e2fccd6d53a55e3ef2 commit b8ab318528067c55a78033e2fccd6d53a55e3ef2 Author: Lann Martin <lannm@google.com> Date: Wed Jul 11 23:09:33 2018 [tricium] Refactor shellcheck analyzer Separate shellcheck execution logic into separate 'runner' module. Add support for 'exclude' and 'shell' flags. Bug: 856733 Change-Id: I54cdc6ec798c57db26175362b3f55556e4f4c681 Reviewed-on: https://chromium-review.googlesource.com/1133436 Commit-Queue: Lann Martin <lannm@chromium.org> Reviewed-by: Quinten Yearsley <qyearsley@chromium.org> [add] https://crrev.com/b8ab318528067c55a78033e2fccd6d53a55e3ef2/go/src/infra/tricium/functions/shellcheck/runner/shellcheck.go [modify] https://crrev.com/b8ab318528067c55a78033e2fccd6d53a55e3ef2/go/src/infra/tricium/functions/shellcheck/Makefile [add] https://crrev.com/b8ab318528067c55a78033e2fccd6d53a55e3ef2/go/src/infra/tricium/functions/shellcheck/runner/runner.infra_testing [modify] https://crrev.com/b8ab318528067c55a78033e2fccd6d53a55e3ef2/go/src/infra/tricium/functions/shellcheck/main_test.go [modify] https://crrev.com/b8ab318528067c55a78033e2fccd6d53a55e3ef2/go/src/infra/tricium/functions/shellcheck/main.go [modify] https://crrev.com/b8ab318528067c55a78033e2fccd6d53a55e3ef2/go/src/infra/tricium/functions/shellcheck/shellcheck.infra_testing [delete] https://crrev.com/50421359a4b6cd83b9f0821346e817a0a817f43c/go/src/infra/tricium/functions/shellcheck/shellcheck_parser.go
,
Jul 11
I have disabled linting for chromiumos-overlay until I can figure out why the path filters aren't working. The config change may take some minutes to propogate.
,
Jul 11
lets turn it on in a quieter repo next time like chromeos-overlay. and send a psa before turning it on so devs don't get confused.
,
Jul 12
I believe the above issue is fixed. I was just confused about how Tricium was supposed to work. > lets turn it on in a quieter repo next time like chromeos-overlay. I'm not sure we'll learn much from this; chromeos-overlay is a little *too* quiet, mostly just stabilize commits.
,
Jul 12
we'll want it there regardless if you want a slightly more busy repo, go with src/overlays/
,
Jul 12
The following revision refers to this bug: https://chromium.googlesource.com/infra/infra/+/6a6882b329d82ae15832a26ac8f1c32afae89020 commit 6a6882b329d82ae15832a26ac8f1c32afae89020 Author: Lann Martin <lannm@google.com> Date: Thu Jul 12 23:27:52 2018 [tricium] Add -path_filters flag to shellcheck analyzer This is needed to prevent the analyzer from attempting to check non-shell files that happen to be in the same CL as shell file changes. Bug: 856733 Change-Id: Ic381d4b3767114da9f3b42e209b5f58aeaf765ff Reviewed-on: https://chromium-review.googlesource.com/1135578 Reviewed-by: Quinten Yearsley <qyearsley@chromium.org> Commit-Queue: Lann Martin <lannm@chromium.org> [modify] https://crrev.com/6a6882b329d82ae15832a26ac8f1c32afae89020/go/src/infra/tricium/functions/shellcheck/main_test.go [modify] https://crrev.com/6a6882b329d82ae15832a26ac8f1c32afae89020/go/src/infra/tricium/functions/shellcheck/main.go
,
Jul 13
The following revision refers to this bug: https://chromium.googlesource.com/infra/infra/+/1df161f34783f3feb1071f926ff2f91355df6828 commit 1df161f34783f3feb1071f926ff2f91355df6828 Author: Lann Martin <lannm@google.com> Date: Fri Jul 13 16:56:42 2018 [tricium] Fix shellcheck analyzer end position numbering ShellCheck returns line and column positions as 1-based inclusive ranges. Tricium expects 1-based lines, 0-based columns, and exclusive end points for its ranges. Bug: 856733 Change-Id: I4bf8e23c35f799d4f78883ad4ab2bbffad4323f9 Reviewed-on: https://chromium-review.googlesource.com/1136654 Reviewed-by: Quinten Yearsley <qyearsley@chromium.org> Commit-Queue: Lann Martin <lannm@chromium.org> [modify] https://crrev.com/1df161f34783f3feb1071f926ff2f91355df6828/go/src/infra/tricium/functions/shellcheck/main_test.go [modify] https://crrev.com/1df161f34783f3feb1071f926ff2f91355df6828/go/src/infra/tricium/functions/shellcheck/main.go
,
Jul 13
,
Jul 31
Seems to be viable. Tracking creation of `cros lint` checker here: crbug.com/869432
,
Jul 31
still need docs written up :)
,
Aug 28
Expand shellcheck coverage to src/platform/dev/
,
Sep 11
There isn't much to document apart from the go/tricium "User guide" and "Contribute" docs. I think we should not try to maintain multiple checkers (like the shellcheck checker above) and instead wrap up `cros lint` as an omni-checker: crbug.com/869432
,
Sep 11
Oh I guess the configuration is important: The tricium.cfg file is on the magical `infra/config` branch of the manfiest project: https://chromium.googlesource.com/chromiumos/manifest/+/infra/config/tricium-prod.cfg |
||||
►
Sign in to add a comment |
||||
Comment 1 by la...@chromium.org
, Jul 2Status: Assigned (was: Available)