Bug 1542666 - perl-Net-OpenSSH package fails to pull in required IO::Pty module
Summary: perl-Net-OpenSSH package fails to pull in required IO::Pty module
Keywords:
Status: CLOSED NEXTRELEASE
Alias: None
Product: Fedora
Classification: Fedora
Component: perl-Net-OpenSSH
Version: 28
Hardware: Unspecified
OS: Unspecified
unspecified
unspecified
Target Milestone: ---
Assignee: Steve Traylen
QA Contact: Fedora Extras Quality Assurance
URL:
Whiteboard:
Depends On:
Blocks:
TreeView+ depends on / blocked
 
Reported: 2018-02-06 18:35 UTC by Laine Stump
Modified: 2018-04-24 14:16 UTC (History)
6 users (show)

Fixed In Version:
Doc Type: If docs needed, set a value
Doc Text:
Clone Of:
Environment:
Last Closed: 2018-04-24 10:06:56 UTC
Type: Bug
Embargoed:


Attachments (Terms of Use)

Description Laine Stump 2018-02-06 18:35:30 UTC
perl-Net-OpenSSH needs to have IO::Pty to operate properly. The specfile has:

  Suggests: perl(IO::Pty)

but that is wrong on two counts:

1) IO::Pty is actually just included as a part of IO:Tty

2) The Suggests: isn't "strong enough" to actually get the proper module installed, so when I try to use Net::OpenSSH, I get this error:


unable to load Perl module IO::Pty: Can't locate IO/Pty.pm in @INC (you may need to install the IO::Pty module) (@INC contains: /usr/local/lib64/perl5 /usr/local/share/perl5 /usr/lib64/perl5/vendor_perl /usr/share/perl5/vendor_perl /usr/lib64/perl5 /usr/share/perl5 .) at (eval 48) line 1.
 at /usr/share/perl5/vendor_perl/Net/OpenSSH.pm line 917.

The fix that I found is to change the above Suggests to this:

  Requires: perl(IO::Tty)

Both changes are needed - I tried "Suggests: perl(IO::Tty)" and that didn't get IO:Tty installed as a dependency.

I'm filing this against rawhide, but the same change needs to be made in F26 and F27.

Comment 1 Petr Pisar 2018-02-07 09:03:42 UTC
Few notes:

(1) If you need IO::Pty, then requiring IO::Tty is wrong. You need to require IO::Pty.

(2) The dependency on IO::Pty is weak because it's optional according to Net-OpenSSH author, see README:

  DEPENDENCIES

  This module requires these other modules and libraries:

  Test::More
  IO::Pty (optional, required for password authentication)

We can change it Fedora, but that won't change user experience for people installing it from CPAN. If you write a code that rely on presence of module, you will still have to specify the dependency in your code metadata.
 
Could you please provide a code that triggers the need for IO::Pty?

(3) The IO::Pty is not the only optional module. There more of them:

lib/Net/OpenSSH.pm:917:        _load_module('IO::Pty');
lib/Net/OpenSSH.pm:1497:            _load_module('IO::Pty');
lib/Net/OpenSSH.pm:1646:        _load_module('Encode');
lib/Net/OpenSSH.pm:2317:    _load_module('Net::SFTP::Foreign', '1.47');
lib/Net/OpenSSH.pm:2392:    _load_module('Object::Remote') or return;
lib/Net/OpenSSH.pm:2393:    _load_module('Net::OpenSSH::ObjectRemote') or return;
lib/Net/OpenSSH.pm:2400:    _load_module('Net::SSH::Any');

Do you think we should turn all of them into a hard dependency?

Comment 2 Laine Stump 2018-02-08 02:27:51 UTC
(1) Okay, I see now that "Requires: perl(IO::Pty)" is properly resolved by rpmbuild to require perl-IO-Tty. I thought I had tried that, but most likely only tried with the original "Suggests: perl(IO::Pty)" and incorrectly jumped to the conclusion that it was due to the module being included in the IO::Tty package (when in fact it was just because "Suggests:" is a useless directive :-P). I finally got it working correctly with Requires perl(IO::Tty) and apparently didn't try backtracking enough.

(2) I haven't been able to figure out a valid use for "Suggests:" Since it's not possible for dnf to know how applications I will install in the future might use Net::OpenSSH, there's no way for the presence of a "Suggests: perl(IO::Pty)" to ever actually lead to that module being installed - as far as I can see the only use for Suggests: is to provide a clue to someone whose application fails, and who then knows enough to go back to the specfile for the parent package, which seems not much of a use at all.

This is the sum of my usage of Net::OpenSSH (in the libvirt-tck package, which starts up a throwaway virtual machine to test the libvirt library):

my $ssh = Net::OpenSSH->new($guestip,
                            user => "root",
                            password => $tck->root_password());
($stdout, $stderr)  = $ssh->capture2("bob");
($stdout, $stderr)  = $ssh->capture2("loblaw");
...

Yeah yeah, allowing password authentication is bad security, I get it. I imagine it's reasonably common though, especially in throwaway scenarios like this (and I also can't imagine that perl-IO-Tty is very large).

(3) To be honest, I think Suggests: is pointless, and any module that is referenced within a package should have a hard Requires:. If someone can explain to me how having Suggests: can lead to the modules being magically installed once they are called, then I might change my opinion :-)

Comment 3 Petr Pisar 2018-02-08 08:58:38 UTC
Suggests: is for optional dependencies. Optional means that someone needs it and someone does not. No package manager can magically decide whether an optional dependency is or is not needed. (If there were a proper instrumentation in Fedora, some of them could be formalized and resolved automatically, but some is not all.)

I recommend you reading <https://pagure.io/packaging-committee/issue/694>.

Because your request is against my honest opinion, I will not implement it and I will leave the decision on the package maintainer.

Comment 4 Paul Howarth 2018-02-08 10:48:43 UTC
As libvirt-tck uses Net::OpenSSH in a way that requires IO::Pty, why not add a dependency on perl(IO::Pty) to libvirt-tck, since that is where it is needed?

Comment 5 Laine Stump 2018-02-08 14:47:28 UTC
> I recommend you reading <https://pagure.io/packaging-committee/issue/694>.

That looks to me like it's talking about a different issue - it's pointing out the problems of having one application that depends on a library that's supplied with the package of another application (i.e. the other package has both an application and a library). Did you maybe mean to point to a different issue?)

> As libvirt-tck uses Net::OpenSSH in a way that requires IO::Pty, why not add
> a dependency on perl(IO::Pty) to libvirt-tck, since that is where it is needed?

That's my fallback plan if Net::OpenSSH doesn't get a Requires:, but doing that leads to maintenance problems - what if the Net::OpenSSH  implementation is changed to use something else instead of IO:Pty? (and that module is given the same Suggests: treatment in the Net::OpenSSH specfile). The user updates their packages, pulls in the new Net::OpenSSH, and suddenly libvirt-tck stops working (and there is the less important issue that IO::Tty will still be installed even though it is no longer used). Just as I dislike C .h files that require their consumers to also include other .h files that end up only being used in the first .h, I think packages should not need to list dependencies on other packages that they don't directly reference.

There's likely several other perl modules that could arguably be made Suggests: instead of Requires: in the Net::OpenSSH specfile because "module X is only used if you use Y function in Net::OpenSSH", but where do you draw the line? Maybe you should just make them all Suggests: and force consumer packages (and user applications) to add their own Requires for (or manually install) every individual subpackage. That seems very unfriendly though...

Comment 6 Petr Pisar 2018-02-08 15:50:54 UTC
It's the correct issue. It talks about this very same problem. It's not important whether the disputed item is an icon in Gnome or a Perl module in @INC. It also discusses the (un)helpfulnes of weak dependencies.

The line for me is that a module load failure is handled by the code. In this case the error message does not looks so, but there is a special handling in the code instead of a plain "require Foo;" or so. Also the documentations clearly states it's an optional feature.

In an ideal world this package would declare a "password authentication" as an optional feature and connect it to the dependency on IO::Pty. And your package would require the "password authentication" feature. The reality is that RPM does
not support it.

Comment 7 Fedora End Of Life 2018-02-20 15:29:10 UTC
This bug appears to have been reported against 'rawhide' during the Fedora 28 development cycle.
Changing version to '28'.

Comment 8 Steve Traylen 2018-04-24 10:04:42 UTC
Adding 'perl(IO::Pty)' to the requires is fine. 

It's a tiny dep at the end of the day.

Comment 9 Steve Traylen 2018-04-24 10:06:56 UTC
Oh 
Suggests:       perl(IO::Pty)

has already been added.

Comment 10 Emmanuel Seyman 2018-04-24 14:16:17 UTC
I think Paul correctly summed up the problem in comment #4.

Net::OpenSSH doesn't require perl(IO::Pty) except when called in the libvirt-tck package.


Note You need to log in before you can comment on or make changes to this bug.