[libvirt] [tck PATCH v2 2/4] new NetworkHelper function get_network_ip()

[Daniel P. Berrangé]

On Mon, Mar 05, 2018 at 03:32:02PM -0500, Laine Stump wrote:
> On 03/05/2018 10:15 AM, Daniel P. Berrangé wrote:
> > On Mon, Mar 05, 2018 at 10:10:36AM -0500, Laine Stump wrote:
> >> On 03/05/2018 04:31 AM, Daniel P. Berrangé wrote:
> >>> On Thu, Mar 01, 2018 at 09:49:58PM -0500, Laine Stump wrote:
> >>>> This function gets the first IP address for the named virtual
> >>>> network. It is returned as a Net::IP object, so that we will have info
> >>>> about its netmask/prefix and can easily get it broadcast address and
> >>>> perform arithmetic on the address.
> >>>>
> >>>> Signed-off-by: Laine Stump <laine at laine.org>
> >>>> ---
> >>>>
> >>>> Change from V1: return a NetAddr::IP object instead of a string.
> >>>>
> >>>>  lib/Sys/Virt/TCK/NetworkHelpers.pm | 22 ++++++++++++++++++++++
> >>>>  1 file changed, 22 insertions(+)
> >>>>
> >>>> diff --git a/lib/Sys/Virt/TCK/NetworkHelpers.pm b/lib/Sys/Virt/TCK/NetworkHelpers.pm
> >>>> index 5f563e5..7bbce62 100644
> >>>> --- a/lib/Sys/Virt/TCK/NetworkHelpers.pm
> >>>> +++ b/lib/Sys/Virt/TCK/NetworkHelpers.pm
> >>>> @@ -1,4 +1,5 @@
> >>>>  use Sys::Virt::TCK qw(xpath);
> >>>> +use NetAddr::IP qw(:lower);
> >>> This isn't part of base perl, so you'll need to list it in Build.PL and
> >>> the RPM spec file.
> >> I originally assumed that, but remembered seeing "something somewhere"
> >> about implicit dependencies and decided to try it out by not listing it
> >> in the specfile - on both Fedora and RHEL7 the dependency was properly
> >> pulled in and it was installed.
> >>
> >> This leads to one of three possibilities:
> >>
> >> 1) implicit dependencies are figured out properly by yum and dnf (at
> >> least for RHEL7, don't know about RHEL6).
> >>
> >> 2) (1), but it's just coincidentally happening and not guaranteed.
> >>
> >> 3) I wasn't paying attention when I tested, and what I say isn't
> >> actually true.
> >>
> >> I don't have any problem putting in the explicit Requires though. Can I
> >> assumed a Reviewed-by with that in place?
> > We don't need to list it with a Requires:  tag because automatic
> > dependancies take care of that. We need it listed as BuildRequires
> > though, *if* the NetworkHelpers mod is pulled in by any of the unit
> > tests which I thought it was (but I could be wrong there).
> 
> Well, being the perl-idiot that I am, I didn't realize that the unit
> tests even *existed* until you said that and I investigated to see what
> you were talking about :-)
> 
> So what you're saying is that if any function in NetworkHelpers is
> either directly or indirectly used in the unit tests (which are in the
> "t" subdirectory of the source tree, right?) then we need to list is in
> BuildRequires, correct? I looked in the unit tests  and while
> NetworkBuilders is pulled it, NetworkHelpers isn't (and NetworkBuilders
> doesn't pull in NetworkHelpers), so I *think* we're safe, but I'll try
> building on a system where I've removed the NetAddr::IP package to
> verify that it's still successful..

Yep, that sounds fine then. 

> > Still need it in the Build.PL no matter what, as that's what CPAN
> > and other Perl tools use.
> 
> Ah, for non-rpm-based installations, right?

Yes, exactly - the cpan CLI uses the info there to figure out what
dependancies need installing.

Regards,
Daniel
-- 
|: https://berrange.com      -o-    https://www.flickr.com/photos/dberrange :|
|: https://libvirt.org         -o-            https://fstop138.berrange.com :|
|: https://entangle-photo.org    -o-    https://www.instagram.com/dberrange :|