Bug 226160 - Merge Review: mpage
Merge Review: mpage
Status: CLOSED RAWHIDE
Product: Fedora
Classification: Fedora
Component: Package Review (Show other bugs)
rawhide
All Linux
medium Severity medium
: ---
: ---
Assigned To: Susi Lehtola
Fedora Package Reviews List
:
Depends On:
Blocks:
  Show dependency treegraph
 
Reported: 2007-01-31 14:43 EST by Nobody's working on this, feel free to take it
Modified: 2009-03-27 09:34 EDT (History)
3 users (show)

See Also:
Fixed In Version:
Doc Type: Bug Fix
Doc Text:
Story Points: ---
Clone Of:
Environment:
Last Closed: 2009-03-27 05:38:07 EDT
Type: ---
Regression: ---
Mount Type: ---
Documentation: ---
CRM:
Verified Versions:
Category: ---
oVirt Team: ---
RHEL 7.3 requirements from Atomic Host:
Cloudforms Team: ---
susi.lehtola: fedora‑review+


Attachments (Terms of Use)

  None (edit)
Description Nobody's working on this, feel free to take it 2007-01-31 14:43:22 EST
Fedora Merge Review: mpage

http://cvs.fedora.redhat.com/viewcvs/devel/mpage/
Initial Owner: mbacovsk@redhat.com
Comment 1 Susi Lehtola 2009-03-20 16:22:01 EDT
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 05:49:23 EDT
(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 06:22:20 EDT
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 07:11:11 EDT
(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 07:18:16 EDT
(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 08:08:49 EDT
(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 05:23:36 EDT
done
Comment 8 Susi Lehtola 2009-03-27 05:38:07 EDT
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 06:43:01 EDT
> 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 06:57:43 EDT
(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 07:07:54 EDT
(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 09:20:04 EDT
Oh, is the man page generated, too..? Very interesting.

In that case, no problem.
Comment 13 Michal Hlavinka 2009-03-27 09:34:49 EDT
(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

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