[libvirt] [PATCH] maint: make syntax-check default to a no-op on maint branches

[Daniel P. Berrange]

On Wed, Oct 09, 2013 at 06:53:00AM -0600, Eric Blake wrote:
> On 10/09/2013 03:32 AM, Daniel P. Berrange wrote:
> > On Tue, Oct 08, 2013 at 08:45:23PM -0600, Eric Blake wrote:
> >> Maintenance branches cherry-pick some, but not all patches, and
> >> sometimes in different order, which means that 'make syntax-check'
> >> is likely to fail for hard-to-predict reasons.  Yet having a
> >> common workflow makes it easier to switch between branches.  This
> >> patch sets up a filter so that 'make syntax-check' is a no-op
> >> other than to print a warning if it detects that the user is
> >> running in a git checkout that branches from some place earlier
> >> than origin; 'make -k syntax-check force-syntax-check=1' can be
> >> used to override the heuristics.
> >>
> >> Tested on master (no change in behavior) and v1.1.2-maint (where
> >> the syntax check was indeed suppressed).  Based on a report by
> >> Cole Robinson.
> >>
> >> * cfg.mk (syntax-check): Add some conditional filtering.
> >>
> >> Signed-off-by: Eric Blake <eblake at redhat.com>
> >> ---
> >>
> >> If approved, I'll backport this to all the v*-maint branches.
> > 
> > I'm not sure I really like this. Quite a few of the checks we do
> > are quite critical to code quality and IMHO should continue to
> > be validated on maint branches to ensure that we don't screw up
> > when resolving conflicts. 
> > 
> > I would be ok with skipping checks which are merely style related,
> > but not skipping checks which are code correctness issues. Obviously
> > skipping the copyright date check is reasonable.
> 
> But the point is right now, you CAN'T successfully run 'make
> syntax-check' on a maint branch; you already HAVE to change workflow to
> test branches differently than master.  This patch doesn't change the
> fact that a different workflow is needed to test a maint branch, but at
> least makes it so that using the same workflow as on master will alert
> you to what steps to actually take, rather than the current situation of
> flat out failing without telling you how to work around the failure).

You cannot successfully run 'make syntax-check' on *some*
maint branches. It certainly works fine on many of the
maint branches & we shouldn't disable it.

For older maint branches the copyright date checks fails,
and that is a valid failure, so we should skip copyright
checks. Other failures I see on maint branches are caused
by bad backports of patches. For example 

commit f63b9694dc031165e55e99e463067be386474611
Author: Richard W.M. Jones <rjones at redhat.com>
Date:   Wed Jan 23 20:09:04 2013 +0000

    selinux: Only create the selabel_handle once.
    
    According to Eric Paris this is slightly more efficient because it
    only loads the regular expressions in libselinux once.
    (cherry picked from commit 6159710ca1eecefa7c81335612c8141c88fc35a9)
    
    Conflicts:
        src/security/security_selinux.c


on v0.10.2-maint is a bad backport which added broken
#ifdef indentation. We should absolutely be running
syntax-check as normal to identify that on -maint branches
just as on master.

IMHO all we want todo is add:

    local-checks-to-skip += sc_copyright_check

When on -maint branches.

Daniel
-- 
|: http://berrange.com      -o-    http://www.flickr.com/photos/dberrange/ :|
|: http://libvirt.org              -o-             http://virt-manager.org :|
|: http://autobuild.org       -o-         http://search.cpan.org/~danberr/ :|
|: http://entangle-photo.org       -o-       http://live.gnome.org/gtk-vnc :|