Bug 714511

Source file is original. Ok.

TODO: Remove the `a full range of features for ' substring from summary. This does not come from upstream and I'm not sure this is or will be true forever (despite your additional patches that should be reported to RT). 

License verified from lib/Net/Google/PicasaWeb.pm and LICENSE. Ok.

URL and Source URL Ok.

Patches reviewed. Ok.
TODO: Submit the patches to upstream.

TODO: Remove the BuildRoot definition and cleaning (rm -rf $RPM_BUILD_ROOT) as this default in all Fedora branches.

No XS code, noarch BuildArch is Ok.

FIX: Remove `BuildRequires:  perl(Test::Mock::LWP)'. The Test::Mock::LWP dependency has been removed from code (Changes:26).
TODO: Ask upstream to fix Makefile.PL/META.yml.

FIX: BuildRequire perl(Carp) (lib/Net/Google/PicasaWeb/Media.pm:11, http://search.cpan.org/dist/Carp/).

FIX: BuildRequier perl(HTTP::Request::Common) (lib/Net/Google/PicasaWeb.pm:10, http://search.cpan.org/~gaas/HTTP-Message/)

FIX: BuildRequire perl(HTTP::Message) and perl(HTTP::Request) introduced by your patches.

TODO: Move the `%{?!_with_network_tests: rm t/offline.t t/online.t}' adjustment from %check phase to %prep phase as it modifies sources.
TODO: Use standardized %bcond_with and %with macros instead of your %_with_network_tests (http://www.rpm.org/wiki/PackagerDocs/ConditionalBuilds).

TODO: Sort BuildRequires and %doc files lexicographically to ease future maintenance.

TODO: Omit `-b' %patch option, then you do not need to remove the back-up files produced by patch.

TODO: Remove %defattr macro from %files section as it is default in all Fedora branches.

TODO: Rename %_bindir scripts (and corresponding manual pages) and package them. Ask upstream for the same change.


$ rpmlint perl-Net-Google-PicasaWeb.spec ../SRPMS/perl-Net-Google-PicasaWeb-0.11-2.fc17.src.rpm ../RPMS/noarch/perl-Net-Google-PicasaWeb-0.11-2.fc17.noarch.rpm 
perl-Net-Google-PicasaWeb.spec:47: W: mixed-use-of-spaces-and-tabs (spaces: line 1, tab: line 47)
perl-Net-Google-PicasaWeb.src:47: W: mixed-use-of-spaces-and-tabs (spaces: line 1, tab: line 47)
perl-Net-Google-PicasaWeb.noarch: E: incorrect-fsf-address /usr/share/doc/perl-Net-Google-PicasaWeb-0.11/LICENSE
2 packages and 1 specfiles checked; 1 errors, 2 warnings.

TODO: Unify white spaces in spec file (you can use expand(1)).
TODO: Report out-dated FSF address to upstream.
rpmlint output is Ok.


$ rpm -q -lv -p ../RPMS/noarch/perl-Net-Google-PicasaWeb-0.11-2.fc17.noarch.rpm
drwxr-xr-x    2 root    root                        0 Sep 30 13:55 /usr/share/doc/perl-Net-Google-PicasaWeb-0.11
-rw-r--r--    1 root    root                       82 May 30 16:35 /usr/share/doc/perl-Net-Google-PicasaWeb-0.11/AUTHORS
-rw-r--r--    1 root    root                     4565 May 30 16:35 /usr/share/doc/perl-Net-Google-PicasaWeb-0.11/Changes
-rw-r--r--    1 root    root                    18291 May 30 16:35 /usr/share/doc/perl-Net-Google-PicasaWeb-0.11/LICENSE
-rw-r--r--    1 root    root                      313 May 30 16:35 /usr/share/doc/perl-Net-Google-PicasaWeb-0.11/README
-rw-r--r--    1 root    root                     5946 Sep 30 13:55 /usr/share/man/man3/Net::Google::PicasaWeb.3pm.gz
-rw-r--r--    1 root    root                     2756 Sep 30 13:55 /usr/share/man/man3/Net::Google::PicasaWeb::Album.3pm.gz
-rw-r--r--    1 root    root                     2692 Sep 30 13:55 /usr/share/man/man3/Net::Google::PicasaWeb::Base.3pm.gz
-rw-r--r--    1 root    root                     2322 Sep 30 13:55 /usr/share/man/man3/Net::Google::PicasaWeb::Comment.3pm.gz
-rw-r--r--    1 root    root                     2455 Sep 30 13:55 /usr/share/man/man3/Net::Google::PicasaWeb::Feed.3pm.gz
-rw-r--r--    1 root    root                     3287 Sep 30 13:55 /usr/share/man/man3/Net::Google::PicasaWeb::Media.3pm.gz
-rw-r--r--    1 root    root                     2646 Sep 30 13:55 /usr/share/man/man3/Net::Google::PicasaWeb::MediaEntry.3pm.gz
-rw-r--r--    1 root    root                     2075 Sep 30 13:55 /usr/share/man/man3/Net::Google::PicasaWeb::MediaFeed.3pm.gz
drwxr-xr-x    2 root    root                        0 Sep 30 13:55 /usr/share/perl5/vendor_perl/Net
drwxr-xr-x    2 root    root                        0 Sep 30 13:55 /usr/share/perl5/vendor_perl/Net/Google
drwxr-xr-x    2 root    root                        0 Sep 30 13:55 /usr/share/perl5/vendor_perl/Net/Google/PicasaWeb
-rw-r--r--    1 root    root                    22829 Sep 30 13:55 /usr/share/perl5/vendor_perl/Net/Google/PicasaWeb.pm
-rw-r--r--    1 root    root                     4050 May 30 16:35 /usr/share/perl5/vendor_perl/Net/Google/PicasaWeb/Album.pm
-rw-r--r--    1 root    root                     2480 May 30 16:35 /usr/share/perl5/vendor_perl/Net/Google/PicasaWeb/Base.pm
-rw-r--r--    1 root    root                     1883 May 30 16:35 /usr/share/perl5/vendor_perl/Net/Google/PicasaWeb/Comment.pm
-rw-r--r--    1 root    root                     3715 May 30 16:35 /usr/share/perl5/vendor_perl/Net/Google/PicasaWeb/Feed.pm
-rw-r--r--    1 root    root                     8008 May 30 16:35 /usr/share/perl5/vendor_perl/Net/Google/PicasaWeb/Media.pm
-rw-r--r--    1 root    root                     3813 May 30 16:35 /usr/share/perl5/vendor_perl/Net/Google/PicasaWeb/MediaEntry.pm
-rw-r--r--    1 root    root                     1424 May 30 16:35 /usr/share/perl5/vendor_perl/Net/Google/PicasaWeb/MediaFeed.pm

File permissions and layout are Ok.

$ rpm -q --requires -p ../RPMS/noarch/perl-Net-Google-PicasaWeb-0.11-2.fc17.noarch.rpm |sort -k2 |uniq -c
      1 perl(Carp)  
      1 perl(HTTP::Message)  
      1 perl(HTTP::Request)  
      1 perl(HTTP::Request::Common)  
      1 perl(LWP::UserAgent)  
      1 perl(:MODULE_COMPAT_5.14.1)  
      1 perl(Moose)  
      1 perl(Net::Google::AuthSub)  
      1 perl(Net::Google::PicasaWeb::Album)  
      1 perl(Net::Google::PicasaWeb::Comment)  
      1 perl(Net::Google::PicasaWeb::Media)  
      1 perl(Net::Google::PicasaWeb::MediaEntry)  
      1 perl(URI)  
      1 perl(XML::Twig)  
      1 rpmlib(CompressedFileNames) <= 3.0.4-1
      1 rpmlib(PayloadFilesHavePrefix) <= 4.0-1
      1 rpmlib(FileDigests) <= 4.6.0-1
      1 rpmlib(PayloadIsXz) <= 5.2-1
FIX: Require perl(MooseX::Role::Matcher) explicitly as rpmbuild does not understand Moose `with' now (lib/Net/Google/PicasaWeb/Base.pm:9).

$ rpm -q --provides -p ../RPMS/noarch/perl-Net-Google-PicasaWeb-0.11-2.fc17.noarch.rpm |sort -k2 |uniq -c
      1 perl(Net::Google::PicasaWeb) = 0.11
      1 perl(Net::Google::PicasaWeb::Album) = 0.11
      1 perl(Net::Google::PicasaWeb::Base) = 0.11
      1 perl(Net::Google::PicasaWeb::Comment) = 0.11
      1 perl(Net::Google::PicasaWeb::Feed) = 0.11
      1 perl(Net::Google::PicasaWeb::Media) = 0.11
      1 perl(Net::Google::PicasaWeb::Media::Content) = 0.11
      1 perl(Net::Google::PicasaWeb::MediaEntry) = 0.11
      1 perl(Net::Google::PicasaWeb::MediaFeed) = 0.11
      1 perl(Net::Google::PicasaWeb::Media::Thumbnail) = 0.11
      1 perl(Net::Google::PicasaWeb::Tag) = 0.11
      1 perl-Net-Google-PicasaWeb = 0.11-2.fc17
Binary Provides are Ok.

$ resolvedeps rawhide ../RPMS/noarch/perl-Net-Google-PicasaWeb-0.11-2.fc17.noarch.rpm 
Binary dependencies resolvable. Ok.

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

Otherwise package is in line with Fedora and Perl packaging guidelines.


Please correct all `FIX' prefixed issues, consider fixing `TODO' items and provide new package.

Resolution: Package NOT approved.

I'm packaging the Test::Able* (bug #742536), so you will able to enable offline tests soon.

The Test::Able* modules has been in Fedora since 2011-10-03.

Thank you for update. I'll publish new version soon.

Updated version

Spec URL: http://andriy.fedorapeople.org/perl-Net-Google-PicasaWeb.spec
SRPM URL: http://andriy.fedorapeople.org/perl-Net-Google-PicasaWeb-0.11-3.sh16.src.rpm

Spec file changes:
--- perl-Net-Google-PicasaWeb.spec.old	2011-06-19 21:14:14.000000000 +0200
+++ perl-Net-Google-PicasaWeb.spec	2012-07-15 16:32:40.000000000 +0200
@@ -1,32 +1,48 @@
 Name:           perl-Net-Google-PicasaWeb
 Version:        0.11
-Release:        2%{?dist}
-Summary:        Implements a full range of features for the Google Picasa Web API
+Release:        3%{?dist}
+Summary:        Implements the Google Picasa Web API
 
 Group:          Development/Libraries
 License:        GPL+ or Artistic
 URL:            http://search.cpan.org/dist/Net-Google-PicasaWeb/
 Source0:        http://search.cpan.org/CPAN/authors/id/H/HA/HANENKAMP/Net-Google-PicasaWeb-%{version}.tar.gz
-Patch1:         0001-Pass-Content-Type-to-request.patch
-Patch2:         0002-Implement-add_album-feature.patch
-Patch3:         0003-First-implementation-of-add_media_entry.patch
-BuildRoot:      %{_tmppath}/%{name}-%{version}-%{release}-root-%(%{__id_u} -n)
+
+# This patch is derived from an post-release upstream commit 996486e
+Patch1:         Net-Google-PicasaWeb-add-lwp-protocol-https.patch
+# Upstream is notified about following patches via
+# https://rt.cpan.org/Public/Bug/Display.html?id=46702 and
+# https://github.com/zostay/net-google-picasaweb/pull/1
+Patch2:         Net-Google-PicasaWeb-pass-Content-Type-to-request.patch
+Patch3:         Net-Google-PicasaWeb-add_album-feature.patch
+Patch4:         Net-Google-PicasaWeb-add_media_entry.patch
+# Upstream is notified via https://rt.cpan.org/Public/Bug/Display.html?id=78384
+Patch5:         Net-Google-PicasaWeb-remove-Test-Mock-LWP.patch
+# This issue is published as
+# https://rt.cpan.org/Public/Bug/Display.html?id=78385
+# The license has been updated accordingly to
+# http://www.gnu.org/licenses/gpl-1.0.txt
+Patch6:         Net-Google-PicasaWeb-GPL.patch
 
 BuildArch:      noarch
-BuildRequires:  perl(Test::More)
-BuildRequires:  perl(Test::Mock::LWP)
+BuildRequires:  perl(Carp)
+BuildRequires:  perl(ExtUtils::MakeMaker)
+BuildRequires:  perl(HTTP::Message)
+BuildRequires:  perl(HTTP::Request)
+BuildRequires:  perl(HTTP::Request::Common)
+Buildrequires:  perl(IO::Prompt)
 BuildRequires:  perl(LWP::UserAgent)
 BuildRequires:  perl(Moose)
+Buildrequires:  perl(MooseX::Role::Matcher)
 BuildRequires:  perl(Net::Google::AuthSub)
+Buildrequires:  perl(Test::Able) >= 0.09
+Buildrequires:  perl(Test::Able::Runner) >= 1.000
+BuildRequires:  perl(Test::More)
 BuildRequires:  perl(URI)
 BuildRequires:  perl(XML::Twig) >= 3.30
-Buildrequires:  perl(MooseX::Role::Matcher)
-# Have no such modules in Fedora yet
-#Buildrequires:  perl(Test::Able) >= 0.09
-#Buildrequires:  perl(Test::Able::Runner) >= 1.000
-Buildrequires:  perl(IO::Prompt)
-BuildRequires:  perl(ExtUtils::MakeMaker)
+
 Requires:  perl(:MODULE_COMPAT_%(eval "`%{__perl} -V:version`"; echo $version))
+Requires:  perl(MooseX::Role::Matcher)
 
 
 %description
@@ -38,15 +54,20 @@
 
 %prep
 %setup -q -n Net-Google-PicasaWeb-%{version}
-%patch1 -p1 -b .pass_content_type_to_req
-%patch2 -p1 -b .add_album
-%patch3 -p1 -b .add_media_entry
-
-# Delete original files
-for e in pass_content_type_to_req add_album add_media_entry; do
-	find $RPM_BUILD_DIR -type f -name "*.$e" -delete
+%patch1 -p1
+%patch2 -p1
+%patch3 -p1
+%patch4 -p1
+%patch5 -p1
+%patch6 -p1
+
+# Fix the script names to not conflict with the original Picasa application
+# Upstream is notified via https://rt.cpan.org/Public/Bug/Display.html?id=78386
+find bin -type f -name "picasa*" | while read name; do
+	newname=$(echo $name | sed -e 's,picasa,picasapl,')
+	mv -f $name $newname
 done
-
+sed -i -e 's,bin/picasa,bin/picasapl,' Makefile.PL MANIFEST bin/picasapl*
 
 %build
 %{__perl} Makefile.PL INSTALLDIRS=vendor
@@ -54,7 +75,6 @@
 
 
 %install
-rm -rf $RPM_BUILD_ROOT
 make pure_install PERL_INSTALL_ROOT=$RPM_BUILD_ROOT
 find $RPM_BUILD_ROOT -type f -name .packlist -exec rm -f {} ';'
 find $RPM_BUILD_ROOT -depth -type d -exec rmdir {} 2>/dev/null ';'
@@ -62,25 +82,21 @@
 
 
 %check
-%{?!_with_network_tests: rm t/offline.t t/online.t}
 make test
 
 
-%clean
-rm -rf $RPM_BUILD_ROOT
-
-
 %files
-%defattr(-,root,root,-)
-%doc README Changes LICENSE AUTHORS
+%doc AUTHORS Changes LICENSE README
 %{perl_vendorlib}/*
 %{_mandir}/man3/*.3*
-# Don't confuse user with original picasa binary name
-%exclude %{_mandir}/man1/*.1*
-%exclude %{_bindir}/*
+%{_bindir}/*
+%{_mandir}/man1/*.1*
 
 
 %changelog
+* Sun Jul 15 2012 Andy Shevchenko <andy.shevchenko> - 0.11-3
+- fix spec file accodingly to the reviewer's comments (#714511)
+
 * Sun Jun 19 2011 Andy Shevchenko <andy.shevchenko> - 0.11-2
 - apply (modified, due to usage of Dist::Zilla) patches to upload photos
 - run the tests which require Test::Able module conditionally


> TODO: Remove the `a full range of features for ' substring from summary. This
> does not come from upstream and I'm not sure this is or will be true forever
> (despite your additional patches that should be reported to RT).
-Release:        2%{?dist}
-Summary:        Implements a full range of features for the Google Picasa Web API
+Release:        3%{?dist}
+Summary:        Implements the Google Picasa Web API
Ok.

> TODO: Submit the patches to upstream.
OK.

> TODO: Remove the BuildRoot definition and cleaning (rm -rf $RPM_BUILD_ROOT) as
> this default in all Fedora branches.
Ok.

> FIX: Remove `BuildRequires:  perl(Test::Mock::LWP)'. The Test::Mock::LWP
> dependency has been removed from code (Changes:26).
-BuildRequires:  perl(Test::Mock::LWP)
Ok.

> TODO: Ask upstream to fix Makefile.PL/META.yml.
Ok.

> FIX: BuildRequire perl(Carp) (lib/Net/Google/PicasaWeb/Media.pm:11,
> http://search.cpan.org/dist/Carp/).
+BuildRequires:  perl(Carp)
Ok.

> FIX: BuildRequier perl(HTTP::Request::Common) (lib/Net/Google/PicasaWeb.pm:10,
> http://search.cpan.org/~gaas/HTTP-Message/)
+BuildRequires:  perl(HTTP::Request::Common)
Ok.

> FIX: BuildRequire perl(HTTP::Message) and perl(HTTP::Request) introduced by
> your patches.
+BuildRequires:  perl(HTTP::Message)
+BuildRequires:  perl(HTTP::Request)
Ok.

> TODO: Move the `%{?!_with_network_tests: rm t/offline.t t/online.t}' adjustment
> from %check phase to %prep phase as it modifies sources.
> TODO: Use standardized %bcond_with and %with macros instead of your
> %_with_network_tests (http://www.rpm.org/wiki/PackagerDocs/ConditionalBuilds).
 %check
-%{?!_with_network_tests: rm t/offline.t t/online.t}
Ok.

> TODO: Sort BuildRequires and %doc files lexicographically to ease future
> maintenance.
-%doc README Changes LICENSE AUTHORS
+%doc AUTHORS Changes LICENSE README
Ok.

> TODO: Omit `-b' %patch option, then you do not need to remove the back-up files
> produced by patch.
-%patch1 -p1 -b .pass_content_type_to_req
-%patch2 -p1 -b .add_album
-%patch3 -p1 -b .add_media_entry
-
-# Delete original files
-for e in pass_content_type_to_req add_album add_media_entry; do
-	find $RPM_BUILD_DIR -type f -name "*.$e" -delete
+%patch1 -p1
+%patch2 -p1
+%patch3 -p1
+%patch4 -p1
+%patch5 -p1
+%patch6 -p1
Ok.

> TODO: Remove %defattr macro from %files section as it is default in all Fedora
> branches.
 %files
-%defattr(-,root,root,-)
Ok.

> TODO: Rename %_bindir scripts (and corresponding manual pages) and package
> them. Ask upstream for the same change.
+# Fix the script names to not conflict with the original Picasa application
+# Upstream is notified via https://rt.cpan.org/Public/Bug/Display.html?id=78386
+find bin -type f -name "picasa*" | while read name; do
+	newname=$(echo $name | sed -e 's,picasa,picasapl,')
+	mv -f $name $newname

-# Don't confuse user with original picasa binary name
-%exclude %{_mandir}/man1/*.1*
-%exclude %{_bindir}/*
+%{_bindir}/*
+%{_mandir}/man1/*.1*
Ok.


FIX: The tests do no pass because you run offline tests now (you removed the _with_network_tests instead of moving it to %prep) which obviously require Test::MockObject:

Executing(%check): /bin/sh -e /var/tmp/rpm-tmp.MJIdUj
+ umask 022
+ cd /home/test/rpmbuild/BUILD
+ cd Net-Google-PicasaWeb-0.11
+ unset DISPLAY
+ make test
PERL_DL_NONLAZY=1 /usr/bin/perl "-MExtUtils::Command::MM" "-e" "test_harness(0, 'blib/lib', 'blib/arch')" t/*.t
t/00-load.t ............... 1/1 # Testing Net::Google::PicasaWeb 0.11, Perl 5.016000, /usr/bin/perl
t/00-load.t ............... ok
t/offline.t ............... Can't locate Test/MockObject.pm in @INC (@INC contains: t/offline /home/test/rpmbuild/BUILD/Net-Google-PicasaWeb-0.11/blib/lib /home/test/rpmbuild/BUILD/Net-Google-PicasaWeb-0.11/blib/arch /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 t/offline/Net/Google/PicasaWeb/Test/Role/Offline.pm line 4.

Any progress?

No response from submitter for more than two years. I mark this review as dead.