llvm dev - Nov 2016 - Highlighting trailing whitespaces on Phab?

Why isn’t it in the LLVM repo?
> On Nov 16, 2016, at 7:44 AM, Johannes Doerfert <doerfert at
cs.uni-saarland.de> wrote:
> 
> We have a clang format based arcanist linter (and some others) in the
> Polly repository. When arcanist is used to create a review, the linter
> result is shown online.  We also have an arcanist add-on to run the lit
> tests and show their result in the review as well.
> 
> The problem is basically that not many ppl use arcanist...
> 
> On 11/16, Eric Liu wrote:
>> So, I forwarded the request for highlighting trailing whitespaces to
>> phabricator upstream (https://secure.phabricator.com/T11879), and
upstream
>> folks suggest we enable the Lint feature in Arcanist (
>>
https://secure.phabricator.com/book/phabricator/article/arcanist_lint/). This
>> will enforce the check when `arc diff` is run (reviewers wouldn't
see the
>> warnings though).
>> 
>> There are two linters we might be interested in enabling:
>> - cpplint (code style checker based on cpplint.py
>> <https://github.com/google/styleguide>)
>> - cppcheck (C++ linter based on cppcheck
<http://cppcheck.sourceforge.net/>)
>> 
>> Note that cpplint assumes google code style, but I think it can
potentially
>> be replaced it with clang-format with configurable code styles.
>> 
>> On Wed, Nov 16, 2016 at 12:28 PM Eric Liu <ioeric at google.com>
wrote:
>> 
>> I'm not sure how easy it is to get clang-format into Phabricator
since it
>> is mostly developed by (phab) upstream. I'll file feature request
regarding
>> trailing whitespaces and clang-format to upstream to see what upstream
>> forks think.
>> 
>> But a bot checking revision sounds great. It might also be useful to
have
>> the bot run clang-tidy and post findings/FixHints as comments on the
patch.
>> 
>> On Wed, Nov 16, 2016 at 12:05 PM Johannes Doerfert via llvm-dev <
>> llvm-dev at lists.llvm.org> wrote:
>> 
>> On 11/14, Mehdi Amini via llvm-dev wrote:
>>> Ideally I’d even really like to have a both checking for revision
on
>>> phab, clang-formatting them, and post a comment when there is a
>>> mismatch :)
>> I'd like that!
>> _______________________________________________
>> LLVM Developers mailing list
>> llvm-dev at lists.llvm.org
>> http://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-dev
> 
> -- 
> 
> Johannes Doerfert
> Researcher / PhD Student
> 
> Compiler Design Lab (Prof. Hack)
> Saarland Informatics Campus, Germany
> Building E1.3, Room 4.31
> 
> Tel. +49 (0)681 302-57521 : doerfert at cs.uni-saarland.de
> Fax. +49 (0)681 302-3065  : http://www.cdl.uni-saarland.de/people/doerfert

I assume those arc linters run on the client side? And people need to have the
various tools installed before all of that works?

Just to throw out another idea: Running a linter on the server side and let it
comment on phab reviews would be the perfect project to get some experience with
pre-commit testing/hooks for llvm :)

- Matthias
> On Nov 16, 2016, at 9:14 AM, Mehdi Amini via llvm-dev <llvm-dev at
lists.llvm.org> wrote:
> 
> Why isn’t it in the LLVM repo?
> 
>> On Nov 16, 2016, at 7:44 AM, Johannes Doerfert <doerfert at
cs.uni-saarland.de> wrote:
>> 
>> We have a clang format based arcanist linter (and some others) in the
>> Polly repository. When arcanist is used to create a review, the linter
>> result is shown online.  We also have an arcanist add-on to run the lit
>> tests and show their result in the review as well.
>> 
>> The problem is basically that not many ppl use arcanist...
>> 
>> On 11/16, Eric Liu wrote:
>>> So, I forwarded the request for highlighting trailing whitespaces
to
>>> phabricator upstream (https://secure.phabricator.com/T11879), and
upstream
>>> folks suggest we enable the Lint feature in Arcanist (
>>>
https://secure.phabricator.com/book/phabricator/article/arcanist_lint/). This
>>> will enforce the check when `arc diff` is run (reviewers
wouldn't see the
>>> warnings though).
>>> 
>>> There are two linters we might be interested in enabling:
>>> - cpplint (code style checker based on cpplint.py
>>> <https://github.com/google/styleguide>)
>>> - cppcheck (C++ linter based on cppcheck
<http://cppcheck.sourceforge.net/>)
>>> 
>>> Note that cpplint assumes google code style, but I think it can
potentially
>>> be replaced it with clang-format with configurable code styles.
>>> 
>>> On Wed, Nov 16, 2016 at 12:28 PM Eric Liu <ioeric at
google.com> wrote:
>>> 
>>> I'm not sure how easy it is to get clang-format into
Phabricator since it
>>> is mostly developed by (phab) upstream. I'll file feature
request regarding
>>> trailing whitespaces and clang-format to upstream to see what
upstream
>>> forks think.
>>> 
>>> But a bot checking revision sounds great. It might also be useful
to have
>>> the bot run clang-tidy and post findings/FixHints as comments on
the patch.
>>> 
>>> On Wed, Nov 16, 2016 at 12:05 PM Johannes Doerfert via llvm-dev
<
>>> llvm-dev at lists.llvm.org> wrote:
>>> 
>>> On 11/14, Mehdi Amini via llvm-dev wrote:
>>>> Ideally I’d even really like to have a both checking for
revision on
>>>> phab, clang-formatting them, and post a comment when there is a
>>>> mismatch :)
>>> I'd like that!
>>> _______________________________________________
>>> LLVM Developers mailing list
>>> llvm-dev at lists.llvm.org
>>> http://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-dev
>> 
>> -- 
>> 
>> Johannes Doerfert
>> Researcher / PhD Student
>> 
>> Compiler Design Lab (Prof. Hack)
>> Saarland Informatics Campus, Germany
>> Building E1.3, Room 4.31
>> 
>> Tel. +49 (0)681 302-57521 : doerfert at cs.uni-saarland.de
>> Fax. +49 (0)681 302-3065  :
http://www.cdl.uni-saarland.de/people/doerfert
> 
> _______________________________________________
> LLVM Developers mailing list
> llvm-dev at lists.llvm.org
> http://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-dev

Hey Mehdi, Hey Matthias,

On 11/16, Matthias Braun wrote:
> I assume those arc linters run on the client side? And people need to have
the various tools installed before all of that works?
Yes they run client side but there are no "tools" needed except
arcanist (we talk about arcanist linters after all).

Here is the .arclint file in the Polly repo:
{
  "linters": {
    "format": {
      "include": "(include/polly/.+\\.h$|lib/.+\\.cpp$)",
      "exclude": "(lib/JSON/.*)",
      "type": "script-and-regex",
      "script-and-regex.script": "sh -c
'./utils/check_format.sh \"$0\" 2> /dev/null ||
true'",
      "script-and-regex.regex":
"/^(OK:(?P<ignore>.+)|Error:) (?P<message>.+)$/m"
    },
    "chmod": {
      "type": "chmod"
    },
    "filename": {
      "exclude": "(www/experiments/.+|.*\\.jscop.*)",
      "type": "filename"
    },
    "merge-conflict": {
      "type": "merge-conflict"
    },
    "spelling": {
      "exclude": "(configure|autoconf/.*)",
      "type": "spelling"
    }
  }
}

We, or better me at some point, used various linters provided by arcanist and
one custom "script-and-regex" linter based on the check_format.sh
script that
wraps clang-format. While it is apparently not present in the repository
anymore [-.-], we used it at some point in the Polly tests
("check-polly") and
the buildbots to verify the source formatting.
> Just to throw out another idea: Running a linter on the server side and let
it comment on phab reviews would be the perfect project to get some experience
with pre-commit testing/hooks for llvm :)
To be honest I would prefer a server side solution or a bot as the clients
will not converge on one way of submitting patches (for review) anyway.

--
  Johannes
> > On Nov 16, 2016, at 9:14 AM, Mehdi Amini via llvm-dev <llvm-dev at
lists.llvm.org> wrote:
> > 
> > Why isn’t it in the LLVM repo?
We could port the linter as well as the test execution engine to llvm if
you like. For the latter I started a discussion once but it did just
fade away...

-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 195 bytes
Desc: Digital signature
URL:
<http://lists.llvm.org/pipermail/llvm-dev/attachments/20161116/f3deea3c/attachment.sig>