[PATCH 1/3] aiaiai-test-patchset: check for coccinelle scripts first

[Keller, Jacob E]

> -----Original Message-----
> From: Artem Bityutskiy [mailto:dedekind1 at gmail.com]
> Sent: Wednesday, May 28, 2014 12:15 AM
> To: Keller, Jacob E
> Cc: aiaiai at lists.infradead.org
> Subject: Re: [PATCH 1/3] aiaiai-test-patchset: check for coccinelle scripts
> first
> 
> On Tue, 2014-05-27 at 10:56 -0700, Jacob Keller wrote:
> > Rather than always archiving the coccinelle scripts, check to ensure
> > they exist first. This prevents issues when attempting to build a
> > non-kernel tree, (ie: ethtool). Will display a warning if coccinelle was
> > requested but no scripts were found.
> >
> > Signed-off-by: Jacob Keller <jacob.e.keller at intel.com>
> > ---
> >  aiaiai-test-patchset | 13 +++++++++----
> >  1 file changed, 9 insertions(+), 4 deletions(-)
> >
> > diff --git a/aiaiai-test-patchset b/aiaiai-test-patchset
> > index 45527f90b4ae..90c790098e38 100755
> > --- a/aiaiai-test-patchset
> > +++ b/aiaiai-test-patchset
> > @@ -428,10 +428,15 @@ git show
> "$commit_id1:scripts/checkpatch.pl" > "$checkpatch_pl"
> >  chmod $verbose u+x "$checkpatch_pl" >&2
> >
> >  if [ -n "$coccinelle" ]; then
> > -	mkdir -p $verbose "$tmpdir/coccinelle" >&2
> > -	git archive "$commit_id1" scripts/coccinelle | \
> > -		tar $verbose --strip-components=2 -C
> "$tmpdir/coccinelle" -x >&2
> > -	coccinelle="--coccinelle=$tmpdir/coccinelle"
> > +	if git cat-file -e "$commit_id1:scripts/coccinelle"; then
> > +		mkdir -p $verbose "$tmpdir/coccinelle" >&2
> > +		git archive "$commit_id1" scripts/coccinelle | \
> > +			tar $verbose --strip-components=2 -C
> "$tmpdir/coccinelle" -x >&2
> > +		coccinelle="--coccinelle=$tmpdir/coccinelle"
> > +	else
> > +		verbose "Can't find coccinelle scripts.. disabling
> coccinelle tests."
> > +		coccinelle=
> > +	fi
> 
> I think I always followed this general principle:
> 
> If user requests something, and we cannot satisfy this, we fail with an
> error message.
> 
> I'd be consistent with this approach and avoided mixing 2 completely
> different principles. Indeed, in this patch, the principle is:
> 
> If users requests something, and we cannot satisfy this, we just
> silently do not do this.
> 
> I say "silently" because this patch would only print a message when -v
> is used, and this message would go to stderr and would be buried in tons
> on other messages.
> 
> I suggest to fail loudly instead.
> 
> What do you think?
> 

I suppose that is alright. Mostly was concerned with blocking other testing options due to one thing missing, but it should be ok to fail loudly to indicate the issue, that makes some amount of sense.

I will respin this to do that.

Thanks,
Jake

> --
> Best Regards,
> Artem Bityutskiy