Re: [libvirt] [PATCH] maint: make syntax-check default to a no-op on maint branches
On 10/09/2013 04:10 PM, Daniel P. Berrange wrote: 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. I agree. make syntax-check is just as useful on a -maint branch as on master; I like to run it on those branches not as a matter of habit, but in order to catch potential problems. -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH] maint: make syntax-check default to a no-op on maint branches
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 ebl...@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. 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 :| -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH] maint: make syntax-check default to a no-op on maint branches
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 ebl...@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). -- Eric Blake eblake redhat com+1-919-301-3266 Libvirt virtualization library http://libvirt.org signature.asc Description: OpenPGP digital signature -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH] maint: make syntax-check default to a no-op on maint branches
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 ebl...@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 rjo...@redhat.com Date: Wed Jan 23 20:09:04 2013 + 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 :| -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCH] maint: make syntax-check default to a no-op on maint branches
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 ebl...@redhat.com --- If approved, I'll backport this to all the v*-maint branches. cfg.mk | 18 ++ 1 file changed, 18 insertions(+) diff --git a/cfg.mk b/cfg.mk index dad8a90..47709b5 100644 --- a/cfg.mk +++ b/cfg.mk @@ -88,6 +88,24 @@ else distdir: sc_vulnerable_makefile_CVE-2012-3386.z endif +# If it looks like we are on a maintenance branch (that is, if our +# merge point with origin is somewhere other than origin), then +# skip syntax checks by default. When backporting only a subset of +# patches, it's difficult, if not impossible, to have syntax-check +# pass by default. Borrows some ideas from public-submodule-commit. +ifeq ($(filter syntax-check, $(MAKECMDGOALS))$(MAKELEVEL),syntax-check0) + ifneq ($(shell if test -d $(srcdir)/.git; then \ + cd $(srcdir) git rev-parse origin; fi), \ + $(shell if test -d $(srcdir)/.git; then \ + cd $(srcdir) git merge-base origin HEAD; fi)) +ifeq ($(force-syntax-check),) + $(info warning: it looks like you are on a maint branch; to force, use:) + $(info $(NULL) make -k syntax-check force-syntax-check=1) + local-checks-to-skip += $(syntax-check-rules) +endif + endif +endif + # Files that should never cause syntax check failures. VC_LIST_ALWAYS_EXCLUDE_REGEX = \ (^(HACKING|docs/(news\.html\.in|.*\.patch))|\.po)$$ -- 1.8.3.1 -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
