Page MenuHomePhabricator

[LINTER] Run some linter only once
AbandonedPublic

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 Linters Available
Unit
No Unit Test Coverage
Build Status
Buildable 6282
Build 10611: Bitcoin ABC Teamcity Staging
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
deadalnix requested changes to this revision.Jun 12 2019, 12:55

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
Fabien requested review of this revision.Jun 12 2019, 13:08

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.

deadalnix requested changes to this revision.Jun 12 2019, 17:01

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 abandoned this revision.Jul 24 2019, 06:40

Superseeded bby D3705