4:39 p.m.
The copyright date syntax check is a source of frustration for me:
- It fails on old maint branches.
- It pokes into non git managed directories in $(srcdir). I like to keep
directories like f18-maint.git, f19-maint.git etc. that are checkouts of the
respective maint branches so I can parallelize when doing maint builds, and I
like to keep them in my libvirt.git dir for organization, but running
syntax-check in libvirt.git fails on the copyright check in
$(srcdir)/f18-maint.git/.gnulib/...
Is there any way to make it non-fatal? If it still spams with all the
'failures' I'm sure that's enough to encourage fixing the issues without
blocking valid usage.
Thanks,
Cole
4:48 p.m.
On 10/08/2013 03:39 PM, Cole Robinson wrote:
The copyright date syntax check is a source of frustration for me:
- It fails on old maint branches.
In general, 'make syntax-check' is NOT guaranteed to work on maint
branches, nor is it worth trying to make it work there. If anything,
the best thing to do would be figuring out how to patch 'make
syntax-check' to be a complete no-op on maint branches and only operate
on a (descendant of) the master branch, precisely because out-of-order
backports to maint branches makes it practically impossible to guarantee
that we can consistently meet whatever style future development on the
master branch decides is now in vogue.
- It pokes into non git managed directories in $(srcdir).
That's probably a bug, and we should be able to fix it upstream in
gnulib. Syntax checkers should be able to be limited to just git files.
I guess the idea was that it checks non-managed files to ensure a
tarball doesn't have generated files with a broken copyright date, but
there's probably a better way to do that.
--
Eric Blake eblake redhat com +1-919-301-3266
Libvirt virtualization library http://libvirt.org
4:57 p.m.
On 10/08/2013 05:48 PM, Eric Blake wrote:
On 10/08/2013 03:39 PM, Cole Robinson wrote:
> The copyright date syntax check is a source of frustration for me:
>
> - It fails on old maint branches.
In general, 'make syntax-check' is NOT guaranteed to work on maint
branches, nor is it worth trying to make it work there. If anything,
the best thing to do would be figuring out how to patch 'make
syntax-check' to be a complete no-op on maint branches and only operate
on a (descendant of) the master branch, precisely because out-of-order
backports to maint branches makes it practically impossible to guarantee
that we can consistently meet whatever style future development on the
master branch decides is now in vogue.
Agreed on that for the most part. It can still be beneficial to run,
particularly after a hairy backport though, and it's frustrating to be blocked
on something like a copyright date. But you're right that we can never
guarantee it will continue to work.
>
> - It pokes into non git managed directories in $(srcdir).
That's probably a bug, and we should be able to fix it upstream in
gnulib. Syntax checkers should be able to be limited to just git files.
I guess the idea was that it checks non-managed files to ensure a
tarball doesn't have generated files with a broken copyright date, but
there's probably a better way to do that.
That's my main complaint, so if it's fixed, I'll be satisfied. Thanks.
But really any test that is basically going to hinder normal work by demanding
a gnulib update on January 1st just seems ill advised IMO.
- Cole
5:06 p.m.
On 10/08/2013 03:57 PM, Cole Robinson wrote:
On 10/08/2013 05:48 PM, Eric Blake wrote:
> On 10/08/2013 03:39 PM, Cole Robinson wrote:
>> The copyright date syntax check is a source of frustration for me:
>>
>> - It fails on old maint branches.
>
> In general, 'make syntax-check' is NOT guaranteed to work on maint
> branches, nor is it worth trying to make it work there. If anything,
> the best thing to do would be figuring out how to patch 'make
> syntax-check' to be a complete no-op on maint branches and only operate
> on a (descendant of) the master branch, precisely because out-of-order
> backports to maint branches makes it practically impossible to guarantee
> that we can consistently meet whatever style future development on the
> master branch decides is now in vogue.
>
Agreed on that for the most part. It can still be beneficial to run,
particularly after a hairy backport though, and it's frustrating to be blocked
on something like a copyright date. But you're right that we can never
guarantee it will continue to work.
You can always run 'make -k syntax-check' or even 'make sc_name_of_test'
to keep going or limit the checking to the subset of tests you care about.
>>
>> - It pokes into non git managed directories in $(srcdir).
>
> That's probably a bug, and we should be able to fix it upstream in
> gnulib. Syntax checkers should be able to be limited to just git files.
> I guess the idea was that it checks non-managed files to ensure a
> tarball doesn't have generated files with a broken copyright date, but
> there's probably a better way to do that.
>
That's my main complaint, so if it's fixed, I'll be satisfied. Thanks.
I'll try and look at it, then.
But really any test that is basically going to hinder normal work by demanding
a gnulib update on January 1st just seems ill advised IMO.
Copyright law is annoying that way. Since libvirt.git can be considered
a form of public distribution, and since the GPL only works if you
assert copyright correctly, this is one case where legal paranoia was
deemed worth the pain in upstream gnulib. On the other hand, since we
do not assign copyright to a common holder, we don't follow quite the
same rules as the FSF demands of GNU projects, and we could probably get
away with disabling the sc_copyright_check altogether (we already
disable several other upstream syntax checkers that don't make sense in
our particular project; such as checks for correct texinfo source
idioms, where we don't use texinfo for our documentation).
--
Eric Blake eblake redhat com +1-919-301-3266
Libvirt virtualization library http://libvirt.org
5:07 p.m.
On 10/08/2013 06:06 PM, Eric Blake wrote:
On 10/08/2013 03:57 PM, Cole Robinson wrote:
> On 10/08/2013 05:48 PM, Eric Blake wrote:
>> On 10/08/2013 03:39 PM, Cole Robinson wrote:
>>> The copyright date syntax check is a source of frustration for me:
>>>
>>> - It fails on old maint branches.
>>
>> In general, 'make syntax-check' is NOT guaranteed to work on maint
>> branches, nor is it worth trying to make it work there. If anything,
>> the best thing to do would be figuring out how to patch 'make
>> syntax-check' to be a complete no-op on maint branches and only operate
>> on a (descendant of) the master branch, precisely because out-of-order
>> backports to maint branches makes it practically impossible to guarantee
>> that we can consistently meet whatever style future development on the
>> master branch decides is now in vogue.
>>
>
> Agreed on that for the most part. It can still be beneficial to run,
> particularly after a hairy backport though, and it's frustrating to be blocked
> on something like a copyright date. But you're right that we can never
> guarantee it will continue to work.
You can always run 'make -k syntax-check' or even 'make sc_name_of_test'
to keep going or limit the checking to the subset of tests you care about.
Doh, I always forget about -k :/ Yes that's a reasonable workaround to all my
complaints.
Thanks,
Cole
10 p.m.
On 10/08/2013 03:48 PM, Eric Blake wrote:
On 10/08/2013 03:39 PM, Cole Robinson wrote:
> The copyright date syntax check is a source of frustration for me:
>
> - It fails on old maint branches.
In general, 'make syntax-check' is NOT guaranteed to work on maint
branches, nor is it worth trying to make it work there. If anything,
the best thing to do would be figuring out how to patch 'make
syntax-check' to be a complete no-op on maint branches and only operate
on a (descendant of) the master branch, precisely because out-of-order
backports to maint branches makes it practically impossible to guarantee
that we can consistently meet whatever style future development on the
master branch decides is now in vogue.
I've attempted this now.
>
> - It pokes into non git managed directories in $(srcdir).
That's probably a bug, and we should be able to fix it upstream in
gnulib. Syntax checkers should be able to be limited to just git files.
I guess the idea was that it checks non-managed files to ensure a
tarball doesn't have generated files with a broken copyright date, but
there's probably a better way to do that.
Still need to investigate here...
--
Eric Blake eblake redhat com +1-919-301-3266
Libvirt virtualization library http://libvirt.org
9:45 p.m.
New subject: [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 <eblake(a)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
4:32 a.m.
New subject: [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 <eblake(a)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 :|
7:53 a.m.
New subject: [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 <eblake(a)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
8:10 a.m.
New subject: [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 <eblake(a)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(a)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 :|
4:34 a.m.
New subject: [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.
4062
days inactive
4064
days old
10 comments
4 participants
participants (4)
-
Cole Robinson -
Daniel P. Berrange -
Eric Blake -
Laine Stump