add automatic checks to crosvm repository |
||
Issue descriptionThese checks should be added to the repo upload hooks as well as to the git pre-push hooks, if the developer so chooses. Initially, it should check `build_test.py` and `cargo fmt`. It should also run clippy once that is properly configured.
,
Nov 29
,
Dec 4
The following revision refers to this bug: https://chromium.googlesource.com/chromiumos/repohooks/+/a66f576c8bb4532f781f82ef8776cf03d4358798 commit a66f576c8bb4532f781f82ef8776cf03d4358798 Author: Daniel Verkamp <dverkamp@chromium.org> Date: Tue Dec 04 02:01:36 2018 pre-upload: add cargo_fmt_check presubmit hook This will be used to check formatting of Rust code. BUG=chromium:908640 TEST=repo upload on platform2 and crosvm Rust code Change-Id: I6d5f188e95b770deef17176fc3732362fa953e19 Signed-off-by: Daniel Verkamp <dverkamp@chromium.org> Reviewed-on: https://chromium-review.googlesource.com/1352648 Reviewed-by: Chirantan Ekbote <chirantan@chromium.org> [modify] https://crrev.com/a66f576c8bb4532f781f82ef8776cf03d4358798/pre-upload.py
,
Dec 4
The following revision refers to this bug: https://chromium.googlesource.com/chromiumos/platform/crosvm/+/35bac991e674e8ff228e3da3f05984ff23a535dd commit 35bac991e674e8ff228e3da3f05984ff23a535dd Author: Daniel Verkamp <dverkamp@chromium.org> Date: Tue Dec 04 08:11:31 2018 presubmit: add cargo fmt check This will automatically check rustfmt during 'repo upload'. BUG=chromium:908640 TEST=repo upload CQ-DEPEND=CL:1352648 Change-Id: I97911e00de2f25f0827c37b9715f7f77a504d2fa Signed-off-by: Daniel Verkamp <dverkamp@chromium.org> Reviewed-on: https://chromium-review.googlesource.com/1351693 Reviewed-by: Zach Reizner <zachr@chromium.org> [add] https://crrev.com/35bac991e674e8ff228e3da3f05984ff23a535dd/PRESUBMIT.cfg
,
Dec 4
The following revision refers to this bug: https://chromium.googlesource.com/chromiumos/platform2/+/fa7f71c70cd539b67016da04350cd534db2f248f commit fa7f71c70cd539b67016da04350cd534db2f248f Author: Daniel Verkamp <dverkamp@chromium.org> Date: Tue Dec 04 08:11:34 2018 PRESUBMIT: Add cargo fmt check for 9s, p9, and crostini_client BUG=chromium:908640 TEST=repo upload CQ-DEPEND=CL:1352648 Change-Id: I3035159d9a4dcc4a1e028f3a1eae682489941700 Signed-off-by: Daniel Verkamp <dverkamp@chromium.org> Reviewed-on: https://chromium-review.googlesource.com/1352653 Reviewed-by: Zach Reizner <zachr@chromium.org> [modify] https://crrev.com/fa7f71c70cd539b67016da04350cd534db2f248f/PRESUBMIT.cfg
,
Dec 5
After synced to ToT today, I got presubmit error at cargo_fmt_check when uploading a patch not containing Rust files like http://crrev.com/c/1362633 and http://crrev.com/c/1350377. ----- $ repo upload PRESUBMIT.cfg: [1/1]: c36c68a1ace3496fc4c32bd76f2f151fc7205bbc: Running clang_fo PRESUBMIT.cfg: [1/1]: c36c68a1ace3496fc4c32bd76f2f151fc7205bbc: Running _check_p PRESUBMIT.cfg: [1/1]: c36c68a1ace3496fc4c32bd76f2f151fc7205bbc: Running cargo_fm ERROR: Traceback (most recent call last): File "/ssd/yamaguchi/work/chromeos/.repo/repo/project.py", line 587, in _ExecuteHook context['main'](**kwargs) File "/ssd/yamaguchi/work/chromeos/src/repohooks/pre-upload.py", line 1851, in main if _run_project_hooks(project, proj_dir=worktree): File "/ssd/yamaguchi/work/chromeos/src/repohooks/pre-upload.py", line 1816, in _run_project_hooks hook_error = hook(project, commit) File "/ssd/yamaguchi/work/chromeos/src/repohooks/pre-upload.py", line 1239, in _check_cargo_fmt error_code_ok=True) File "/ssd/yamaguchi/work/chromeos/chromite/lib/cros_build_lib.py", line 652, in RunCommand raise RunCommandError(estr, CommandResult(cmd=cmd), exception=e) RunCommandError: return code: None; command: cargo fmt --all -- --check [Errno 2] No such file or directory Failed to run main() for pre-upload hook; see traceback above.
,
Dec 5
I'm working on a follow-up change that only runs cargo fmt when Rust files are modified: https://chromium-review.googlesource.com/c/chromiumos/repohooks/+/1361800
,
Dec 11
I get the same problem when running presubmit checks in platform2. It seems to only happen when I'm outside the chroot.
ERROR: Traceback (most recent call last):e5215c47a5d738d90db69: Running cargo_fmt_check
File "/usr/local/google/home/chowes/chromiumos/.repo/repo/project.py", line 587, in _ExecuteHook
context['main'](**kwargs)
File "/usr/local/google/home/chowes/chromiumos/src/repohooks/pre-upload.py", line 1851, in main
if _run_project_hooks(project, proj_dir=worktree):
File "/usr/local/google/home/chowes/chromiumos/src/repohooks/pre-upload.py", line 1816, in _run_project_hooks
hook_error = hook(project, commit)
File "/usr/local/google/home/chowes/chromiumos/src/repohooks/pre-upload.py", line 1239, in _check_cargo_fmt
error_code_ok=True)
File "/usr/local/google/home/chowes/chromiumos/chromite/lib/cros_build_lib.py", line 651, in RunCommand
raise RunCommandError(estr, CommandResult(cmd=cmd), exception=e)
RunCommandError: return code: None; command: cargo fmt --all -- --check
[Errno 2] No such file or directory
Failed to run main() for pre-upload hook; see traceback above.
,
Dec 11
Sorry for the delay - we're still discussing the best way for the Rust formatting checks to work. I've uploaded a new change that reverts the platform2 changes in preparation for a different approach where Rust formatting will be opt out rather than opt in: https://chromium-review.googlesource.com/c/chromiumos/platform2/+/1372420 You can also run repo upload with --no-verify to skip the checks until that's merged.
,
Dec 15
The following revision refers to this bug: https://chromium.googlesource.com/chromiumos/platform/crosvm/+/63bc050f5a160fa98e588c4850d82d49d7c3518d commit 63bc050f5a160fa98e588c4850d82d49d7c3518d Author: Daniel Verkamp <dverkamp@chromium.org> Date: Sat Dec 15 06:40:48 2018 Revert "presubmit: add cargo fmt check" The `cargo fmt` preupload check will be replaced with a rustfmt check that does not require projects to opt in. This reverts commit 35bac991e674e8ff228e3da3f05984ff23a535dd. BUG=chromium:908640 TEST=`repo upload` Change-Id: I08f081ef7b889542a6d95fe3f13cdc480759207c Signed-off-by: Daniel Verkamp <dverkamp@chromium.org> Reviewed-on: https://chromium-review.googlesource.com/1376070 Commit-Ready: ChromeOS CL Exonerator Bot <chromiumos-cl-exonerator@appspot.gserviceaccount.com> Reviewed-by: Chirantan Ekbote <chirantan@chromium.org> [delete] https://crrev.com/c5899296c41f9c3991542350a3e2e6118e1972c2/PRESUBMIT.cfg
,
Dec 15
The following revision refers to this bug: https://chromium.googlesource.com/chromiumos/platform2/+/5b59bd98351708a6fb2a0f1eded9d4523c086efc commit 5b59bd98351708a6fb2a0f1eded9d4523c086efc Author: Daniel Verkamp <dverkamp@chromium.org> Date: Sat Dec 15 06:40:37 2018 Revert "PRESUBMIT: Add cargo fmt check for 9s, p9, and crostini_client" Based on the discussion in CL:1361800, let's make Rust formatting opt out rather than opt in. This reverts commit fa7f71c70cd539b67016da04350cd534db2f248f. BUG=chromium:908640 TEST=None Change-Id: Ia7ca5243295ecb71a143ab5319dbc5c099a008b4 Signed-off-by: Daniel Verkamp <dverkamp@chromium.org> Reviewed-on: https://chromium-review.googlesource.com/1372420 Reviewed-by: Brian Norris <briannorris@chromium.org> [modify] https://crrev.com/5b59bd98351708a6fb2a0f1eded9d4523c086efc/PRESUBMIT.cfg
,
Dec 19
The following revision refers to this bug: https://chromium.googlesource.com/chromiumos/platform2/+/549cffb35b527ebb58af8794abac9aff01293ce6 commit 549cffb35b527ebb58af8794abac9aff01293ce6 Author: Daniel Verkamp <dverkamp@chromium.org> Date: Wed Dec 19 09:12:35 2018 vm_tools: crostini_client: format with Rust 1.31.0 Automatically formatted with `cargo fmt --all` from Rust 1.31.0. BUG=chromium:908640 TEST=cargo test Change-Id: Ibe0b9fe42dc6fd3a14e6dbe0b3976b5a69a152d4 Signed-off-by: Daniel Verkamp <dverkamp@chromium.org> Reviewed-on: https://chromium-review.googlesource.com/1379016 Reviewed-by: David Tolnay <dtolnay@chromium.org> Reviewed-by: Zach Reizner <zachr@chromium.org> [modify] https://crrev.com/549cffb35b527ebb58af8794abac9aff01293ce6/vm_tools/crostini_client/backends/chromeos.rs [modify] https://crrev.com/549cffb35b527ebb58af8794abac9aff01293ce6/vm_tools/crostini_client/frontends/vmc.rs [modify] https://crrev.com/549cffb35b527ebb58af8794abac9aff01293ce6/vm_tools/crostini_client/unsafe_misc.rs [modify] https://crrev.com/549cffb35b527ebb58af8794abac9aff01293ce6/vm_tools/crostini_client/build.rs |
||
►
Sign in to add a comment |
||
Comment 1 by dverkamp@chromium.org
, Nov 26