Make checkperms.py --bare also return failure reason

Make checkperms.py --bare also return failure reason

In https://codereview.chromium.org/318863006 checkperms.py was made to only
report a list of files failing checkperms. This seems to have been added only
for presubmit according to the comment
"--file implies --bare (for PRESUBMIT.py)."

This change makes the --bare parameter only suppress the TESTING header instead
of the failure reasons as well in an effort to make presubmit errors more
understandable to the Chromium developer community.

R=phajdan.jr@chromium.org, thestig@chromium.org
BUG=423697

[Daniel Imms, 2015-10-02 20:39:46]

daniimms@amazon.com changed reviewers:
+ thestig@chromium.org
- maruel@chromium.org

[Daniel Imms, 2015-10-02 20:39:46]

PTAL, let me know if the using --bare to return more than just the file list is
alright or is an alternate solution would be better. Thanks.

phajdan.jr: fyi
thestig: review

[Lei Zhang, 2015-10-02 20:54:56]

I'll actually defer to Pawel, but will check back on Monday in case Pawel kicks
it back to me.

[Paweł Hajdan Jr., 2015-10-05 13:46:15]

Why not change the presubmit check instead (_CheckFilePermissions) not to use
--file, and instead use the exit code to determine success/failure, and the
generic presubmit logic to print output of failed commands?

[Daniel Imms, 2015-10-05 16:57:15]

On 2015/10/05 at 13:46:15, phajdan.jr wrote:
> Why not change the presubmit check instead (_CheckFilePermissions) not to use
--file, and instead use the exit code to determine success/failure, and the
generic presubmit logic to print output of failed commands?

Not using --file runs it over the whole repo, not just the changed files.

[Paweł Hajdan Jr., 2015-10-06 15:09:51]

On 2015/10/05 at 16:57:15, daniimms wrote:
> On 2015/10/05 at 13:46:15, phajdan.jr wrote:
> > Why not change the presubmit check instead (_CheckFilePermissions) not to
use --file, and instead use the exit code to determine success/failure, and the
generic presubmit logic to print output of failed commands?
> 
> Not using --file runs it over the whole repo, not just the changed files.

Ah, right - sorry, I was just confused.

How about changing the script so that --file no longer implies --bare, and also
adjusting the PRESUBMIT check to use --json to return list of files to the
presubmit script? Then the presubmit check should also use the exit code as the
primary way of seeing whether it worked, and use the returned json to help
construct a meaningful error message for the user.

[Daniel Imms, 2015-10-06 17:50:27]

On 2015/10/06 at 15:09:51, phajdan.jr wrote:
> On 2015/10/05 at 16:57:15, daniimms wrote:
> > On 2015/10/05 at 13:46:15, phajdan.jr wrote:
> > > Why not change the presubmit check instead (_CheckFilePermissions) not to
use --file, and instead use the exit code to determine success/failure, and the
generic presubmit logic to print output of failed commands?
> > 
> > Not using --file runs it over the whole repo, not just the changed files.
> 
> Ah, right - sorry, I was just confused.
> 
> How about changing the script so that --file no longer implies --bare, and
also adjusting the PRESUBMIT check to use --json to return list of files to the
presubmit script? Then the presubmit check should also use the exit code as the
primary way of seeing whether it worked, and use the returned json to help
construct a meaningful error message for the user.

--json outputs a file though, seems messy for such a simple presubmit script to
create a temp file? Would a --bare-with-error arg work if --bare should only
return the list of files?

[Paweł Hajdan Jr., 2015-10-07 12:55:52]

I uploaded https://codereview.chromium.org/1395493002