Rawhide : http://koji.fedoraproject.org/koji/taskinfo?taskID=6543781
F20 : http://koji.fedoraproject.org/koji/taskinfo?taskID=6543796
F19 : http://koji.fedoraproject.org/koji/taskinfo?taskID=6543806
EPEL7 : https://koji.fedoraproject.org/koji/taskinfo?taskID=6543842 (None completed because it isn't include BR vte-devel)
EPEL6 : http://koji.fedoraproject.org/koji/taskinfo?taskID=6543822

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

Legend:
[x] = Pass, [!] = Fail, [-] = Not applicable, [?] = Not evaluated



Issues:
=======
- Package installs properly.
  Note: Installation errors (see attachment)
  See: https://fedoraproject.org/wiki/Packaging:Guidelines
- gtk-update-icon-cache is invoked in %postun and %posttrans if package
  contains icons.
  Note: icons in gtkdialog
  See: http://fedoraproject.org/wiki/Packaging:ScriptletSnippets#Icon_Cache
- Texinfo files are installed using install-info in %post and %preun if
  package has .info files.
  Note: Texinfo .info file(s) in gtkdialog-doc
  See: http://fedoraproject.org/wiki/Packaging:ScriptletSnippets#Texinfo
- 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.
  Note: Cannot find COPYING-elementary-icons in rpm(s)
  See:
  http://fedoraproject.org/wiki/Packaging/LicensingGuidelines#License_Text
- Package installs a %{name}.desktop using desktop-file-install or desktop-
  file-validate if there is such a file.


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

C/C++:
[x]: Package does not contain kernel modules.
[x]: Package contains no static executables.
[x]: Package does not contain any libtool archives (.la)
[x]: Rpath absent or only used for internal libs.

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.
[ ]: 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) (with incorrect FSF address)", "GPL (v2 or later)",
     "GPL (v3 or later)", "Unknown or generated". 5 files have unknown
     license. Detailed output of licensecheck in
     /home/rpmaker/Desktop/gtkdialog/licensecheck.txt
[x]: License file installed when any subpackage combination is installed.
[!]: Package requires other packages for directories it uses.
     Note: No known owner of /usr/share/gtkdialog
[!]: Package must own all directories that it creates.
     Note: Directories without known owners: /usr/share/doc/gtkdialog,
     /usr/share/gtkdialog
[!]: Package does not own files or directories owned by other packages.
     Note: Dirs in package are owned also by:
     /usr/share/icons/hicolor/32x32/apps(hicolor-icon-theme, fedora-logos,
     eom), /usr/share/icons/hicolor/32x32(hicolor-icon-theme, fedora-logos,
     eom), /usr/share/icons/hicolor(hicolor-icon-theme, fedora-logos)
[x]: %build honors applicable compiler flags or justifies otherwise.
[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 uses nothing in %doc for runtime.
[x]: Package consistently uses macros (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]: Requires correct, justified where necessary.
[x]: Spec file is legible and written in American English.
[-]: Package contains systemd file(s) if in need.
[x]: Useful -debuginfo package or justification otherwise.
[x]: Package is not known to require an ExcludeArch tag.
[x]: Large documentation must go in a -doc subpackage. Large could be size
     (~1MB) or number of files.
     Note: Documentation size is 696320 bytes in 44 files.
[x]: Package complies to the Packaging Guidelines
[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).
[x]: All build dependencies are listed in BuildRequires, except for any that
     are listed in the exceptions section of Packaging Guidelines.
[x]: Package uses either %{buildroot} or $RPM_BUILD_ROOT
[x]: Package does not run rm -rf %{buildroot} (or $RPM_BUILD_ROOT) at the
     beginning of %install.
[x]: Macros in Summary, %description expandable at SRPM build time.
[x]: Package contains desktop file if it is a GUI application.
[x]: Package does not contain duplicates in %files.
[x]: Permissions on files are set properly.
[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

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

Generic:
[!]: No file requires outside of /etc, /bin, /sbin, /usr/bin, /usr/sbin.
     Note: Incorrect Requires : /usr/local/bin/gtkdialog
     See: http://fedoraproject.org/wiki/Packaging/Guidelines#FileDeps
[x]: 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]: Fully versioned dependency in subpackages if applicable.
     Note: No Requires: %{name}%{?_isa} = %{version}-%{release} in gtkdialog-
     doc , gtkdialog-examples
[x]: Package functions as described.
[x]: Latest version is packaged.
[x]: Package does not include license text files separate from upstream.
[x]: 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 (not strictly required in GL).
[x]: Uses parallel make %{?_smp_mflags} macro.
[x]: SourceX is a working URL.
[x]: Spec use %global instead of %define unless justified.

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

Generic:
[!]: Rpmlint is run on all installed packages.
     Note: Mock build failed
     See: http://fedoraproject.org/wiki/Packaging/Guidelines#rpmlint
[x]: Large data in /usr/share should live in a noarch subpackage if package is
     arched.


Installation errors
-------------------
INFO: mock.py version 1.1.36 starting...
Start: init plugins
INFO: selinux enabled
Finish: init plugins
Start: run
Mock Version: 1.1.36
INFO: Mock Version: 1.1.36
Start: lock buildroot
INFO: installing package(s): /home/rpmaker/Desktop/gtkdialog/results/gtkdialog-0.8.3-1.fc21.i686.rpm /home/rpmaker/Desktop/gtkdialog/results/gtkdialog-doc-0.8.3-1.fc21.noarch.rpm /home/rpmaker/Desktop/gtkdialog/results/gtkdialog-examples-0.8.3-1.fc21.noarch.rpm
ERROR: Command failed: 
 # ['/usr/bin/yum', '--installroot', '/var/lib/mock/fedora-rawhide-i386/root/', '--releasever', '21', 'install', '/home/rpmaker/Desktop/gtkdialog/results/gtkdialog-0.8.3-1.fc21.i686.rpm', '/home/rpmaker/Desktop/gtkdialog/results/gtkdialog-doc-0.8.3-1.fc21.noarch.rpm', '/home/rpmaker/Desktop/gtkdialog/results/gtkdialog-examples-0.8.3-1.fc21.noarch.rpm', '--setopt=tsflags=nocontexts']
Error: Package: gtkdialog-examples-0.8.3-1.fc21.noarch (/gtkdialog-examples-0.8.3-1.fc21.noarch)
           Requires: /usr/local/bin/gtkdialog
 You could try using --skip-broken to work around the problem
 You could try running: rpm -Va --nofiles --nodigest



Rpmlint
-------
Checking: gtkdialog-0.8.3-1.fc21.i686.rpm
          gtkdialog-doc-0.8.3-1.fc21.noarch.rpm
          gtkdialog-examples-0.8.3-1.fc21.noarch.rpm
          gtkdialog-0.8.3-1.fc21.src.rpm
gtkdialog.i686: E: incorrect-fsf-address /usr/share/doc/gtkdialog/COPYING
gtkdialog.i686: W: no-manual-page-for-binary gtkdialog
gtkdialog.i686: W: install-file-in-docs /usr/share/doc/gtkdialog/INSTALL
gtkdialog-doc.noarch: E: info-files-without-install-info-postin /usr/share/info/gtkdialog.info.gz
gtkdialog-doc.noarch: E: info-files-without-install-info-postun /usr/share/info/gtkdialog.info.gz
gtkdialog-examples.noarch: E: invalid-dependency /usr/local/bin/gtkdialog
gtkdialog-examples.noarch: W: no-documentation
gtkdialog-examples.noarch: W: devel-file-in-non-devel-package /usr/share/gtkdialog/examples/c_embedded/example01.c
gtkdialog-examples.noarch: E: non-executable-script /usr/share/gtkdialog/examples/glade/glade-entries_functions.functions 0644L /bin/sh
gtkdialog-examples.noarch: E: incorrect-fsf-address /usr/share/gtkdialog/examples/pfeme/COPYING-elementary-icons
gtkdialog-examples.noarch: E: incorrect-fsf-address /usr/share/gtkdialog/examples/button/COPYING-elementary-icons
gtkdialog-examples.noarch: E: wrong-script-interpreter /usr/share/gtkdialog/examples/miscellaneous/standalone_file /usr/local/bin/gtkdialog
gtkdialog-examples.noarch: E: incorrect-fsf-address /usr/share/gtkdialog/examples/pfeme/COPYING-fast-forward-icons
gtkdialog-examples.noarch: E: incorrect-fsf-address /usr/share/gtkdialog/examples/pfeme/COPYING-nuvola-icons
gtkdialog-examples.noarch: E: incorrect-fsf-address /usr/share/gtkdialog/examples/togglebutton/COPYING-elementary-icons
gtkdialog.src: W: invalid-url Source0: https://gtkdialog.googlecode.com/files/gtkdialog-0.8.3.tar.gz <urlopen error [Errno 101] Network is unreachable>
4 packages and 0 specfiles checked; 11 errors, 5 warnings.




Requires
--------
gtkdialog (rpmlib, GLIBC filtered):
    libX11.so.6
    libXext.so.6
    libatk-1.0.so.0
    libc.so.6
    libcairo.so.2
    libfontconfig.so.1
    libfreetype.so.6
    libgdk-x11-2.0.so.0
    libgdk_pixbuf-2.0.so.0
    libgio-2.0.so.0
    libglade-2.0.so.0
    libglib-2.0.so.0
    libgobject-2.0.so.0
    libgthread-2.0.so.0
    libgtk-x11-2.0.so.0
    libpango-1.0.so.0
    libpangocairo-1.0.so.0
    libpangoft2-1.0.so.0
    libpthread.so.0
    libvte.so.9
    libxml2.so.2
    rtld(GNU_HASH)

gtkdialog-doc (rpmlib, GLIBC filtered):

gtkdialog-examples (rpmlib, GLIBC filtered):
    /bin/bash
    /bin/sh
    /usr/local/bin/gtkdialog



Provides
--------
gtkdialog:
    gtkdialog
    gtkdialog(x86-32)

gtkdialog-doc:
    gtkdialog-doc

gtkdialog-examples:
    gtkdialog-examples



Source checksums
----------------
https://gtkdialog.googlecode.com/files/gtkdialog-0.8.3.tar.gz :
  CHECKSUM(SHA256) this package     : ff89d2d7f1e6488e5df5f895716ac1d4198c2467a2a5dc1f51ab408a2faec38e
  CHECKSUM(SHA256) upstream package : ff89d2d7f1e6488e5df5f895716ac1d4198c2467a2a5dc1f51ab408a2faec38e


Generated by fedora-review 0.5.1 (bb9bf27) last change: 2013-12-13
Command line :/usr/bin/fedora-review -rvn gtkdialog-0.8.3-1.oj35.src.rpm
Buildroot used: fedora-rawhide-i386
Active plugins: Generic, Shell-api, C/C++
Disabled plugins: Java, Python, fonts, SugarActivity, Ocaml, Perl, Haskell, R, PHP, Ruby
Disabled flags: EXARCH, EPEL5, BATCH, DISTTAG

--------------------------------
1. Yes vte is missing on EPEL7 now.

2. I don't think that you need to split out example and doc subpackages, since gtkdialog is a dev tool for developers(junior level IMO), they want to use it with example in hand.

3. %makeinstall not allowed.

4. https://gtkdialog.googlecode.com/files/gtkdialog-0.8.3.tar.gz

You should use a %{version} macro at least:

https://gtkdialog.googlecode.com/files/gtkdialog-%{version}.tar.gz

5. Fix ALL issues above.

All Fixed, except %makeinstall , It isn't built with: make install DESTDIR=%{buildroot}


-----------

Spec URL: http://ojuba.org/oji/SPECS/gtkdialog.spec
SRPM URL: http://ojuba.org/oji/SRPMS/gtkdialog-0.8.3-2.oj35.src.rpm

https://fedoraproject.org/wiki/Packaging:Guidelines#Why_the_.25makeinstall_macro_should_not_be_used

I read that , But First Line in Article ::

>>
Fedora's RPM includes a %makeinstall macro but it must NOT be used when make install DESTDIR=%{buildroot} works.
>> 

Not only this way but also %%make_install macro doesn't work , Otherwise Gtkdialog isn't include old libs or any thing that forbid this way.

Mosaab.

Rpmlint
-------
Checking: gtkdialog-0.8.3-2.fc21.i686.rpm
          gtkdialog-doc-0.8.3-2.fc21.noarch.rpm
          gtkdialog-0.8.3-2.fc21.src.rpm
gtkdialog.i686: W: install-file-in-docs /usr/share/doc/gtkdialog/INSTALL
gtkdialog-doc.noarch: E: invalid-dependency /usr/local/bin/gtkdialog
gtkdialog-doc.noarch: E: non-executable-script /usr/share/gtkdialog/examples/glade/glade-entries_functions.functions 0644L /bin/sh
gtkdialog-doc.noarch: E: wrong-script-interpreter /usr/share/gtkdialog/examples/miscellaneous/standalone_file /usr/local/bin/gtkdialog

They cause these:

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

Error: Package: gtkdialog-doc-0.8.3-2.fc21.noarch (/gtkdialog-doc-0.8.3-2.fc21.noarch)
           Requires: /usr/local/bin/gtkdialog
 You could try using --skip-broken to work around the problem
 You could try running: rpm -Va --nofiles --nodigest

2.   [!]: No file requires outside of /etc, /bin, /sbin, /usr/bin, /usr/sbin.
     Note: Incorrect Requires : /usr/local/bin/gtkdialog
     See: http://fedoraproject.org/wiki/Packaging/Guidelines#FileDeps

And, where is the desktop file?

> %%make_install macro doesn't work

Guess why! As a hint, run "rpm -E %make_install".

Please add a comment in the spec file above your %makeinstall usage, which tells that it must be used currently. For future upgrades of the software the macro usage may be revisited.

There in no-desktop-file and it is not-needed.

-----------

Spec URL: http://ojuba.org/oji/SPECS/gtkdialog.spec
SRPM URL: http://ojuba.org/oji/SRPMS/gtkdialog-0.8.3-3.oj35.src.rpm

Mosaab,
   Lets first ask Christopher if he is having time to finish this package review.

This review has now been assigned for about 9 months with no action.  Will you be able to start the review in the next 2 days?  If not, please release it.

I was busy since last few days will start the review soon.

Mosaab,
   I am trying to fix some issues in the spec by using %make_install and today when I found we can use make_install I find this binary gtkdialog is failing. Can you check and tell if above last updated srpm which is using %makeinstall is working for you?

   For me on Fedora 21 its failing 
$ gtkdialog 
** (gtkdialog:1413): ERROR **: Gtkdialog: Could not find the dialog description in the environment variable 'MAIN_DIALOG'.
Trace/breakpoint trap (core dumped)

To using program you must read some refs http://www.tecmint.com/gtkdialog-create-graphical-interfaces-and-dialog-boxes/ ..

I tried many times with %make_install without any success !! only %makeinstall can continue ..
 
I tried to rebuild now and fix an error ..

Spec URL: http://ojuba.org/test/gtkdialog.spec
SRPM URL: http://ojuba.org/test/gtkdialog-0.8.3-4.oj35.src.rpm

Can you please add full changelog for every new release? I see you are just adding "Fixes". Now when I want to check what has changed from last release, there is no information where to look into the spec file.

Some other improvements:
1) After looking into this program usage, a question, why we need to install icon files if this not that end user need to execute directly? Also we don't need desktop file so what is use case to have icon file installed?

2) doc files are added to both the packages. Let's install only required doc files. Use %files for both binary rpms as

%files
%{_bindir}/gtkdialog
%{_datadir}/icons/hicolor/32x32/apps/gtkdialog.png
%doc AUTHORS ChangeLog COPYING NEWS README TODO
%exclude %{_pkgdocdir}/examples
%exclude %{_pkgdocdir}/reference

%files doc
%{_datadir}/info/%{name}.info.gz
%{_pkgdocdir}/
%exclude %{_pkgdocdir}/AUTHORS
%exclude %{_pkgdocdir}/ChangeLog
%exclude %{_pkgdocdir}/COPYING
%exclude %{_pkgdocdir}/NEWS
%exclude %{_pkgdocdir}/README
%exclude %{_pkgdocdir}/TODO

3) now to work for using %make_install, you need to remove makefile rules that causes compilation error when used %make_install. Add to end of %prep as

sed -i '3,8d' data/icons/hicolor/Makefile.am
autoreconf -ivf
%configure

now to use autoreconf, you need to add 
BuildRequires: autoconf
BuildRequires: automake

and then in %install replace %makeinstall with
%make_install

4) I did a scratch build with above changes and it failed for missing makeinfo command. So now add
BuildRequires: texinfo

Just check what I suggested working for you also :)

Submit new srpm with what has changed since last srpm in changelog.

Created attachment 960413 [details]
fix for %make_install

Beyond that, gtkdialog-0.8.3-4.oj35.src.rpm didn't build again, because it commented out the "rm %{buildroot}%{_datadir}/info/dir" again.

Mosaab,
   I think what Michael has suggested above is better than what I asked in point 2 above. I have not checked above patch but I believe above patch is good way to have %make_install working for that package.

- Using icon due to original MAKE operation.
- All Done.

Thank you both

===========

Spec URL: http://ojuba.org/test/gtkdialog.spec
SRPM URL: http://ojuba.org/test/gtkdialog-0.8.3-5.oj35.src.rpm

1) I see mock build failed. You still have following. Un-comment it.
#rm %{buildroot}%{_datadir}/info/dir

2) Can you align the spec tag's and its value as
-------------------------------------------------------------
Name:          gtkdialog
Version:       0.8.3
Release:       5%{?dist}
Summary:       Fast and easy GUI builder
License:       GPLv2
URL:           https://code.google.com/p/gtkdialog/
Source:        https://gtkdialog.googlecode.com/files/%{name}-%{version}.tar.gz

Patch0:        gtkdialog_fix_for_make_install.patch

BuildRequires: libglade2-devel
BuildRequires: gtk2-devel
BuildRequires: vte-devel
BuildRequires: flex
BuildRequires: bison
BuildRequires: glib2-devel
BuildRequires: byacc

Requires(post): info
Requires(preun):info
----------------------------------------------------------------

3) Recommended to use patch file name starting with %{name}-%{version} so use it as
Patch0:        %{name}-%{version}-fix_for_make_install.patch

Otherwise package looks good now. And as given in docs it needs some shell scrip program which actually invoke gtkdialog binary.

- Condition to remove /info/dir
- Delete one tab from every tag
- Use recomended patch name

=======


Spec URL: http://ojuba.org/test/gtkdialog.spec
SRPM URL: http://ojuba.org/test/gtkdialog-0.8.3-6.oj35.src.rpm

I have not seen such conditional usage for removal of info directory. I don't think you need it, you can remove conditional and directly remove /info/dir.

also you can actually write it as
rm -f %{buildroot}%{_infodir}/dir

On my (f20) locally it was generate this file but now not ^_^

Please get familiar with https://fedoraproject.org/wiki/Using_the_Koji_build_system#Scratch_Builds

This will give you what is actually wrong with your package if it fails :)

Now It's almost perfect :)

- Use rm -f for all
- Compatible with EPEL7


Spec URL: http://ojuba.org/test/gtkdialog.spec
SRPM URL: http://ojuba.org/test/gtkdialog-0.8.3-7.oj35.src.rpm

not yet ;-)

rpmlint complains some warnings like doc-file-dependency, incorrect-fsf-address and mixed-use-of-spaces-and-tabs.

To fix doc-file-dependency, you need to add in %prep following
chmod 644 examples/*/*

To fix  incorrect-fsf-address, you need to ask upstream 

To fix this mixed-use-of-spaces-and-tabs, you can use sed on spec file
sed -i 's|\t| |g' gtkdialog.spec

and then create new srpm.

I think this is the last change required for this package.

> 3) Recommended to use patch file name starting with %{name}-%{version}
> so use it as
> Patch0:        %{name}-%{version}-fix_for_make_install.patch

Ill-advised, and it is not what most packagers have been doing for several years at Red Hat Linux and the Fedora Project.

Better get used to *no* macros at all in patch file names. Why? For many thousands of patch files, they still apply fine even when %version changes. Not so, if you use a %version macro in the Patch tag. You would need to rename the patch files in git and touch the spec file, too, just to make the patch apply again.

I saw that no need specially it's just in examples and already included, also I'll contact to him.
----


Spec URL: http://ojuba.org/test/gtkdialog.spec
SRPM URL: http://ojuba.org/test/gtkdialog-0.8.3-8.oj35.src.rpm

Right, packagers (yes including me also) not done it before.  I don't remember now but I think since I start using fedora-review tool and its review.txt, I am suggesting to use patch name prefix like that as fedora-review suggested. Right now I can't find such example on my machine or did fedora-review changed recently?

Mosaab, please follow what Michael suggested in comment27.

Rest looks okay.

APPROVED.

ThanX.

New Package SCM Request
=======================
Package Name: gtkdialog
Short Description: Fast easy GUI builder
Owners: moceap
Branches: f19 f20 f21 epel7

Git done (by process-git-requests).

gtkdialog-0.8.3-8.fc19 has been submitted as an update for Fedora 19.
https://admin.fedoraproject.org/updates/gtkdialog-0.8.3-8.fc19

gtkdialog-0.8.3-8.el7 has been submitted as an update for Fedora EPEL 7.
https://admin.fedoraproject.org/updates/gtkdialog-0.8.3-8.el7

gtkdialog-0.8.3-8.fc20 has been submitted as an update for Fedora 20.
https://admin.fedoraproject.org/updates/gtkdialog-0.8.3-8.fc20

gtkdialog-0.8.3-8.fc21 has been submitted as an update for Fedora 21.
https://admin.fedoraproject.org/updates/gtkdialog-0.8.3-8.fc21

gtkdialog-0.8.3-8.el7 has been pushed to the Fedora EPEL 7 testing repository.

gtkdialog-0.8.3-8.fc20 has been pushed to the Fedora 20 stable repository.

gtkdialog-0.8.3-8.fc21 has been pushed to the Fedora 21 stable repository.

gtkdialog-0.8.3-8.fc19 has been pushed to the Fedora 19 stable repository.

Package Change Request
======================
Package Name: gtkdialog
New Branches: el6
Owners: moceap

Git done (by process-git-requests).

gtkdialog-0.8.3-8.el7 has been pushed to the Fedora EPEL 7 stable repository.