[LINTER] Run some linter only once
AbandonedPublic
Actions

Authored by Fabien on Jun 12 2019, 10:36.

Details

Reviewers

deadalnix

Group Reviewers

Restricted Project

Summary

The check-doc linter needs to be run when some kind of files are
modified (*.h or *.cpp), but only needs to run once if one of these
files is modified because it parses the whole tree at each call.

This diffs adds a new .runonce.arclint configuration file which
contains linters to be run only once per arc lint call. The
check-doc linter is moved from the .arclint file to this new
configuration.

This speeds up arc lint on large diffs.
arc lint --everything now takes 0'34" on my machine (was 2'22").

Depends on D3299

Test Plan

Add some comments into at least 2 cpp files under src/ and commit
them.

arc lint --trace

Check in the output that the check-doc linter is run only once.

arc diff --preview --trace

Check in the output that the check-doc linter is run only once.

Diff Detail

Repository

rABC Bitcoin ABC

Branch

arcanist_run_once

Lint

No Lint Coverage

Unit

No Test Coverage

Build Status

Buildable 6282
Build 10611: Bitcoin ABC Buildbot (legacy)
Build 10610: arc lint + arc unit

Event Timeline

Fabien created this revision.Jun 12 2019, 10:36

Herald added a reviewer: Restricted Project. · View Herald TranscriptJun 12 2019, 10:37

Herald added a reviewer: deadalnix. · View Herald Transcript

Build Bitcoin-ABC / Diffs / Bitcoin-ABC Diff Testing started.

Build Bitcoin-ABC / Diffs / Bitcoin-ABC Diff Testing passed.

Harbormaster completed remote builds in B6282: Diff 9347.Jun 12 2019, 10:45

Why can't the linter un once to begin with, instead o having a linter that run multiple times and then build all kind of scafolding to have it run only once ?

This revision now requires changes to proceed.Jun 12 2019, 12:55

Arcanist (and more specifically the ArcanistLintWorkFlow) calls each linter for every path that match the filter.
There is no feature to indicate to run it once, or you have to create another workflow from scratch rather than using a linter.
I evaluated this as well, but that would make a much more complicated solution with no real advantage.

You can call the linter n times, but only do the work once. You know a configuration is not what you are after because it makes no sense to run that linter multiple time. This is a property of the linter, not of the environment in which it runs so we clearly are facing a new type of linter, not a new workflow to be configured

Just create a ArcanistGlobalLinter or something and have CheckDocLinter inherit from it. The magic happens within ArcanistGlobalLinter . You may want to use that guy as an example: https://github.com/phacility/arcanist/blob/master/src/lint/linter/ArcanistFutureLinter.php

This revision now requires changes to proceed.Jun 12 2019, 17:01

Fabien mentioned this in D3335: [LINTER] Make the check-doc linter run only once.Jun 14 2019, 15:16

Superseeded bby D3705