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

[Laine Stump]

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..


>
> 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?

Okay, I'll add that and resend.