Bug 736861

Please inform upstream on the wrong address.

Use the name macro in Source0 and the files section.

BuildRequires and Requires are separated by spaces, not commas. Even better: Put each on a separate line.

Drop the version constraint for PyQt4. All possible build targets have a version new enough. The version restriction for Mercurial should probably go to the BRs as well, so the package won't build.

(In reply to comment #1)

Thanks for the quick response.

> Please inform upstream on the wrong address.

https://www.logilab.org/ticket/75295

> Use the name macro in Source0 and the files section.

I don't fully agree with that. I used explicit "hgview" where it didn't refer to the package name or upstream tar name. I don't think this increased use of %{name} increases the readability or flexibility of the spec. But ok ...

> BuildRequires and Requires are separated by spaces, not commas. Even better:
> Put each on a separate line.

Yes, that is another way of doing it, but I don't see any requirement/preference for that in the Packaging Guidelines.

FWIW http://fedoraproject.org/wiki/How_to_create_an_RPM_package and http://docs.fedoraproject.org/en-US/Fedora_Draft_Documentation/0.1/html/RPM_Guide/ch-specfile-syntax.html describe it as a comma-separated list.

It would be great of the guidelines could help making it more consistent.

> Drop the version constraint for PyQt4. All possible build targets have a
> version new enough.

Ok.

> The version restriction for Mercurial should probably go to
> the BRs as well, so the package won't build.

This build time dependency is bogus and isn't really used, so I assume it would work with all future versions of Mercurial. https://www.logilab.org/ticket/75296


Spec URL: http://kiilerix.fedorapeople.org/hgview.spec
SRPM URL: http://kiilerix.fedorapeople.org/hgview-1.3.0-2.fc15.src.rpm

Spec URL: http://kiilerix.fedorapeople.org/hgview.spec
SRPM URL: http://kiilerix.fedorapeople.org/hgview-1.4.0-1.fc15.src.rpm

(In reply to comment #2)

> > Use the name macro in Source0 and the files section.
> 
> I don't fully agree with that. I used explicit "hgview" where it didn't refer
> to the package name or upstream tar name. I don't think this increased use of
> %{name} increases the readability or flexibility of the spec. But ok ...

It depends, I think. The basic rule is to call the package like the tarball. In that case, it fits well. If you're going to rename the package for a different reason, it doesn't help.

> > BuildRequires and Requires are separated by spaces, not commas. Even better:
> > Put each on a separate line.
> 
> Yes, that is another way of doing it, but I don't see any
> requirement/preference for that in the Packaging Guidelines.
> 
> FWIW http://fedoraproject.org/wiki/How_to_create_an_RPM_package and
> http://docs.fedoraproject.org/en-US/Fedora_Draft_Documentation/0.1/html/RPM_Guide/ch-specfile-syntax.html
> describe it as a comma-separated list.
> 
> It would be great of the guidelines could help making it more consistent.
>

You're right. The reason why I suggested putting them on separate lines is, you can spot duplicates easily. I saw that happen a couple of times, when the list of BRs was long. They're also easier to comment out.

(In reply to comment #4)
> You're right. The reason why I suggested putting them on separate lines is ...

Please try to convince FPC to clarify the packaging guideline - then I would be happy to comply.

There is some typo or grammar mistake in the first line of the description:

"hgview is a simple tool aiming at visually navigate in a Mercurial repository" -- "navigating" maybe?

You should only set python_sitelib for systems that don't define it. Please see http://fedoraproject.org/wiki/Packaging:Python#Macros

The rm in the install section is not necessary. 

Qscintilla-python already requires PyQt4, so you don't have to state that explicitly.

README: "The Text interface depends on urwid, pygments and pyinotify" -- Did you leave that out on purpose?

If you find the time, please correct "intarfece" in the README file and tell upstream. You probably shouldn't ship that file at all, as it gives instructions on how to run it from a check-out, which is not what the package is about.

(In reply to comment #6)

Thanks for the comments. Most of it has just been done. Only a couple of comments:

The description (based on upstreams description) has been changed a lot, so it should perhaps be reviewed again.

> Qscintilla-python already requires PyQt4, so you don't have to state that
> explicitly.

I would like to keep that. It is explicitly stated as a dependency by upstream, and I like to have the important Qt4 dependency explicit.

> README: "The Text interface depends on urwid, pygments and pyinotify" -- Did
> you leave that out on purpose?

I had not updated the package for this new feature. Done now. It is a bit weird that the package both depends on urwid and qt4, but I don't see any good way to split it up. I have however tried not to mix these two parts too much.

Spec URL: http://kiilerix.fedorapeople.org/hgview.spec
SRPM URL: http://kiilerix.fedorapeople.org/hgview-1.4.0-2.fc15.src.rpm

Spec URL: http://kiilerix.fedorapeople.org/hgview.spec
SRPM URL: http://kiilerix.fedorapeople.org/hgview-1.5.0-1.fc16.src.rpm

* Thu Feb 16 2012 Mads Kiilerich <mads> - 1.5.0-1
- hgview-1.5.0
- make qt and curses support optional

Spec URL: http://kiilerix.fedorapeople.org/hgview.spec
SRPM URL: http://kiilerix.fedorapeople.org/hgview-1.7.0-1.fc17.src.rpm

* Sat Nov 24 2012 Mads Kiilerich <mads> - 1.7.0-1
- hgview-1.7.0

Spec URL: http://kiilerix.fedorapeople.org/hgview.spec
SRPM URL: http://kiilerix.fedorapeople.org/hgview-1.7.1-1.fc17.src.rpm

http://koji.fedoraproject.org/koji/taskinfo?taskID=5102288

* Sat Mar 09 2013 Mads Kiilerich <mads> - 1.7.1-1
- hgview-1.7.1

What about generating the desktop files as source1/2 before building? I seldom see the EOF script in spec now in Fedora.

Yes, that would be another way to do it. I think it would be less readable without being more guideline compliant.

For the record, I agree; there's nothing particularly wrong with catting those simple files out from the spec, and as a bonus you get to use macro substitutions if you want, which you often do.

Unfortunately I know pretty much nothing of mercurial (can barely handle git and don't see the need for yet another VCS of that type) so I don't have any real way to test this but I sure would like to see this finally get reviewed after something like 20 months, so I guess I'll try to take a look.

However, while I can build this just fine, I can't install it because rawhide has mercurial 2.6 and this explicitly requires mercurial < 2.6.  What to do?

(In reply to Jason Tibbitts from comment #13)
> Unfortunately I know pretty much nothing of mercurial (can barely handle git
> and don't see the need for yet another VCS of that type)

Mercurial and git were created simultaneously and independently. For me git is the yet another VCS that I would rather avoid using. YMMV.

> so I don't have any real way to test this 

You can get a repo to browse with
hg clone https://bitbucket.org/logilab/hgview

> but I sure would like to see this finally get reviewed
> after something like 20 months, so I guess I'll try to take a look.

Thanks.

> However, while I can build this just fine, I can't install it because
> rawhide has mercurial 2.6 and this explicitly requires mercurial < 2.6. 
> What to do?

That was a conservative estimate. I have updated the spec with more relaxed requirements.

Spec URL: http://kiilerix.fedorapeople.org/hgview.spec
SRPM URL: http://kiilerix.fedorapeople.org/hgview-1.7.1-2.fc18.src.rpm
http://koji.fedoraproject.org/koji/taskinfo?taskID=5471697

duplicate files packaged: `fdupes -r ${_reviewdir}/rpms-unpacked/`

./hgview-curses-1.7.1-2.fc20.noarch.rpm/etc/mercurial/hgrc.d/hgview.rc
./hgview-1.7.1-2.fc20.noarch.rpm/etc/mercurial/hgrc.d/hgview.rc

./hgview-curses-1.7.1-2.fc20.noarch.rpm/usr/share/man/man1/hgview.1.gz
./hgview-1.7.1-2.fc20.noarch.rpm/usr/share/man/man1/hgview.1.gz

./hgview-curses-1.7.1-2.fc20.noarch.rpm/usr/lib/python2.7/site-packages/hgext/hgview.py
./hgview-1.7.1-2.fc20.noarch.rpm/usr/lib/python2.7/site-packages/hgext/hgview.py

./hgview-curses-1.7.1-2.fc20.noarch.rpm/usr/lib/python2.7/site-packages/hgext/hgview.pyc
./hgview-1.7.1-2.fc20.noarch.rpm/usr/lib/python2.7/site-packages/hgext/hgview.pyc

./hgview-curses-1.7.1-2.fc20.noarch.rpm/usr/bin/hgview
./hgview-1.7.1-2.fc20.noarch.rpm/usr/bin/hgview

5 duplicate files (in 5 sets), occupying 8.2 kylobytes

---> A package must NOT list a file more than once in the %files listings.
     see: https://fedoraproject.org/wiki/Packaging/Guidelines#Duplicate_Files

#####

Installation errors
-------------------
INFO: mock.py version 1.1.32 starting...
Start: init plugins
INFO: selinux enabled
Finish: init plugins
Start: run
Mock Version: 1.1.32
INFO: Mock Version: 1.1.32
Start: lock buildroot
INFO: installing package(s): /home/bjoern.esser/fedora/review/736861-hgview/results/hgview-1.7.1-2.fc20.noarch.rpm
ERROR: Command failed: 
 # ['/usr/bin/yum', '--installroot', '/var/lib/mock/fedora-rawhide-x86_64/root/', 'install', '/home/bjoern.esser/fedora/review/736861-hgview/results/hgview-1.7.1-2.fc20.noarch.rpm', '--setopt=tsflags=nocontexts']
Error: Package: hgview-1.7.1-2.fc20.noarch (/hgview-1.7.1-2.fc20.noarch)
            Requires: hgview-common = 1.7.1-2.fc20

---> Why do you split-up the build in three noarched pkgs?
     Having everything in a single noarch-pkg and an additional
     qt-subpkg with desktop-file, icon and Requires: ${qt_stuff} hgview
     would be fine, I suppose?!

#####

cat > $RPM_BUILD_ROOT%{_sysconfdir}/mercurial/hgrc.d/hgview.rc << EOT
[extensions]
# Enable hgview extension to be able to invoke hgview as 'hg hgview' or 'hg qv'.
#hgview = %{python_sitelib}/hgext/hgview.py

---> should be: hgext.hgview = %{python_sitelib}/hgext/hgview.py
     Why do you comment-out by default?

#####

Please fix those sure blockers and I'll run a more detailed review.

(In reply to Björn Esser from comment #15)
> ---> A package must NOT list a file more than once in the %files listings.
>      see: https://fedoraproject.org/wiki/Packaging/Guidelines#Duplicate_Files

You are right. I have seen it used (for instance in grub2) and was sure it was allowed - and don't see why it shouldn't. Including files in multiple sub packages can make it possible to avoid -common packages and avoid providing something before it actually is available.

However, I have moved more files to the -common package.

> Installation errors
...
> Error: Package: hgview-1.7.1-2.fc20.noarch (/hgview-1.7.1-2.fc20.noarch)
>             Requires: hgview-common = 1.7.1-2.fc20

Eeh? Yes, installing a package without its dependencies is an "installation error", not a packaging error.
 
> ---> Why do you split-up the build in three noarched pkgs?
>      Having everything in a single noarch-pkg and an additional
>      qt-subpkg with desktop-file, icon and Requires: ${qt_stuff} hgview
>      would be fine, I suppose?!

There are two independent UIs for hgview. qt users might not want to install the curses dependencies.

> #hgview = %{python_sitelib}/hgext/hgview.py
> 
> ---> should be: hgext.hgview = %{python_sitelib}/hgext/hgview.py

No it should not. hgext is optional (and the left hand side doesn't matter when there is a right hand side).

>      Why do you comment-out by default?

Because it is a best practice for Mercurial that extensions shouldn't be enabled by default. The ability to invoke hgview as 'hg hgview' instead of 'hgview' is also not essential.

Spec URL: http://kiilerix.fedorapeople.org/hgview.spec
SRPM URL: http://kiilerix.fedorapeople.org/hgview-1.7.1-3.fc19.src.rpm

Package has four issues as shown in review-report below.

#####

Package Review
==============

Legend:
[x] = Pass
[!] = Fail
[-] = Not applicable
[?] = Not evaluated
[ ] = Manual review needed


Issues:
=======
- Package installs properly.
  Note: Installation errors (see attachment)
  See: https://fedoraproject.org/wiki/Packaging:Guidelines

  ---> you should probably just split into two pkgs:
         * hgview (containing former -common and -ncurses)
         * hgview-qt (containing former main-pkg, requiring new-main-pkg)
       ncurses-deps don't occupy that much bandwith and disk-space and
       will make main-pkg basically useful on ANY install.

- Package contains BR: python2-devel or python3-devel
  See: http://fedoraproject.org/wiki/Packaging:Python#BuildRequires

  ---> 's!python-devel!python2-devel!'

- update-desktop-database is invoked when required
  Note: desktop file(s) in hgview
  See: http://fedoraproject.org/wiki/Packaging:ScriptletSnippets#Icon_Cache

  ---> add scriptlets as proposed in link

- Packages should try to preserve timestamps of original installed files.

  ---> -install -m 644 -D %{SOURCE1} ...
       +install -pDm 0644 %{SOURCE1} ...


===== MUST items =====

Generic:
[x]: Package is licensed with an open-source compatible license and meets
     other legal requirements as defined in the legal section of Packaging
     Guidelines.
[x]: Package contains no bundled libraries without FPC exception.
[x]: Changelog in prescribed format.
[x]: Sources contain only permissible code or content.
[-]: Development files must be in a -devel package
[x]: Package requires other packages for directories it uses.
[x]: Package uses nothing in %doc for runtime.
[x]: Package is not known to require ExcludeArch.
[x]: Package complies to the Packaging Guidelines
[x]: If (and only if) the source package includes the text of the license(s)
     in its own file, then that file, containing the text of the license(s)
     for the package is included in %doc.
[x]: License field in the package spec file matches the actual license.
     Note: Checking patched sources after %prep for licenses. Licenses found:
     "GPL (v2 or later)", "Unknown or generated". 8 files have unknown
     license. Detailed output of licensecheck in
     /home/bjoern.esser/fedora/review/736861-hgview/licensecheck.txt
[x]: Package consistently uses macro is (instead of hard-coded directory
     names).
[x]: Package is named according to the Package Naming Guidelines.
[x]: Package does not generate any conflict.
[x]: Package obeys FHS, except libexecdir and /usr/target.
[-]: If the package is a rename of another package, proper Obsoletes and
     Provides are present.
[x]: Package must own all directories that it creates.
[x]: Package does not own files or directories owned by other packages.
[x]: Requires correct, justified where necessary.
[x]: Spec file is legible and written in American English.
[-]: Package contains systemd file(s) if in need.
[-]: Large documentation must go in a -doc subpackage.
     Note: Documentation size is 30720 bytes in 2 files.
[x]: All build dependencies are listed in BuildRequires, except for any that
     are listed in the exceptions section of Packaging Guidelines.
[x]: Package does not run rm -rf %{buildroot} (or $RPM_BUILD_ROOT) at the
     beginning of %install.
[x]: Each %files section contains %defattr if rpm < 4.4
[x]: Macros in Summary, %description expandable at SRPM build time.
[x]: Package contains desktop file if it is a GUI application.
[x]: Package installs a %{name}.desktop using desktop-file-install if there is
     such a file.
[x]: Package does not contain duplicates in %files.
[x]: Permissions on files are set properly.
[x]: Fully versioned dependency in subpackages, if present.
[x]: Package use %makeinstall only when make install' ' DESTDIR=... doesn't
     work.
[x]: Package is named using only allowed ASCII characters.
[x]: Package do not use a name that already exist
[x]: Package is not relocatable.
[x]: Sources used to build the package match the upstream source, as provided
     in the spec URL.
[x]: Spec file name must match the spec package %{name}, in the format
     %{name}.spec.
[x]: File names are valid UTF-8.
[x]: Packages must not store files under /srv, /opt or /usr/local
[x]: Package successfully compiles and builds into binary rpms on at least one
     supported primary architecture.
[x]: Rpmlint is run on all rpms the build produces.
     Note: There are rpmlint messages (see attachment).

Python:
[x]: Python eggs must not download any dependencies during the build process.
[x]: A package which is used by another package via an egg interface should
     provide egg info.
[x]: Package meets the Packaging Guidelines::Python
[x]: Binary eggs must be removed in %prep

===== SHOULD items =====

Generic:
[-]: If the source package does not include license text(s) as a separate file
     from upstream, the packager SHOULD query upstream to include it.
[x]: Final provides and requires are sane (see attachments).
[x]: Package functions as described.
[x]: Latest version is packaged.
[x]: Package does not include license text files separate from upstream.
[-]: Patches link to upstream bugs/comments/lists or are otherwise justified.
[-]: Description and summary sections in the package spec file contains
     translations for supported Non-English languages, if available.
[x]: Package should compile and build into binary rpms on all supported
     architectures.
[-]: %check is present and all tests pass.
[!]: Packages should try to preserve timestamps of original installed files.

     ---> -install -m 644 -D %{SOURCE1} ...
          +install -pDm 0644 %{SOURCE1} ...

[x]: Packager, Vendor, PreReq, Copyright tags should not be in spec file
[x]: Sources can be downloaded from URI in Source: tag
[x]: Reviewer should test that the package builds in mock.
[x]: Buildroot is not present
[x]: Package has no %clean section with rm -rf %{buildroot} (or
     $RPM_BUILD_ROOT)
[x]: Dist tag is present.
[x]: No file requires outside of /etc, /bin, /sbin, /usr/bin, /usr/sbin.
[x]: SourceX tarball generation or download is documented.
[x]: SourceX is a working URL.
[x]: Spec use %global instead of %define.

===== EXTRA items =====

Generic:
[!]: Rpmlint is run on all installed packages.
     Note: Mock build failed
[x]: Large data in /usr/share should live in a noarch subpackage if package is
     arched.
[x]: Spec file according to URL is the same as in SRPM.


Installation errors
-------------------
INFO: mock.py version 1.1.32 starting...
Start: init plugins
INFO: selinux enabled
Finish: init plugins
Start: run
Mock Version: 1.1.32
INFO: Mock Version: 1.1.32
Start: lock buildroot
INFO: installing package(s): /home/bjoern.esser/fedora/review/736861-hgview/results/hgview-1.7.1-3.fc20.noarch.rpm
ERROR: Command failed: 
 # ['/usr/bin/yum', '--installroot', '/var/lib/mock/fedora-rawhide-x86_64/root/', 'install', '/home/bjoern.esser/fedora/review/736861-hgview/results/hgview-1.7.1-3.fc20.noarch.rpm', '--setopt=tsflags=nocontexts']
Error: Package: hgview-1.7.1-3.fc20.noarch (/hgview-1.7.1-3.fc20.noarch)
           Requires: hgview-common = 1.7.1-3.fc20
 You could try using --skip-broken to work around the problem
 You could try running: rpm -Va --nofiles --nodigest



Rpmlint
-------
Checking: hgview-1.7.1-3.fc20.noarch.rpm
hgview.noarch: W: no-documentation
hgview.noarch: W: desktopfile-without-binary /usr/share/applications/hgview.desktop hgview
1 packages and 0 specfiles checked; 0 errors, 2 warnings.




Requires
--------
hgview (rpmlib, GLIBC filtered):
    PyQt4
    hgview-common
    python(abi)
    python-docutils
    qscintilla-python



Provides
--------
hgview:
    hgview



Source checksums
----------------
http://download.logilab.org/pub/hgview/hgview-1.7.1.tar.gz :
  CHECKSUM(SHA256) this package     : 633862c3a2313e5f2432f19b9da9fa19a1ca8f2f2cd0b86df019832e86afc001
  CHECKSUM(SHA256) upstream package : 633862c3a2313e5f2432f19b9da9fa19a1ca8f2f2cd0b86df019832e86afc001


Generated by fedora-review 0.4.1 (b2e211f) last change: 2013-04-29
Buildroot used: fedora-rawhide-x86_64
Command line :/usr/bin/fedora-review -m fedora-rawhide-x86_64 -b 736861

#####

Please fix the issues and I'll grant review.

(In reply to Björn Esser from comment #17)

>   ---> you should probably just split into two pkgs:

Thank you for the opinion and advice.

I am still convinced that curses should be a separate package. Installing hgview-curses can take 7.5 MB on disk including the dependencies.

> - Package contains BR: python2-devel

Right. The guidelines has changed since the package was created.

> - update-desktop-database is invoked when required
>   Note: desktop file(s) in hgview
>   See: http://fedoraproject.org/wiki/Packaging:ScriptletSnippets#Icon_Cache

The package has no icons - only a pixmap. That section do thus not apply.

You mean http://fedoraproject.org/wiki/Packaging:ScriptletSnippets#desktop-database ? No - the .desktop file has no MimeType.

> - Packages should try to preserve timestamps of original installed files.
> 
>   ---> -install -m 644 -D %{SOURCE1} ...
>        +install -pDm 0644 %{SOURCE1} ...

That is a file I created by converting the favicon from upstreams website. The "original" timestamp is completely irrelevant. The guidelines says "consider". But ok.

> [!]: Rpmlint is run on all installed packages.
>      Note: Mock build failed
...
> Installation errors
> -------------------
> INFO: mock.py version 1.1.32 starting...
...
> INFO: installing package(s):
> /home/bjoern.esser/fedora/review/736861-hgview/results/hgview-1.7.1-3.fc20.
> noarch.rpm
> ERROR: Command failed: 
>  # ['/usr/bin/yum', '--installroot',
> '/var/lib/mock/fedora-rawhide-x86_64/root/', 'install',
> '/home/bjoern.esser/fedora/review/736861-hgview/results/hgview-1.7.1-3.fc20.
> noarch.rpm', '--setopt=tsflags=nocontexts']
> Error: Package: hgview-1.7.1-3.fc20.noarch (/hgview-1.7.1-3.fc20.noarch)
>            Requires: hgview-common = 1.7.1-3.fc20
>  You could try using --skip-broken to work around the problem
>  You could try running: rpm -Va --nofiles --nodigest

You said that before, and I pointed out that the command you show is attempting do do an invalid installation - that is not a mock build.

Please explain what you are trying to do, what you are doing, and why you think that is the right thing to do.


Spec URL: http://kiilerix.fedorapeople.org/hgview.spec
SRPM URL: http://kiilerix.fedorapeople.org/hgview-1.7.1-4.fc19.src.rpm

Created attachment 762010 [details]
improvements for spec-file

(In reply to Mads Kiilerich from comment #18)
> Thank you for the opinion and advice.
> 
> I am still convinced that curses should be a separate package. Installing
> hgview-curses can take 7.5 MB on disk including the dependencies.

The other way round one can argue about hgview pulling in "half-a-desktop", even if user just needs/wants ncurses interface, e.g. on terminal-only machine.

So I though a bit about it and come in with another aproach making both sides happy, I hope.

When you have a look at the attached patch, you'll see I changed package-order a bit.  There are three pkgs now: hgview (is what was former -common) depending hgview-ui and hgview-{curses,qt} providing hgview-ui, requiring hgview.

This results in 

  a) the user can choose which ui to install

  b) even just installing hgview will install an ui, so we have a working base
     install. Yum should be smart enough to pull in the
     "cheaper = less bandwith/disk" ui.

What's your opinion to this?

(In reply to Björn Esser from comment #19)
> The other way round one can argue about hgview pulling in "half-a-desktop",
> even if user just needs/wants ncurses interface, e.g. on terminal-only
> machine.

If the user wants hgview-curses without the qt stuff in hgview then he just installs hgview-curses.

> What's your opinion to this?

My opinion is that you are proposing changes that might be another valid way to do this packaging but wouldn't improve the packaging. Thanks for sharing your opinion, but it is hardly relevant for a review.

Package is fine.

#####

Package Review
==============

Legend:
[x] = Pass
[!] = Fail
[-] = Not applicable
[?] = Not evaluated
[ ] = Manual review needed


Issues:
=======
- update-desktop-database is invoked when required
  Note: desktop file(s) in hgview
  See: http://fedoraproject.org/wiki/Packaging:ScriptletSnippets#Icon_Cache

  ---> false positive: icon is installed as pixmap.


===== MUST items =====

Generic:
[x]: Package is licensed with an open-source compatible license and meets
     other legal requirements as defined in the legal section of Packaging
     Guidelines.
[x]: Package contains no bundled libraries without FPC exception.
[x]: Changelog in prescribed format.
[x]: Sources contain only permissible code or content.
[-]: Development files must be in a -devel package
[x]: Package requires other packages for directories it uses.
[x]: Package uses nothing in %doc for runtime.
[x]: Package is not known to require ExcludeArch.
[x]: Package complies to the Packaging Guidelines
[x]: If (and only if) the source package includes the text of the license(s)
     in its own file, then that file, containing the text of the license(s)
     for the package is included in %doc.
[x]: License field in the package spec file matches the actual license.
     Note: Checking patched sources after %prep for licenses. Licenses found:
     "GPL (v2 or later)", "Unknown or generated". 8 files have unknown
     license. Detailed output of licensecheck in
     /home/bjoern.esser/fedora/review/736861-hgview/licensecheck.txt

     ---> License-Tag is fine.

[x]: Package consistently uses macro is (instead of hard-coded directory
     names).
[x]: Package is named according to the Package Naming Guidelines.
[x]: Package does not generate any conflict.
[x]: Package obeys FHS, except libexecdir and /usr/target.
[-]: If the package is a rename of another package, proper Obsoletes and
     Provides are present.
[x]: Package must own all directories that it creates.
[x]: Package does not own files or directories owned by other packages.
[x]: Requires correct, justified where necessary.
[x]: Spec file is legible and written in American English.
[-]: Package contains systemd file(s) if in need.
[-]: Large documentation must go in a -doc subpackage.
     Note: Documentation size is 30720 bytes in 2 files.
[x]: All build dependencies are listed in BuildRequires, except for any that
     are listed in the exceptions section of Packaging Guidelines.
[x]: Package does not run rm -rf %{buildroot} (or $RPM_BUILD_ROOT) at the
     beginning of %install.
[x]: Each %files section contains %defattr if rpm < 4.4
[x]: Macros in Summary, %description expandable at SRPM build time.
[x]: Package contains desktop file if it is a GUI application.
[x]: Package installs a %{name}.desktop using desktop-file-install if there is
     such a file.
[x]: Package does not contain duplicates in %files.
[x]: Permissions on files are set properly.
[x]: Fully versioned dependency in subpackages, if present.
[x]: Package use %makeinstall only when make install' ' DESTDIR=... doesn't
     work.
[x]: Package is named using only allowed ASCII characters.
[x]: Package do not use a name that already exist
[x]: Package is not relocatable.
[x]: Sources used to build the package match the upstream source, as provided
     in the spec URL.
[x]: Spec file name must match the spec package %{name}, in the format
     %{name}.spec.
[x]: File names are valid UTF-8.
[x]: Packages must not store files under /srv, /opt or /usr/local
[x]: Package successfully compiles and builds into binary rpms on at least one
     supported primary architecture.
[x]: Package installs properly.
[x]: Rpmlint is run on all rpms the build produces.
     Note: There are rpmlint messages (see attachment).

Python:
[x]: Python eggs must not download any dependencies during the build process.
[-]: A package which is used by another package via an egg interface should
     provide egg info.
[x]: Package meets the Packaging Guidelines::Python
[x]: Package contains BR: python2-devel or python3-devel
[x]: Binary eggs must be removed in %prep

===== SHOULD items =====

Generic:
[-]: If the source package does not include license text(s) as a separate file
     from upstream, the packager SHOULD query upstream to include it.
[x]: Final provides and requires are sane (see attachments).
[x]: Package functions as described.
[x]: Latest version is packaged.
[x]: Package does not include license text files separate from upstream.
[x]: Patches link to upstream bugs/comments/lists or are otherwise justified.
[-]: Description and summary sections in the package spec file contains
     translations for supported Non-English languages, if available.
[x]: Package should compile and build into binary rpms on all supported
     architectures.
[-]: %check is present and all tests pass.
[x]: Packages should try to preserve timestamps of original installed files.
[x]: Packager, Vendor, PreReq, Copyright tags should not be in spec file
[x]: Sources can be downloaded from URI in Source: tag
[x]: Reviewer should test that the package builds in mock.
[x]: Buildroot is not present
[x]: Package has no %clean section with rm -rf %{buildroot} (or
     $RPM_BUILD_ROOT)
[x]: Dist tag is present.
[x]: No file requires outside of /etc, /bin, /sbin, /usr/bin, /usr/sbin.
[x]: SourceX tarball generation or download is documented.
[x]: SourceX is a working URL.
[x]: Spec use %global instead of %define.

===== EXTRA items =====

Generic:
[x]: Large data in /usr/share should live in a noarch subpackage if package is
     arched.
[x]: Rpmlint is run on all installed packages.
     Note: There are rpmlint messages (see attachment).
[x]: Spec file according to URL is the same as in SRPM.


Rpmlint
-------
Checking: hgview-1.7.1-4.fc20.noarch.rpm
hgview.noarch: W: no-documentation
hgview.noarch: W: desktopfile-without-binary /usr/share/applications/hgview.desktop hgview
1 packages and 0 specfiles checked; 0 errors, 2 warnings.

---> false positve: "binary" gets pulled by dependency.


Rpmlint (installed packages)
----------------------------
# rpmlint hgview
hgview.noarch: W: no-documentation
1 packages and 0 specfiles checked; 0 errors, 1 warnings.
# echo 'rpmlint-done:'



Requires
--------
hgview (rpmlib, GLIBC filtered):
    PyQt4
    hgview-common
    python(abi)
    python-docutils
    qscintilla-python



Provides
--------
hgview:
    hgview



Source checksums
----------------
http://download.logilab.org/pub/hgview/hgview-1.7.1.tar.gz :
  CHECKSUM(SHA256) this package     : 633862c3a2313e5f2432f19b9da9fa19a1ca8f2f2cd0b86df019832e86afc001
  CHECKSUM(SHA256) upstream package : 633862c3a2313e5f2432f19b9da9fa19a1ca8f2f2cd0b86df019832e86afc001


Generated by fedora-review 0.4.1 (b2e211f) last change: 2013-04-29
Buildroot used: fedora-rawhide-x86_64
Command line :/usr/bin/fedora-review -m fedora-rawhide-x86_64 -b 736861

#####

APPROVED!

> %files
> %{python_sitelib}/hgviewlib/qt4/

> %files -n %{name}-common
> %{python_sitelib}/hgext/%{name}.py*

%python_sitelib is below /usr/lib for x86_64, whereas mercurial.x86_64 stores its extensions below /usr/lib64 => %python_sitearch:
http://koji.fedoraproject.org/koji/packageinfo?packageID=2518

It does not include %{python_sitelib}/hgext/ yet, but some package ought to own it: https://fedoraproject.org/wiki/Packaging:Guidelines#File_and_Directory_Ownership

(In reply to Michael Schwendt from comment #22)

Hmm ... yeah ... nothing owns /usr/lib/.../hgext on x86_64 ... but it is owned on x86.

Mercurial on x86_64 doesn't look in /usr/lib/.../hgext at all so it would be strange if Mercurial owned that directory.

For almost the same reasons my sample hg config for this extension uses an absolute path. This hgview.py should perhaps just be stored in the ordinary hgview directory instead of in an hgext directory.

NEWS?

New Package SCM Request
=======================
Package Name: hgview
Short Description: A fast Mercurial log navigator for qt4 or curses
Owners: kiilerix
Branches: f19 f20
InitialCC:

Git done (by process-git-requests).

hgview-1.7.1-6.fc19 has been submitted as an update for Fedora 19.
https://admin.fedoraproject.org/updates/hgview-1.7.1-6.fc19

Package hgview-1.7.1-6.fc19:
* should fix your issue,
* was pushed to the Fedora 19 testing repository,
* should be available at your local mirror within two days.
Update it with:
# su -c 'yum update --enablerepo=updates-testing hgview-1.7.1-6.fc19'
as soon as you are able to.
Please go to the following url:
https://admin.fedoraproject.org/updates/FEDORA-2013-15644/hgview-1.7.1-6.fc19
then log in and leave karma (feedback).

hgview-1.7.1-6.fc19 has been pushed to the Fedora 19 stable repository.  If problems still persist, please make note of it in this bug report.