Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Module::Signature dependency #1

Open
charsbar opened this issue Apr 11, 2012 · 4 comments
Open

Module::Signature dependency #1

charsbar opened this issue Apr 11, 2012 · 4 comments

Comments

@charsbar
Copy link
Contributor

Do we really want people to install Module::Signature as a dependency? Even if they only use Module::CPANTS::Analyse as a dependency of Test::Kwalitee?

It's ok for a server-side dependency, but I think it better not to include Module::Signature dependency in Build.PL, and test if Module::Signature is available in Module::CPANTS::Kwalitee::Signature (and if not, just skip).

Thoughts?

@daxim
Copy link
Owner

daxim commented Apr 14, 2012

What's wrong with it?

What makes e.g. Module::Pluggable an acceptable dependency and Module::Signature not?

@charsbar
Copy link
Contributor Author

First of all, Module::Signature depends on an external executable or an XS module, and doesn't always install cleanly under some environments (http://www.cpantesters.org/distro/M/Method-Signatures.html). So if we depend on it, Test::Kwalitee (one of the reasons (or only one reason) why Module::CPANTS::Analyse should be an independent module from other server-side components) may fail to install, either, which is bad.

Secondly, signature test is something you should do after you make a tarball, that means, it's useless when module authors run "make test" or "prove -lr t/" before they ship. As Test::Kwalitee is generally for module authors and not for users who install their modules, it probably would ignore a signature test anyway (like extracts_nicely). I doubt there's any good reason for Test::Kwalitee users to install Module::Signature.

Thirdly, it has (or had) long been considered a module to avoid at least by some people (and I consider so too; see http://cpanratings.perl.org/dist/Module-Signature), so CPAN::FirstTime turns off its signature check by default. I think some of the issues noted at the rating page have already been fixed, but I don't want to encourage people to install it (as I said before, it's ok for a server-side maintainer, though I'm afraid it might increase the load significantly).

I'm not saying that adding dependencies is simply bad; adding Module::Signature as a dependency has more downsides than upsides.

@charsbar
Copy link
Contributor Author

And the signature test should probably be extra metrics because of the second issue I wrote above (it's not something module authors can easily test by themselves before they ship).

@daxim
Copy link
Owner

daxim commented Apr 15, 2012

Module::Signature depends on an external executable

This is not a hard dependency, there's a fallback to an OpenPGP Perl module.

or an XS module,

M::C::A depends on XS modules, too. Someone who has trouble installing M::S for this reason will fail totally with M::C::A. This is not a good argument.

doesn't always install cleanly under some environments

CPAN testers reports for the current M::C::A trial release overwhelmingly indicate success. If bugs in M::S prevent people from using it, the answer is not that M::C::A should stick its head in the sand, but instead the bugs in M::S should be fixed.

signature test is something you should do after you make a tarball, that means, it's useless when module authors run "make test" or "prove -lr t/" before they ship.

Authors should run make disttest at release time, or cpants_lint.pl on the tarball before uploading. I have verified that both catch a broken signature/detect the valid_signature violation.

As Test::Kwalitee is generally for module authors and not for users who install their modules, it probably would ignore a signature test anyway (like extracts_nicely).

I agree that users should not run author tests. I trivially added valid_signature to T::K. I have verified that it does catch a broken signature since it operates on the directory after a make distdir step; in other words: the valid_signature indicator does not need a tarball.

see http://cpanratings.perl.org/dist/Module-Signature

What's the compelling part of it?

long been considered a module to avoid … so CPAN::FirstTime turns off its signature check by default

I find no evidence for this claim. Rather, the check_sigs default value has been "off" since the beginning (commit e09a2336ad1e2598dd69c75a58ea0a08eaacf5bd) back in 2006 when no one had formed an opinion about M::S yet.

some of the issues noted at the rating page have already been fixed

The line-ending bugs mentioned by Robert and Burak are fixed in 0.54 and 0.67 each. The feature requests about subkeys and WoT are not implemented yet. The module is usable despite the lack of these features.

I'm afraid it might increase the load significantly

Do you have measurement numbers? Is the load so debilitating that M::S performs a denial of service?

signature test should probably be extra metrics

The purpose of valid_signature is not a signature test per se (a signature.t does that already) - but to catch dists that have invalid signatures. These dists are essentially broken because they are not installable (not even with "force") by people with CPAN and M::S, therefore I veto status as only extra category metric. These dists actually should be withdrawn from the net, like release managers outside the Perl world do with their signed tarballs when people notice that the signature does not match the contents for whatever reason.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants