Bug 891873 - Review Request: perl-Net-IDN-Nameprep - Stringprep Profile for Internationalized Domain Names (RFC 3491)
Review Request: perl-Net-IDN-Nameprep - Stringprep Profile for Internationali...
Status: CLOSED NEXTRELEASE
Product: Fedora
Classification: Fedora
Component: Package Review (Show other bugs)
rawhide
All Linux
medium Severity medium
: ---
: ---
Assigned To: Petr Pisar
Fedora Extras Quality Assurance
:
Depends On: 891868
Blocks: FE-Legal 892433
  Show dependency treegraph
 
Reported: 2013-01-04 05:23 EST by Mathieu Bridon
Modified: 2013-02-06 22:52 EST (History)
4 users (show)

See Also:
Fixed In Version:
Doc Type: Bug Fix
Doc Text:
Story Points: ---
Clone Of:
Environment:
Last Closed: 2013-02-06 22:52:13 EST
Type: ---
Regression: ---
Mount Type: ---
Documentation: ---
CRM:
Verified Versions:
Category: ---
oVirt Team: ---
RHEL 7.3 requirements from Atomic Host:
ppisar: fedora‑review+
limburgher: fedora‑cvs+


Attachments (Terms of Use)

  None (edit)
Description Mathieu Bridon 2013-01-04 05:23:28 EST
Spec URL: http://bochecha.fedorapeople.org/packages/perl-Net-IDN-Nameprep.spec
SRPM URL: http://bochecha.fedorapeople.org/packages/perl-Net-IDN-Nameprep-1.101-1.fc18.src.rpm

Description:
This module implements the nameprep specification, which describes how to
prepare internationalized domain name (IDN) labels in order to increase the
likelihood that name input and name comparison work in ways that make sense
for typical users throughout the world. Nameprep is a profile of the
stringprep protocol and is used as part of a suite of on-the-wire protocols
for internationalizing the Domain Name System (DNS).

Fedora Account System Username: bochecha
Comment 1 Mathieu Bridon 2013-01-04 05:24:03 EST
This depends on Unicode::Stringprep, submitted for review as bug 891868.
Comment 2 Petr Pisar 2013-01-22 09:29:33 EST
URL and Source0 are usable. Ok.
Source file is original (SHA-256: 2861058143a8b26fb06b691ad289e3c6a0b0ae093f32769089f2a7c5fae4d4f8). Ok.
Summary verified from lib/Net/IDN/Nameprep.pm. Ok.
Description verified from lib/Net/IDN/Nameprep.pm. Ok.

Licence of t/nameprep_vec.t is not (GPL+ or Artistic):

# Copyright (C) The Internet Society (2003). All Rights Reserved.
#
# This document and translations of it may be copied and furnished
# to others, and derivative works that comment on or otherwise
# explain it or assist in its implementation may be prepared,
# copied, published and distributed, in whole or in part, without
# restriction of any kind, provided that the above copyright
# notice and this paragraph are included on all such copies and
# derivative works. However, this document itself may not be
# modified in any way, such as by removing the copyright notice or
# references to the Internet Society or other Internet
# organizations, except as needed for the purpose of developing
# Internet standards in which case the procedures for copyrights
# defined in the Internet Standards process must be followed, or
# as required to translate it into languages other than English.

It's not included into binary package. So it does not affect License tag in spec file.

However it forbids modifications. Actually I don't understand the text clearly: It allows derivative works, but it forbids modifying original document. This contradicts. Maybe the copyright holder wanted to forbid modification of the license text. But this way how it is written is ambiguous to me. Fedora cannot redistribute code which cannot be modified.

FIX: Please ask <fedora-legal-list@redhat.com> for clarification.


No XS code included, noarch BuildArch is Ok.

TODO: You can replace %{__perl} macro with plain perl.

TODO: Build-require `perl(Exporter)' (lib/Net/IDN/Nameprep.pm:10).

FIX: Build-require `perl(Unicode::Stringprep::Mapping)' (lib/Net/IDN/Nameprep.pm:16).
FIX: Build-require `perl(Unicode::Stringprep::Prohibited)' (lib/Net/IDN/Nameprep.pm:17).

All tests pass. Ok.

$ rpmlint perl-Net-IDN-Nameprep.spec ../SRPMS/perl-Net-IDN-Nameprep-1.101-1.fc19.src.rpm ../RPMS/noarch/perl-Net-IDN-Nameprep-1.101-1.fc19.noarch.rpm
perl-Net-IDN-Nameprep.src: W: spelling-error Summary(en_US) Stringprep -> String prep, String-prep, Stripping
perl-Net-IDN-Nameprep.src: W: spelling-error %description -l en_US stringprep -> string prep, string-prep, stripping
perl-Net-IDN-Nameprep.noarch: W: spelling-error Summary(en_US) Stringprep -> String prep, String-prep, Stripping
perl-Net-IDN-Nameprep.noarch: W: spelling-error %description -l en_US stringprep -> string prep, string-prep, stripping
perl-Net-IDN-Nameprep.noarch: W: spurious-executable-perm /usr/share/doc/perl-Net-IDN-Nameprep-1.101/Changes
perl-Net-IDN-Nameprep.noarch: W: file-not-utf8 /usr/share/doc/perl-Net-IDN-Nameprep-1.101/README
perl-Net-IDN-Nameprep.noarch: E: script-without-shebang /usr/share/perl5/vendor_perl/Net/IDN/Nameprep.pm
perl-Net-IDN-Nameprep.noarch: W: file-not-utf8 /usr/share/doc/perl-Net-IDN-Nameprep-1.101/LICENSE
2 packages and 1 specfiles checked; 1 errors, 7 warnings.

FIX: Remove executable bit from Changes file.
FIX: Convert LICENSE and README to UTF-8.
FIX: Remove executable bit from Nameprep.pm.

$ rpm -q -lv -p ../RPMS/noarch/perl-Net-IDN-Nameprep-1.101-1.fc19.noarch.rpm                                                              drwxr-xr-x    2 root    root                        0 Jan 22 15:15 /usr/share/doc/perl-Net-IDN-Nameprep-1.101
-rwxr-xr-x    1 root    root                     1176 Dec  8  2011 /usr/share/doc/perl-Net-IDN-Nameprep-1.101/Changes
-rw-r--r--    1 root    root                    18407 Dec  8  2011 /usr/share/doc/perl-Net-IDN-Nameprep-1.101/LICENSE
-rw-r--r--    1 root    root                      769 Dec  8  2011 /usr/share/doc/perl-Net-IDN-Nameprep-1.101/README
-rw-r--r--    1 root    root                     2731 Jan 22 15:15 /usr/share/man/man3/Net::IDN::Nameprep.3pm.gz
drwxr-xr-x    2 root    root                        0 Jan 22 15:15 /usr/share/perl5/vendor_perl/Net
drwxr-xr-x    2 root    root                        0 Jan 22 15:15 /usr/share/perl5/vendor_perl/Net/IDN
-rwxr-xr-x    1 root    root                     3004 Jan 22 15:15 /usr/share/perl5/vendor_perl/Net/IDN/Nameprep.pm
FIX: Remove executable bit from /usr/share/doc/perl-Net-IDN-Nameprep-1.101/Changes and /usr/share/perl5/vendor_perl/Net/IDN/Nameprep.pm.

$ rpm -q --requires -p ../RPMS/noarch/perl-Net-IDN-Nameprep-1.101-1.fc19.noarch.rpm | sort | uniq -c
      1 perl(Exporter)
      1 perl(:MODULE_COMPAT_5.16.2)
      1 perl(strict)
      1 perl(Unicode::Stringprep) >= 1.1
      1 perl(Unicode::Stringprep::Mapping)
      1 perl(Unicode::Stringprep::Prohibited)
      1 perl(utf8)
      1 perl(warnings)
      1 rpmlib(CompressedFileNames) <= 3.0.4-1
      1 rpmlib(FileDigests) <= 4.6.0-1
      1 rpmlib(PayloadFilesHavePrefix) <= 4.0-1
      1 rpmlib(PayloadIsXz) <= 5.2-1
Binary requires are Ok.

$ rpm -q --provides -p ../RPMS/noarch/perl-Net-IDN-Nameprep-1.101-1.fc19.noarch.rpm | sort | uniq -c
      1 perl(Net::IDN::Nameprep) = 1.101
      1 perl-Net-IDN-Nameprep = 1.101-1.fc19
Binary provides are Ok.

$ resolvedeps f19-build ../RPMS/noarch/perl-Net-IDN-Nameprep-1.101-1.fc19.noarch.rpm 
Binary dependencies resolvable. Ok.

Package builds in F19 (http://koji.fedoraproject.org/koji/taskinfo?taskID=4893535). Ok.

Otherwise the package is in line with Fedora and Packaging guidelines.

Please correct all `FIX' issues, obtain blessing from Fedora Legal departments, consider fixing all `TODO' items, and provide new spec file.
Resolution: NOT approved.
Comment 3 Mathieu Bridon 2013-01-22 23:45:08 EST
(In reply to comment #2)
> TODO: You can replace %{__perl} macro with plain perl.

Fixed.

> TODO: Build-require `perl(Exporter)' (lib/Net/IDN/Nameprep.pm:10).

Fixed.

> FIX: Build-require `perl(Unicode::Stringprep::Mapping)'
> (lib/Net/IDN/Nameprep.pm:16).
> FIX: Build-require `perl(Unicode::Stringprep::Prohibited)'
> (lib/Net/IDN/Nameprep.pm:17).

I added the requirements, but now I'm wondering: should they be versioned explicitly, like Unicode::Stringprep just above? (version >= 1.1 is required)

After all, they are all modules from the same package.

> FIX: Remove executable bit from Changes file.
> FIX: Remove executable bit from Nameprep.pm.
> FIX: Convert LICENSE and README to UTF-8.

Fixed.

> Please correct all `FIX' issues, obtain blessing from Fedora Legal
> departments, consider fixing all `TODO' items, and provide new spec file.
> Resolution: NOT approved.

Those should all be fixed, here's the new submission.

Spec URL: http://bochecha.fedorapeople.org/packages/perl-Net-IDN-Nameprep.spec
SRPM URL: http://bochecha.fedorapeople.org/packages/perl-Net-IDN-Nameprep-1.101-2.fc19.src.rpm
Comment 4 Mathieu Bridon 2013-01-22 23:50:44 EST
I left the legal issue out from the previous comment so that you could still review the technical changes separately.

(In reply to comment #2)
> Licence of t/nameprep_vec.t is not (GPL+ or Artistic):
> 
> # Copyright (C) The Internet Society (2003). All Rights Reserved.
> #
> # This document and translations of it may be copied and furnished
> # to others, and derivative works that comment on or otherwise
> # explain it or assist in its implementation may be prepared,
> # copied, published and distributed, in whole or in part, without
> # restriction of any kind, provided that the above copyright
> # notice and this paragraph are included on all such copies and
> # derivative works. However, this document itself may not be
> # modified in any way, such as by removing the copyright notice or
> # references to the Internet Society or other Internet
> # organizations, except as needed for the purpose of developing
> # Internet standards in which case the procedures for copyrights
> # defined in the Internet Standards process must be followed, or
> # as required to translate it into languages other than English.
> 
> It's not included into binary package. So it does not affect License tag in
> spec file.
> 
> However it forbids modifications. Actually I don't understand the text
> clearly: It allows derivative works, but it forbids modifying original
> document. This contradicts. Maybe the copyright holder wanted to forbid
> modification of the license text. But this way how it is written is
> ambiguous to me. Fedora cannot redistribute code which cannot be modified.

Yeah, that's not clear at all. :-/

It seems to me that the code of the unit test is under the same license as the module itself, and that this copyright notice only applies to the test vectors.

At least that's what I understand from the lines just above it:

# Test vectors extracted from:
#
#                    Nameprep and IDNA Test Vectors
#                   draft-josefsson-idn-test-vectors

> FIX: Please ask <fedora-legal-list@redhat.com> for clarification.

Legal people, what's your take on this?
Comment 5 Petr Pisar 2013-01-23 04:21:58 EST
Spec file changes:
--- perl-Net-IDN-Nameprep.spec.old      2013-01-04 11:22:13.000000000 +0100
+++ perl-Net-IDN-Nameprep.spec  2013-01-23 05:44:32.000000000 +0100
@@ -1,19 +1,22 @@
 Name:           perl-Net-IDN-Nameprep
 Summary:        Stringprep Profile for Internationalized Domain Names (RFC 3491)
 Version:        1.101
-Release:        1%{?dist}
+Release:        2%{?dist}
 License:        GPL+ or Artistic
 URL:            http://search.cpan.org/dist/Net-IDN-Nameprep/
 Source0:        http://www.cpan.org/authors/id/C/CF/CFAERBER/Net-IDN-Nameprep-%{version}.tar.gz

 BuildArch:      noarch

+BuildRequires:  perl(Exporter)
 BuildRequires:  perl(Module::Build)
 BuildRequires:  perl(Test::More)
 BuildRequires:  perl(Test::NoWarnings)
 BuildRequires:  perl(Unicode::Stringprep) >= 1.1
+BuildRequires:  perl(Unicode::Stringprep::Mapping)
+BuildRequires:  perl(Unicode::Stringprep::Prohibited)

-Requires:       perl(:MODULE_COMPAT_%(eval "`%{__perl} -V:version`"; echo $version))
+Requires:       perl(:MODULE_COMPAT_%(eval "`perl -V:version`"; echo $version))

 %{?perl_default_filter}

@@ -29,9 +32,19 @@
 %prep
 %setup -q -n Net-IDN-Nameprep-%{version}

+# Remove incorrect executable bits
+chmod -x Changes \
+         lib/Net/IDN/Nameprep.pm
+
+# Convert files to UTF-8
+for FILE in LICENSE README; do
+  iconv -f ISO_8859-1 -t UTF8 $FILE > $FILE.utf8
+  mv $FILE.utf8 $FILE
+done
+

 %build
-%{__perl} Build.PL installdirs=vendor
+perl Build.PL installdirs=vendor
 ./Build


@@ -52,5 +65,11 @@


 %changelog
+* Wed Jan 23 2013 Mathieu Bridon <mathieu.bridon@network-box.com> - 1.101-2
+- Replace the usage of the %%{__perl} macro by the plain perl command.
+- Add missing build requirements.
+- Remove the incorrect executable bits.
+- Make sure all files are UTF-8 encoded.
+
 * Fri Jan 04 2013 Mathieu Bridon <bochecha@fedoraproject.org> - 1.101-1
 - Initial package for Fedora, with help from cpanspec.

> TODO: You can replace %{__perl} macro with plain perl.
-Requires:       perl(:MODULE_COMPAT_%(eval "`%{__perl} -V:version`"; echo $version))
+Requires:       perl(:MODULE_COMPAT_%(eval "`perl -V:version`"; echo $version))
Ok.

> TODO: Build-require `perl(Exporter)' (lib/Net/IDN/Nameprep.pm:10).
+BuildRequires:  perl(Exporter)
Ok.

> FIX: Build-require `perl(Unicode::Stringprep::Mapping)' (lib/Net/IDN/Nameprep.pm:16).
+BuildRequires:  perl(Unicode::Stringprep::Mapping)
Ok.

> FIX: Build-require `perl(Unicode::Stringprep::Prohibited)' (lib/Net/IDN/Nameprep.pm:17).
+BuildRequires:  perl(Unicode::Stringprep::Prohibited)
Ok.

> FIX: Remove executable bit from Changes file.
> FIX: Remove executable bit from Nameprep.pm.
> FIX: Remove executable bit from /usr/share/doc/perl-Net-IDN-Nameprep-1.101/Changes and /usr/share/perl5/vendor_perl/Net/IDN/Nameprep.pm.
+chmod -x Changes \
+         lib/Net/IDN/Nameprep.pm
Ok.

> FIX: Convert LICENSE and README to UTF-8.
+# Convert files to UTF-8
+for FILE in LICENSE README; do
+  iconv -f ISO_8859-1 -t UTF8 $FILE > $FILE.utf8
+  mv $FILE.utf8 $FILE
+done
Ok.

$ rpmlint perl-Net-IDN-Nameprep.spec ../SRPMS/perl-Net-IDN-Nameprep-1.101-2.fc19.src.rpm ../RPMS/noarch/perl-Net-IDN-Nameprep-1.101-2.fc19.noarch.rpm
perl-Net-IDN-Nameprep.src: W: spelling-error Summary(en_US) Stringprep -> String prep, String-prep, Stripping
perl-Net-IDN-Nameprep.src: W: spelling-error %description -l en_US stringprep -> string prep, string-prep, stripping
perl-Net-IDN-Nameprep.noarch: W: spelling-error Summary(en_US) Stringprep -> String prep, String-prep, Stripping
perl-Net-IDN-Nameprep.noarch: W: spelling-error %description -l en_US stringprep -> string prep, string-prep, stripping
2 packages and 1 specfiles checked; 0 errors, 4 warnings.
rpmlint is Ok.

$ rpm -q -lv -p ../RPMS/noarch/perl-Net-IDN-Nameprep-1.101-2.fc19.noarch.rpm 
drwxr-xr-x    2 root    root                        0 Jan 23 10:07 /usr/share/doc/perl-Net-IDN-Nameprep-1.101
-rw-r--r--    1 root    root                     1176 Dec  8  2011 /usr/share/doc/perl-Net-IDN-Nameprep-1.101/Changes
-rw-r--r--    1 root    root                    18410 Jan 23 10:07 /usr/share/doc/perl-Net-IDN-Nameprep-1.101/LICENSE
-rw-r--r--    1 root    root                      770 Jan 23 10:07 /usr/share/doc/perl-Net-IDN-Nameprep-1.101/README
-rw-r--r--    1 root    root                     2732 Jan 23 10:07 /usr/share/man/man3/Net::IDN::Nameprep.3pm.gz
drwxr-xr-x    2 root    root                        0 Jan 23 10:07 /usr/share/perl5/vendor_perl/Net
drwxr-xr-x    2 root    root                        0 Jan 23 10:07 /usr/share/perl5/vendor_perl/Net/IDN
-rw-r--r--    1 root    root                     3004 Jan 23 10:07 /usr/share/perl5/vendor_perl/Net/IDN/Nameprep.pm
File layout and permissions are Ok.

Package builds in F19 (http://koji.fedoraproject.org/koji/taskinfo?taskID=4896073). Ok.

Package is good except the unclarified license.
Resolution: Waiting on legal.
Comment 6 Petr Pisar 2013-01-23 04:26:10 EST
(In reply to comment #3)
> (In reply to comment #2)
> > FIX: Build-require `perl(Unicode::Stringprep::Mapping)'
> > (lib/Net/IDN/Nameprep.pm:16).
> > FIX: Build-require `perl(Unicode::Stringprep::Prohibited)'
> > (lib/Net/IDN/Nameprep.pm:17).
> 
> I added the requirements, but now I'm wondering: should they be versioned
> explicitly, like Unicode::Stringprep just above? (version >= 1.1 is required)
> 
I don't think so. Some upstream versions only `main' perl module from package, some upstream versions each module in one package independently. Some upstream keeps module's version in synchronization. I think the best approach is to stick on upstream specifications.
Comment 7 Tom "spot" Callaway 2013-02-04 02:45:28 EST
Can you patch out that unit test? The license on it is non-free.
Comment 8 Mathieu Bridon 2013-02-04 03:06:20 EST
(In reply to comment #7)
> Can you patch out that unit test? The license on it is non-free.

You mean drop it from the source tarball?

Or remove it during %prep?
Comment 9 Tom "spot" Callaway 2013-02-05 09:42:30 EST
In this case, I leave that up to you. Technically, there are no restrictions on redistribution in this non-free license, so you can delete it in %prep if you choose.

I would strongly recommend you point out this problem to upstream and try to get them to remove this test.
Comment 10 Petr Pisar 2013-02-05 10:33:03 EST
Interesting legal resolution. Approving this package.

Resolution: Package APPROVED.
Comment 11 Mathieu Bridon 2013-02-06 01:21:02 EST
(In reply to comment #9)
> In this case, I leave that up to you. Technically, there are no restrictions
> on redistribution in this non-free license, so you can delete it in %prep if
> you choose.

Ok, I drop it in %prep then, as it's much simpler than messing with the tarball.

For the record, here is the new package with this fixed:

Spec URL: http://bochecha.fedorapeople.org/packages/perl-Net-IDN-Nameprep.spec
SRPM URL: http://bochecha.fedorapeople.org/packages/perl-Net-IDN-Nameprep-1.101-3.fc19.src.rpm

That's what I'll import into Fedora, now that Petr approved the review request.

> I would strongly recommend you point out this problem to upstream and try to
> get them to remove this test.

Done:
    https://rt.cpan.org/Public/Bug/Display.html?id=83149Spec URL: 

-----

New Package SCM Request
=======================
Package Name: perl-Net-IDN-Nameprep
Short Description: Stringprep Profile for Internationalized Domain Names (RFC 3491)
Owners: bochecha
Branches: devel
InitialCC: perl-sig
Comment 12 Jon Ciesla 2013-02-06 08:34:15 EST
Git done (by process-git-requests).
Comment 13 Mathieu Bridon 2013-02-06 22:52:13 EST
Thanks for the Git process, Jon.

Package built in Rawhide, closing.

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