8:05 a.m.
Currently, we are checking if libvirt.so contains public symbols.
However, sometimes we rename an internal symbol and forget to
change libvirt_private.syms accordingly. Hence, it's safer to check
for internal symbols as well.
---
src/Makefile.am | 7 ++++++-
src/check-symfile.pl | 2 +-
2 files changed, 7 insertions(+), 2 deletions(-)
diff --git a/src/Makefile.am b/src/Makefile.am
index ae3d491..c5840c0 100644
--- a/src/Makefile.am
+++ b/src/Makefile.am
@@ -318,8 +318,13 @@ if WITH_LINUX
check-symfile: libvirt.syms libvirt.la
$(AM_V_GEN)$(PERL) $(srcdir)/check-symfile.pl libvirt.syms \
.libs/libvirt.so
+
+check-private-symfile: libvirt_private.syms libvirt.la
+ $(AM_V_GEN)$(PERL) $(srcdir)/check-symfile.pl libvirt_private.syms \
+ .libs/libvirt.so
else
check-symfile:
+check-private-symfile:
endif
PROTOCOL_STRUCTS = \
@@ -344,7 +349,7 @@ else !WITH_REMOTE
check-protocol:
endif
EXTRA_DIST += $(PROTOCOL_STRUCTS) check-symfile.pl
-check-local: check-protocol check-symfile
+check-local: check-protocol check-symfile check-private-symfile
.PHONY: check-protocol $(PROTOCOL_STRUCTS:structs=struct)
# Mock driver, covering domains, storage, networks, etc
diff --git a/src/check-symfile.pl b/src/check-symfile.pl
index ac37b46..435e045 100755
--- a/src/check-symfile.pl
+++ b/src/check-symfile.pl
@@ -44,7 +44,7 @@ foreach my $elflib (@elflibs) {
close NM;
}
-foreach my $sym (@wantsyms) {
+foreach my $sym (keys(%wantsyms)) {
next if exists $gotsyms{$sym};
print STDERR "Expected symbol $sym is not in ELF library\n";
--
1.7.8.6
8:13 a.m.
On 10/05/2012 07:05 AM, Michal Privoznik wrote:
Currently, we are checking if libvirt.so contains public symbols.
However, sometimes we rename an internal symbol and forget to
change libvirt_private.syms accordingly. Hence, it's safer to check
for internal symbols as well.
---
src/Makefile.am | 7 ++++++-
src/check-symfile.pl | 2 +-
2 files changed, 7 insertions(+), 2 deletions(-)
ACK. I confirmed that this catches the problem fixed by your commit
fcd6477.
--
Eric Blake eblake(a)redhat.com +1-919-301-3266
Libvirt virtualization library http://libvirt.org
8:15 a.m.
On 05.10.2012 15:13, Eric Blake wrote:
On 10/05/2012 07:05 AM, Michal Privoznik wrote:
> Currently, we are checking if libvirt.so contains public symbols.
> However, sometimes we rename an internal symbol and forget to
> change libvirt_private.syms accordingly. Hence, it's safer to check
> for internal symbols as well.
> ---
> src/Makefile.am | 7 ++++++-
> src/check-symfile.pl | 2 +-
> 2 files changed, 7 insertions(+), 2 deletions(-)
>
ACK. I confirmed that this catches the problem fixed by your commit
fcd6477.
Thanks, pushed.
Michal
9:42 a.m.
On 10/05/2012 07:05 AM, Michal Privoznik wrote:
Currently, we are checking if libvirt.so contains public symbols.
However, sometimes we rename an internal symbol and forget to
change libvirt_private.syms accordingly. Hence, it's safer to check
for internal symbols as well.
---
src/Makefile.am | 7 ++++++-
src/check-symfile.pl | 2 +-
2 files changed, 7 insertions(+), 2 deletions(-)
Phooey. I had only tested in-tree builds when I gave ack.
check-symfile: libvirt.syms libvirt.la
$(AM_V_GEN)$(PERL) $(srcdir)/check-symfile.pl libvirt.syms \
.libs/libvirt.so
This works in VPATH builds, since libvirt.syms is version-controlled...
+
+check-private-symfile: libvirt_private.syms libvirt.la
+ $(AM_V_GEN)$(PERL) $(srcdir)/check-symfile.pl libvirt_private.syms \
+ .libs/libvirt.so
...but this fails in a VPATH build, since libvirt_private.syms is
generated into builddir. I'm working on a followup.
--
Eric Blake eblake(a)redhat.com +1-919-301-3266
Libvirt virtualization library http://libvirt.org
9:50 a.m.
On 10/05/2012 08:42 AM, Eric Blake wrote:
On 10/05/2012 07:05 AM, Michal Privoznik wrote:
> Currently, we are checking if libvirt.so contains public symbols.
> However, sometimes we rename an internal symbol and forget to
> change libvirt_private.syms accordingly. Hence, it's safer to check
> for internal symbols as well.
> ---
> src/Makefile.am | 7 ++++++-
> src/check-symfile.pl | 2 +-
> 2 files changed, 7 insertions(+), 2 deletions(-)
>
Phooey. I had only tested in-tree builds when I gave ack.
> check-symfile: libvirt.syms libvirt.la
> $(AM_V_GEN)$(PERL) $(srcdir)/check-symfile.pl libvirt.syms \
> .libs/libvirt.so
This works in VPATH builds, since libvirt.syms is version-controlled...
> +
> +check-private-symfile: libvirt_private.syms libvirt.la
> + $(AM_V_GEN)$(PERL) $(srcdir)/check-symfile.pl libvirt_private.syms \
> + .libs/libvirt.so
...but this fails in a VPATH build, since libvirt_private.syms is
generated into builddir. I'm working on a followup.
I got that backwards. libvirt.syms is generated, and in builddir, and
_includes_ all of libvirt_public.syms AND libvirt_private.syms.
libvirt_private.syms is in srcdir only, so the VPATH build is failing to
find it. The reason that your patch "worked" at detecting the failure is
not because of the new makefile rule, but because of the fix to use
keys(%wantsyms) instead of @wantsyms in the perl script, where
check-symfile, rather than the new check-private-symfile, was detecting
the error.
--
Eric Blake eblake(a)redhat.com +1-919-301-3266
Libvirt virtualization library http://libvirt.org
9:58 a.m.
New subject: [libvirt] [PATCH] build: fix VPATH builds
This reverts part of commit 5468594f4655d20674a9429a0671f6afa3538274;
the perl changes were sufficient. Since libvirt.syms is already a
generated file created from libvirt_private.syms, we don't need a
second pass over libvirt_private.syms in isolation.
---
Pushing under the build-breaker rule.
src/Makefile.am | 7 +------
1 file changed, 1 insertion(+), 6 deletions(-)
diff --git a/src/Makefile.am b/src/Makefile.am
index c5840c0..ae3d491 100644
--- a/src/Makefile.am
+++ b/src/Makefile.am
@@ -318,13 +318,8 @@ if WITH_LINUX
check-symfile: libvirt.syms libvirt.la
$(AM_V_GEN)$(PERL) $(srcdir)/check-symfile.pl libvirt.syms \
.libs/libvirt.so
-
-check-private-symfile: libvirt_private.syms libvirt.la
- $(AM_V_GEN)$(PERL) $(srcdir)/check-symfile.pl libvirt_private.syms \
- .libs/libvirt.so
else
check-symfile:
-check-private-symfile:
endif
PROTOCOL_STRUCTS = \
@@ -349,7 +344,7 @@ else !WITH_REMOTE
check-protocol:
endif
EXTRA_DIST += $(PROTOCOL_STRUCTS) check-symfile.pl
-check-local: check-protocol check-symfile check-private-symfile
+check-local: check-protocol check-symfile
.PHONY: check-protocol $(PROTOCOL_STRUCTS:structs=struct)
# Mock driver, covering domains, storage, networks, etc
--
1.7.11.4
4432
days inactive
4432
days old
5 comments
2 participants
participants (2)
-
Eric Blake -
Michal Privoznik