Bug 226160

Summary: Merge Review: mpage
Product: [Fedora] Fedora Reporter: Nobody's working on this, feel free to take it <nobody>
Component: Package ReviewAssignee: Susi Lehtola <susi.lehtola>
Status: CLOSED RAWHIDE QA Contact: Fedora Package Reviews List <fedora-package-review>
Severity: medium Docs Contact:
Priority: medium    
Version: rawhideCC: mbacovsk, mhlavink, susi.lehtola
Target Milestone: ---Flags: susi.lehtola: fedora-review+
Target Release: ---   
Hardware: All   
OS: Linux   
Whiteboard:
Fixed In Version: Doc Type: Bug Fix
Doc Text:
Story Points: ---
Clone Of: Environment:
Last Closed: 2009-03-27 09:38:07 UTC Type: ---
Regression: --- Mount Type: ---
Documentation: --- CRM:
Verified Versions: Category: ---
oVirt Team: --- RHEL 7.3 requirements from Atomic Host:
Cloudforms Team: --- Target Upstream Version:
Embargoed:

Description Nobody's working on this, feel free to take it 2007-01-31 19:43:22 UTC
Fedora Merge Review: mpage

http://cvs.fedora.redhat.com/viewcvs/devel/mpage/
Initial Owner: mbacovsk

Comment 1 Susi Lehtola 2009-03-20 20:22:01 UTC
rpmlint output:
mpage.src:9: W: hardcoded-path-in-buildroot-tag /var/tmp/mpage-root
mpage.src: W: summary-ended-with-dot A tool for printing multiple pages of text on each printed page.
mpage.src: W: no-url-tag
mpage.x86_64: W: file-not-utf8 /usr/share/doc/mpage-2.5.6/CHANGES
mpage.x86_64: W: summary-ended-with-dot A tool for printing multiple pages of text on each printed page.
mpage.x86_64: W: no-url-tag
mpage-debuginfo.x86_64: W: no-url-tag
3 packages and 0 specfiles checked; 0 errors, 7 warnings.

- Fix the rpmlint warnings; url should be http://www.mesa.nl/pub/mpage/ (AFAIK no project home page exists).

- Some problem with the license? Program source code seems to be licensed under the following license:
 *     Permission is granted to anyone to make or distribute verbatim
 *     copies of this document as received, in any medium, provided
 *     that this copyright notice is preserved, and that the
 *     distributor grants the recipient permission for further
 *     redistribution as permitted by this notice.
On the other hand, the file gpl.in states that mpage is licensed under GPLv2+. Please contact upstream to clarify license.

- Include FAQ, COPYING, COPYING.LESSER to copyright.

Comment 2 Michal Hlavinka 2009-03-24 09:49:23 UTC
(In reply to comment #1)
> rpmlint output:
> mpage.src:9: W: hardcoded-path-in-buildroot-tag /var/tmp/mpage-root

fixed

> mpage.src: W: no-url-tag

fixed

> mpage.src: W: summary-ended-with-dot A tool for printing multiple pages of text
> on each printed page.

fixed

> mpage.x86_64: W: file-not-utf8 /usr/share/doc/mpage-2.5.6/CHANGES

fixed

> - Some problem with the license? Program source code seems to be licensed under
> the following license:
>  *     Permission is granted to anyone to make or distribute verbatim
>  *     copies of this document as received, in any medium, provided
>  *     that this copyright notice is preserved, and that the
>  *     distributor grants the recipient permission for further
>  *     redistribution as permitted by this notice.
> On the other hand, the file gpl.in states that mpage is licensed under GPLv2+.
> Please contact upstream to clarify license.

I've asked upstream, waiting for answer.

> - Include FAQ, COPYING, COPYING.LESSER to copyright.  

fixed

Comment 3 Michal Hlavinka 2009-03-26 10:22:20 UTC
from the emails:

license is GPLv2+

when new version is released files will contain notes about the license in the headers and other documents.

Comment 4 Susi Lehtola 2009-03-26 11:11:11 UTC
(In reply to comment #3)
> from the emails:
> 
> license is GPLv2+
> 
> when new version is released files will contain notes about the license in the
> headers and other documents.  

Okay, when is it coming out?

Are you going to delay updating the spec until the new version is released?

Comment 5 Michal Hlavinka 2009-03-26 11:18:16 UTC
(In reply to comment #4)
> Okay, when is it coming out?
> 

can't tell for sure... I don't know if new version will be released only because of gpl headers or when there is something new in the code

> Are you going to delay updating the spec until the new version is released?  

no, I've fixed other problems and based on the emails the license in spec is correct, so I think there is no need to wait for new version

Comment 6 Susi Lehtola 2009-03-26 12:08:49 UTC
(In reply to comment #5)
> > Are you going to delay updating the spec until the new version is released?  
> 
> no, I've fixed other problems and based on the emails the license in spec is
> correct, so I think there is no need to wait for new version  

Okay; the CVS spec changelog just didn't have any mention of the fixes made.

- Character set conversion is unsafe and loses time stamp. Change to:

iconv -f iso-8859-2 -t utf-8 CHANGES > CHANGES.tmp && \
touch -r CHANGES CHANGES.tmp && mv CHANGES.tmp CHANGES


Fix this and build package in rawhide, then I can review the package (I can get the srpm from koji).

Comment 7 Michal Hlavinka 2009-03-27 09:23:36 UTC
done

Comment 8 Susi Lehtola 2009-03-27 09:38:07 UTC
rpmlint output: clean.

MUST: The spec file for the package is legible and macros are used consistently. OK
MUST: The package must be named according to the Package Naming Guidelines. OK
MUST: The spec file name must match the base package %{name}. OK
MUST: The package must be licensed with a Fedora approved license and meet the  Licensing Guidelines. OK
MUST: The License field in the package spec file must match the actual license. OK
MUST: The sources used to build the package must match the upstream source, as provided in the spec URL. OK
MUST: The package MUST successfully compile and build into binary rpms. OK
MUST: The spec file MUST handle locales properly. OK

MUST: Optflags are used and time stamps preserved. 
 - Optflags OK, but 
SHOULD: time stamps are not preserved. You need to patch the makefile to use 'cp -p' instead of 'cp'.

MUST: Packages containing shared library files must call ldconfig. OK
MUST: A package must own all directories that it creates or require the package that owns the directory. OK
MUST: Files only listed once in %files listings. OK
MUST: Permissions on files must be set properly. OK
MUST: Clean section exists. OK
MUST: Large documentation files must go in a -doc subpackage. OK
MUST: Items in %doc do not affect runtime of application. OK
MUST: Header files must be in a -devel package. OK
MUST: Static libraries must be in a -static package. OK
MUST: Packages containing pkgconfig(.pc) files must 'Requires: pkgconfig'. OK
MUST: If a package contains library files with a suffix then library files ending in .so must go in a -devel package. OK
MUST: In the vast majority of cases, devel packages must require the base package using a fully versioned dependency. OK
MUST: Packages does not contain any .la libtool archives. OK
MUST: Desktop files are installed properly. OK
MUST: No file conflicts with other packages and no general names. OK
MUST: Buildroot cleaned before install. OK
SHOULD: %{?dist} tag is used in release. OK
SHOULD: If the package does not include license text(s) as separate files from upstream, the packager should query upstream to include it. OK
SHOULD: The package builds in mock. OK


No blockers; the package is

APPROVED.

Fix the time stamp problem in CVS.

Comment 9 Michal Hlavinka 2009-03-27 10:43:01 UTC
> SHOULD: time stamps are not preserved. You need to patch the makefile 
> ...
> Fix the time stamp problem in CVS.  

done

Comment 10 Susi Lehtola 2009-03-27 10:57:43 UTC
(In reply to comment #9)
> > SHOULD: time stamps are not preserved. You need to patch the makefile 
> > ...
> > Fix the time stamp problem in CVS.  
> 
> done  

Hmm, what did you do, exactly?

According to the build log, the install phase still doesn't preserve time stamps on the man page: http://kojipkgs.fedoraproject.org/packages/mpage/2.5.6/6.fc11/data/logs/i586/build.log

cp mpage /builddir/build/BUILDROOT/mpage-2.5.6-6.fc11.i386//usr/bin
cp mpage.1 /builddir/build/BUILDROOT/mpage-2.5.6-6.fc11.i386//usr/share/man/man1
install -p -m 644 Encodings/* /builddir/build/BUILDROOT/mpage-2.5.6-6.fc11.i386//usr/share/mpage

Oh, and you have CopyRight listed in %doc two times, now. Fix this.

Comment 11 Michal Hlavinka 2009-03-27 11:07:54 UTC
(In reply to comment #10)
> (In reply to comment #9)
> > > SHOULD: time stamps are not preserved. You need to patch the makefile 
> > > ...
> > > Fix the time stamp problem in CVS.  
> > 
> > done  
> 
> Hmm, what did you do, exactly?

cp + chmod Encodings/*
replaced by install -p -m 644 Encodings/*

mpage and mpage.1 are product of the build process and thus there is no time stamp to preserve

> Oh, and you have CopyRight listed in %doc two times, now. Fix this.  

will fix if no other objections - I don't want to DoS cvs & koji :)

Comment 12 Susi Lehtola 2009-03-27 13:20:04 UTC
Oh, is the man page generated, too..? Very interesting.

In that case, no problem.

Comment 13 Michal Hlavinka 2009-03-27 13:34:49 UTC
(In reply to comment #12)
> Oh, is the man page generated, too..? Very interesting.
> 
> In that case, no problem.  

yes, usually man pages are generated: something.X from something.X.in