3:28 a.m.
Unfortunately, since gnulib's commit of 2c5d558745 there's an
unused parameter to stat_time_normalize() function which gnulib
developers don't want to fix [1]. Therefore, we have to work
around it by temporarily suspending -Wunused-parameter.
1: http://lists.gnu.org/archive/html/bug-gnulib/2018-01/msg00000.html
Signed-off-by: Michal Privoznik <mprivozn(a)redhat.com>
---
While we have 'gnulib update free' push rule, this one is not trivial at
all and thus I have not pushed it. It's ugly and I don't like it. So any
ideas are welcome.
.gnulib | 2 +-
bootstrap | 4 ++--
src/storage/storage_util.c | 3 +++
3 files changed, 6 insertions(+), 3 deletions(-)
diff --git a/.gnulib b/.gnulib
index 5e9abf871..c2cb55b34 160000
--- a/.gnulib
+++ b/.gnulib
@@ -1 +1 @@
-Subproject commit 5e9abf87163ad4aeaefef0b02961f8674b0a4879
+Subproject commit c2cb55b34e76546479f195c14202dfcc870c4914
diff --git a/bootstrap b/bootstrap
index 85b85c530..25920e991 100755
--- a/bootstrap
+++ b/bootstrap
@@ -4,7 +4,7 @@ scriptversion=2017-09-19.08; # UTC
# Bootstrap this package from checked-out sources.
-# Copyright (C) 2003-2017 Free Software Foundation, Inc.
+# Copyright (C) 2003-2018 Free Software Foundation, Inc.
# This program is free software: you can redistribute it and/or modify
# it under the terms of the GNU General Public License as published by
@@ -792,7 +792,7 @@ symlink_to_dir()
# aren't confused into doing unnecessary builds. Conversely, if the
# existing symlink's timestamp is older than the source, make it afresh,
# so that broken tools aren't confused into skipping needed builds. See
- # <https://lists.gnu.org/archive/html/bug-gnulib/2011-05/msg00326.html>;.
+ # <https://lists.gnu.org/r/bug-gnulib/2011-05/msg00326.html>;.
test -h "$dst" &&
src_ls=$(ls -diL "$src" 2>/dev/null) && set $src_ls &&
src_i=$1 &&
dst_ls=$(ls -diL "$dst" 2>/dev/null) && set $dst_ls &&
dst_i=$1 &&
diff --git a/src/storage/storage_util.c b/src/storage/storage_util.c
index 899a55758..da6381f52 100644
--- a/src/storage/storage_util.c
+++ b/src/storage/storage_util.c
@@ -64,7 +64,10 @@
#include "virfile.h"
#include "virjson.h"
#include "virqemu.h"
+#pragma GCC diagnostic push
+#pragma GCC diagnostic ignored "-Wunused-parameter"
#include "stat-time.h"
+#pragma GCC diagnostic pop
#include "virstring.h"
#include "virxml.h"
#include "virfdstream.h"
--
2.13.6
7:09 a.m.
On 01/02/2018 04:28 AM, Michal Privoznik wrote:
Unfortunately, since gnulib's commit of 2c5d558745 there's
an
unused parameter to stat_time_normalize() function which gnulib
developers don't want to fix [1]. Therefore, we have to work
around it by temporarily suspending -Wunused-parameter.
1: http://lists.gnu.org/archive/html/bug-gnulib/2018-01/msg00000.html
Signed-off-by: Michal Privoznik <mprivozn(a)redhat.com>
---
While we have 'gnulib update free' push rule, this one is not trivial at
all and thus I have not pushed it. It's ugly and I don't like it. So any
ideas are welcome.
.gnulib | 2 +-
bootstrap | 4 ++--
src/storage/storage_util.c | 3 +++
3 files changed, 6 insertions(+), 3 deletions(-)
No bright ideas on this other than perhaps only including changes just
prior to the particular one that breaks things or somehow revert just
that one in our local copy.
Other thoughts require additional local changes just to "work around"
the broken definition. Such as grabbing the "old" stat-time.h, rename
it, save it in libvirt sources, then use that new name instead of the
new .gnulib file. That's probably worse than what you've done though.
At this point, I think your solution to minimize the impact to one
include file is perhaps the easiest/best solution albeit not perfect.
It's interesting that there is no desire to fix this problem in .gnulib
especially since there are already 2 patches proposed and in reality the
change fundamentally breaks on every platform other than __sun when
STAT_TIMESPEC is defined just because it's possible (or more desirable
in the reviewers feedback) to compile with ignore unused-parameter.
Wonder what would happen if someone posted a patch to .gnulib to revert
the change for the reason that it breaks other platforms that don't
desire to configure in that manner. Perhaps Eric Blake would have some
thoughts or additional muscle with the .gnulib maintainers.
John
diff --git a/.gnulib b/.gnulib
index 5e9abf871..c2cb55b34 160000
--- a/.gnulib
+++ b/.gnulib
@@ -1 +1 @@
-Subproject commit 5e9abf87163ad4aeaefef0b02961f8674b0a4879
+Subproject commit c2cb55b34e76546479f195c14202dfcc870c4914
diff --git a/bootstrap b/bootstrap
index 85b85c530..25920e991 100755
--- a/bootstrap
+++ b/bootstrap
@@ -4,7 +4,7 @@ scriptversion=2017-09-19.08; # UTC
# Bootstrap this package from checked-out sources.
-# Copyright (C) 2003-2017 Free Software Foundation, Inc.
+# Copyright (C) 2003-2018 Free Software Foundation, Inc.
# This program is free software: you can redistribute it and/or modify
# it under the terms of the GNU General Public License as published by
@@ -792,7 +792,7 @@ symlink_to_dir()
# aren't confused into doing unnecessary builds. Conversely, if the
# existing symlink's timestamp is older than the source, make it afresh,
# so that broken tools aren't confused into skipping needed builds. See
- # <https://lists.gnu.org/archive/html/bug-gnulib/2011-05/msg00326.html>;.
+ # <https://lists.gnu.org/r/bug-gnulib/2011-05/msg00326.html>;.
test -h "$dst" &&
src_ls=$(ls -diL "$src" 2>/dev/null) && set $src_ls
&& src_i=$1 &&
dst_ls=$(ls -diL "$dst" 2>/dev/null) && set $dst_ls
&& dst_i=$1 &&
diff --git a/src/storage/storage_util.c b/src/storage/storage_util.c
index 899a55758..da6381f52 100644
--- a/src/storage/storage_util.c
+++ b/src/storage/storage_util.c
@@ -64,7 +64,10 @@
#include "virfile.h"
#include "virjson.h"
#include "virqemu.h"
+#pragma GCC diagnostic push
+#pragma GCC diagnostic ignored "-Wunused-parameter"
#include "stat-time.h"
+#pragma GCC diagnostic pop
#include "virstring.h"
#include "virxml.h"
#include "virfdstream.h"
7:55 a.m.
On 01/02/2018 02:09 PM, John Ferlan wrote:
On 01/02/2018 04:28 AM, Michal Privoznik wrote:
> Unfortunately, since gnulib's commit of 2c5d558745 there's an
> unused parameter to stat_time_normalize() function which gnulib
> developers don't want to fix [1]. Therefore, we have to work
> around it by temporarily suspending -Wunused-parameter.
>
> 1: http://lists.gnu.org/archive/html/bug-gnulib/2018-01/msg00000.html
>
> Signed-off-by: Michal Privoznik <mprivozn(a)redhat.com>
> ---
>
> While we have 'gnulib update free' push rule, this one is not trivial at
> all and thus I have not pushed it. It's ugly and I don't like it. So any
> ideas are welcome.
>
> .gnulib | 2 +-
> bootstrap | 4 ++--
> src/storage/storage_util.c | 3 +++
> 3 files changed, 6 insertions(+), 3 deletions(-)
>
No bright ideas on this other than perhaps only including changes just
prior to the particular one that breaks things
Unfortunately, that is not possible. The gnulib's commit that breaks
things was merged 23rd of November and we need the most recent commit
which fixes the stupid year check.
or somehow revert just
that one in our local copy.
I'm worried that in the end this would make us diverge from the gnulib's
upstream too much.
Other thoughts require additional local changes just to "work around"
the broken definition. Such as grabbing the "old" stat-time.h, rename
it, save it in libvirt sources, then use that new name instead of the
new .gnulib file. That's probably worse than what you've done though.
Agreed.
At this point, I think your solution to minimize the impact to one
include file is perhaps the easiest/best solution albeit not perfect.
It's interesting that there is no desire to fix this problem in .gnulib
especially since there are already 2 patches proposed and in reality the
change fundamentally breaks on every platform other than __sun when
STAT_TIMESPEC is defined just because it's possible (or more desirable
in the reviewers feedback) to compile with ignore unused-parameter.
Wonder what would happen if someone posted a patch to .gnulib to revert
the change for the reason that it breaks other platforms that don't
desire to configure in that manner. Perhaps Eric Blake would have some
thoughts or additional muscle with the .gnulib maintainers.
That's what I hope for. Since the problem lies in a header file I think
it's impossible to require all projects that use gnulib to compile
without -Wunused-parameter.
Another solution might be to move the function that causes troubles into
.c since it's purely internal function and nobody outside gnulib
calls/should call it. At least then we could use different sets of
CFLAGS for libvirt and gnulib (not that I'd fancy it or know how to do
it).
Michal
3:37 p.m.
On 01/02/2018 07:09 AM, John Ferlan wrote:
On 01/02/2018 04:28 AM, Michal Privoznik wrote:
> Unfortunately, since gnulib's commit of 2c5d558745 there's an
> unused parameter to stat_time_normalize() function which gnulib
> developers don't want to fix [1]. Therefore, we have to work
> around it by temporarily suspending -Wunused-parameter.
>
> 1: http://lists.gnu.org/archive/html/bug-gnulib/2018-01/msg00000.html
>
> Signed-off-by: Michal Privoznik <mprivozn(a)redhat.com>
> ---
>
> While we have 'gnulib update free' push rule, this one is not trivial at
> all and thus I have not pushed it. It's ugly and I don't like it. So any
> ideas are welcome.
>
> .gnulib | 2 +-
> bootstrap | 4 ++--
> src/storage/storage_util.c | 3 +++
> 3 files changed, 6 insertions(+), 3 deletions(-)
>
No bright ideas on this other than perhaps only including changes just
prior to the particular one that breaks things or somehow revert just
that one in our local copy.
We can provide a downstream-only patch against the gnulib file that adds
the attribute unused marker in our builds, since upstream didn't like
the patch. I'll work up something like that in a moment...
Other thoughts require additional local changes just to "work around"
the broken definition. Such as grabbing the "old" stat-time.h, rename
it, save it in libvirt sources, then use that new name instead of the
new .gnulib file. That's probably worse than what you've done though.
At this point, I think your solution to minimize the impact to one
include file is perhaps the easiest/best solution albeit not perfect.
Yes, the #pragma usage is more concise than carrying a downstream gnulib
patch, but the two approaches are not that much different in
maintenance, so it will be a matter of taste which variant of the patch
to use.
It's interesting that there is no desire to fix this problem in .gnulib
especially since there are already 2 patches proposed and in reality the
change fundamentally breaks on every platform other than __sun when
STAT_TIMESPEC is defined just because it's possible (or more desirable
in the reviewers feedback) to compile with ignore unused-parameter.
I'm also going to reply in the upstream gnulib thread. When the warning
is in a .c file, they are justified in not caring. But when it is in a
.h file, it really does seem like something worth cleaning up in gnulib.
Wonder what would happen if someone posted a patch to .gnulib to
revert
the change for the reason that it breaks other platforms that don't
desire to configure in that manner. Perhaps Eric Blake would have some
thoughts or additional muscle with the .gnulib maintainers.
--
Eric Blake, Principal Software Engineer
Red Hat, Inc. +1-919-301-3266
Virtualization: qemu.org | libvirt.org
8:46 a.m.
On Tue, Jan 02, 2018 at 08:09:37 -0500, John Ferlan wrote:
On 01/02/2018 04:28 AM, Michal Privoznik wrote:
> Unfortunately, since gnulib's commit of 2c5d558745 there's an
> unused parameter to stat_time_normalize() function which gnulib
> developers don't want to fix [1]. Therefore, we have to work
> around it by temporarily suspending -Wunused-parameter.
>
> 1: http://lists.gnu.org/archive/html/bug-gnulib/2018-01/msg00000.html
>
> Signed-off-by: Michal Privoznik <mprivozn(a)redhat.com>
> ---
>
> While we have 'gnulib update free' push rule, this one is not trivial at
> all and thus I have not pushed it. It's ugly and I don't like it. So any
> ideas are welcome.
>
> .gnulib | 2 +-
> bootstrap | 4 ++--
> src/storage/storage_util.c | 3 +++
> 3 files changed, 6 insertions(+), 3 deletions(-)
>
No bright ideas on this other than perhaps only including changes just
prior to the particular one that breaks things or somehow revert just
that one in our local copy.
How about just killing that stupid syntax check in our local copy?
8:52 a.m.
On 01/03/2018 03:46 PM, Peter Krempa wrote:
On Tue, Jan 02, 2018 at 08:09:37 -0500, John Ferlan wrote:
>
>
> On 01/02/2018 04:28 AM, Michal Privoznik wrote:
>> Unfortunately, since gnulib's commit of 2c5d558745 there's an
>> unused parameter to stat_time_normalize() function which gnulib
>> developers don't want to fix [1]. Therefore, we have to work
>> around it by temporarily suspending -Wunused-parameter.
>>
>> 1: http://lists.gnu.org/archive/html/bug-gnulib/2018-01/msg00000.html
>>
>> Signed-off-by: Michal Privoznik <mprivozn(a)redhat.com>
>> ---
>>
>> While we have 'gnulib update free' push rule, this one is not trivial at
>> all and thus I have not pushed it. It's ugly and I don't like it. So any
>> ideas are welcome.
>>
>> .gnulib | 2 +-
>> bootstrap | 4 ++--
>> src/storage/storage_util.c | 3 +++
>> 3 files changed, 6 insertions(+), 3 deletions(-)
>>
>
>
> No bright ideas on this other than perhaps only including changes just
> prior to the particular one that breaks things or somehow revert just
> that one in our local copy.
How about just killing that stupid syntax check in our local copy?
That wouldn't help either. We could not upgrade to newer gnulib unless
it's fixed. But fortunately, it looks like we're close to an agreement
how to fix gnulib so we can update it in our repo too.
michal
3:57 p.m.
On 01/03/2018 08:46 AM, Peter Krempa wrote:
>
> No bright ideas on this other than perhaps only including changes just
> prior to the particular one that breaks things or somehow revert just
> that one in our local copy.
How about just killing that stupid syntax check in our local copy?
As in, tweaking local-checks-to-skip to exclude the copyright date
check? I'd be okay with that idea, so that we don't hit this problem in
future years.
--
Eric Blake, Principal Software Engineer
Red Hat, Inc. +1-919-301-3266
Virtualization: qemu.org | libvirt.org
2515
days inactive
2516
days old
6 comments
4 participants
participants (4)
-
Eric Blake -
John Ferlan -
Michal Privoznik -
Peter Krempa