Spec URL: http://www.environnement.ens.fr/perso/dumas/fc-srpms/xdialog.spec SRPM URL: http://www.environnement.ens.fr/perso/dumas/fc-srpms/xdialog-2.3.1-1.fc9.src.rpm Description: Xdialog is designed to be a drop in replacement for the cdialog program. It converts any terminal based program into a program with an X-windows interface. The dialogs are easier to see and use and Xdialog adds even more functionalities (help button+box, treeview, editbox, file selector, range box, and much more).
I have set the gtk2 Xdialog to be the default one, even though the gtk1 is said to be more stable because the bug reported is with non UTF8 locales which should be rare in fedora.
Informal package review: ======================== -Release: 1%{dist} -License: GPL+ +Release: 1%{?dist} +License: GPLv2 * The preferred dist tag is now ?dist. * License should be as concrete as possible, in source archive is GPLv2 -URL: http://xdialog.dyns.net/ +URL: http://xdialog.free.fr * This is the server, where sources are located. -BuildRoot: %{_tmppath}/%{name}-%{version}-%{release}-root +BuildRoot: %(mktemp -ud %{_tmppath}/%{name}-%{version}-%{release}-XXXXXX) * Tip: this is #1 in "BuildRoot tag" section of https://fedoraproject.org/wiki/Packaging/Guidelines BuildRequires: gtk+-devel >= 1.2.0 * Isn't it possible to completely get rid of GTK+v1? It's ugly, not widely supported these days and adds build dependency. -%{__rm} -rf %{buildroot} +rm -rf %{buildroot} * Be consistent, use command style OR macro style. -%{__rm} -rf %{buildroot} +rm -rf %{buildroot} * Same ^ here. -%defattr(-, root, root, 0755) -%doc AUTHORS BUGS ChangeLog COPYING NEWS README +%defattr(-, root, root, -) +%doc AUTHORS BUGS ChangeLog COPYING * IMO, useless for docs. * README is not maintained for years (just read it) and NEWS is symlink to ChangeLog. -%{_mandir}/man1/Xdialog.1* +%{_mandir}/man?/%{real_name}* * This is more general way how to play with man pages, don't have to care of every one page and of the section. -* Sat Apr 5 2008 Patrice Dumas <pertusus> 2.3.1-1 -- submit to fedora. +* Sat Apr 5 2008 Patrice Dumas <pertusus> - 2.3.1-1 +- Submit to Fedora. * Just some more consistency issues. -- Please see the output of rpmlint on arch dependent package (e.g. i386) you'll see lot of warning about +x on doc files: xdialog.i386: W: spurious-executable-perm /usr/share/doc/xdialog-2.3.1/samples/timebox * Change it to 0644 or erase them. -- Hope it's useful.
(In reply to comment #2) > -Release: 1%{dist} > +Release: 1%{?dist} > * The preferred dist tag is now ?dist. Thanks. Not only is it preferred, but it is bad to have %{dist} since it may not be defined. > -License: GPL+ > +License: GPLv2 > * License should be as concrete as possible, in source archive is GPLv2 I don't understand. All the files in src seems not to have any license header, which means GPL+. Am I missing something? > -URL: http://xdialog.dyns.net/ > +URL: http://xdialog.free.fr Indeed, looks like it is better to use xdialog.free.fr. > -BuildRoot: %{_tmppath}/%{name}-%{version}-%{release}-root > +BuildRoot: %(mktemp -ud %{_tmppath}/%{name}-%{version}-%{release}-XXXXXX) > > * Tip: this is #1 in "BuildRoot tag" section of > https://fedoraproject.org/wiki/Packaging/Guidelines I prefer a reproducible buildroot. I changed it to the 2nd BuildRoot tag, though. > BuildRequires: gtk+-devel >= 1.2.0 > > * Isn't it possible to completely get rid of GTK+v1? It's ugly, not widely > supported these days and adds build dependency. For this package it could have been the converse. As said above in Comment #1 the author prefers the gtk1 version. Besides build dependencies are not an issue. It is true that gtk1 is lacking some features, prominently utf8, still it is not a reason to leave it apart when upstream advertises to use it. > -%{__rm} -rf %{buildroot} > +rm -rf %{buildroot} > > * Be consistent, use command style OR macro style. > > -%defattr(-, root, root, 0755) > +%defattr(-, root, root, -) Both issues are not a big deal in my opinion, but changed anyway. > -%doc AUTHORS BUGS ChangeLog COPYING NEWS README > +%doc AUTHORS BUGS ChangeLog COPYING > > * IMO, useless for docs. > * README is not maintained for years (just read it) and NEWS is symlink to > ChangeLog. I'll leave the README, it has meaningful informations in it and the fact that it is no more maintained is plainly documented. > -%{_mandir}/man1/Xdialog.1* > +%{_mandir}/man?/%{real_name}* > > * This is more general way how to play with man pages, don't have to care of > every one page and of the section. I prefer listing files more precisely such that build fails if file name changes or new file appear. > -* Sat Apr 5 2008 Patrice Dumas <pertusus> 2.3.1-1 > -- submit to fedora. > +* Sat Apr 5 2008 Patrice Dumas <pertusus> - 2.3.1-1 > +- Submit to Fedora. > > * Just some more consistency issues. I prefer keeping the changelog of Dag/Dries like they want it to be formatted, but switch to my preferred format in fedora, so I'll leave it as is. > Please see the output of rpmlint on arch dependent package (e.g. i386) you'll > see lot of warning about +x on doc files: > > xdialog.i386: W: spurious-executable-perm > /usr/share/doc/xdialog-2.3.1/samples/timebox > > * Change it to 0644 or erase them. Nope, they are sample that can be run as is and are right to be there and executable, rpmlint cannot always be right. Thanks for the review. Are you waiting to be sponsored? Updated package: http://www.environnement.ens.fr/perso/dumas/fc-srpms/xdialog-2.3.1-2.fc10.src.rpm
I have tried a mock build and it gives the following error: Making all in po make[2]: Entering directory `/builddir/build/BUILD/Xdialog-2.3.1/po' test -z "fr.gmo de.gmo ru.gmo es.gmo hu.gmo pt_BR.gmo no_NO.gmo id.gmo nl.gmo it.gmo pl.gmo ca.gmo sv_SE.gmo" || make fr.gmo de.gmo ru.gmo es.gmo hu.gmo pt_BR.gmo no_NO.gmo id.gmo nl.gmo it.gmo pl.gmo ca.gmo sv_SE.gmo make[3]: Entering directory `/builddir/build/BUILD/Xdialog-2.3.1/po' rm -f fr.gmo && : -c --statistics -o fr.gmo fr.po rm -f de.gmo && : -c --statistics -o de.gmo de.po mv: cannot stat `t-de.gmo': No such file or directory rm -f ru.gmo && : -c --statistics -o ru.gmo ru.po mv: cannot stat `t-fr.gmo': No such file or directory make[3]: *** [de.gmo] Error 1 make[3]: *** Waiting for unfinished jobs.... make[3]: *** [fr.gmo] Error 1 mv: cannot stat `t-ru.gmo': No such file or directory make[3]: *** [ru.gmo] Error 1 make[3]: Leaving directory `/builddir/build/BUILD/Xdialog-2.3.1/po' make[2]: *** [stamp-po] Error 2 make[2]: Leaving directory `/builddir/build/BUILD/Xdialog-2.3.1/po' make[1]: *** [all-recursive] Error 1 make[1]: Leaving directory `/builddir/build/BUILD/Xdialog-2.3.1' make: *** [all] Error 2 error: Bad exit status from /var/tmp/rpm-tmp.71846 (%build) Adding gettext-devel as BR solves that. Once adding that I see no real blockers except for one small problem. On one hand, the guidelines require the presence of a desktop file. On the other hand this application (which works OK in Fedora 7, BTW) needs arguments to be provided to it, so a mere desktop file will be useless. Maybe we could add a comment to the spec specifying why no desktop file is included ?
(In reply to comment #4) > Adding gettext-devel as BR solves that. gettext is enough.
(In reply to comment #3) > I don't understand. All the files in src seems not to have any license > header, which means GPL+. Am I missing something? No, you are completely right. > Thanks for the review. Are you waiting to be sponsored? Yes, I am and I'd appreciate sponsorship.
(In reply to comment #6) > > Thanks for the review. Are you waiting to be sponsored? > > Yes, I am and I'd appreciate sponsorship. Could you please point me to other work you have done in fedora?
Added the getext BR and the comment: http://www.environnement.ens.fr/perso/dumas/fc-srpms/xdialog-2.3.1-3.fc10.src.rpm
Package Review ============== Key: - = N/A x = Check ! = Problem ? = Not evaluated === REQUIRED ITEMS === [x] Package is named according to the Package Naming Guidelines. [x] Spec file name must match the base package %{name}, in the format %{name}.spec. [x] Package meets the Packaging Guidelines. [x] Package successfully compiles and builds into binary rpms on at least one supported architecture. Tested on: devel/x86_64 [x] Rpmlint output: source RPM: empty binary RPM: 31 warnings triggered by the fact that all example files are marked executable; quite ugly at the first sight but since no additional dependency is pulled in + it is intentional to have the examples runable by default (as opposed to using "sh <example file>" ), I will not object [x] Package is not relocatable. [x] Buildroot is correct (%{_tmppath}/%{name}-%{version}-%{release}-root-%(%{__id_u} -n)) [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] License field in the package spec file matches the actual license. License type: GPL+ [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] Spec file is legible and written in American English. [x] Sources used to build the package matches the upstream source, as provided in the spec URL. SHA1SUM of package: 292c552506633c54a28d51aa290277b7b5c0c708 Xdialog-2.3.1.tar.bz2 [x] Package is not known to require ExcludeArch [x] All build dependencies are listed in BuildRequires, except for any that are listed in the exceptions section of Packaging Guidelines. [x] The spec file handles locales properly. [-] ldconfig called in %post and %postun if required. [x] Package must own all directories that it creates. [x] Package requires other packages for directories it uses. [x] Package does not contain duplicates in %files. [x] Permissions on files are set properly. [x] Package has a %clean section, which contains rm -rf %{buildroot} (or $RPM_BUILD_ROOT). [x] Package consistently uses macros. [x] Package contains code, or permissable content. [-] Large documentation files are in a -doc subpackage, if required. [x] Package uses nothing in %doc for runtime. [-] Header files in -devel subpackage, if present. [-] Static libraries in -devel subpackage, if present. [-] Package requires pkgconfig, if .pc files are present. [-] Development .so files in -devel subpackage, if present. [-] Fully versioned dependency in subpackages, if present. [x] Package does not contain any libtool archives (.la). [x] Package contains a properly installed %{name}.desktop file if it is a GUI application. It is a GUI, but requires a mandatory argument when run, so a desktop file to launch it would be useless. A comment specofying this aspect is included in the spec [x] Package does not own files or directories owned by other packages. === SUGGESTED ITEMS === [x] Latest version is packaged. [x] Package does not include license text files separate from upstream. [-] Description and summary sections in the package spec file contains translations for supported Non-English languages, if available. [x] Reviewer should test that the package builds in mock. Tested on: devel/x86_64 [x] Package should compile and build into binary rpms on all supported architectures. Tested on:devel/x86_64, F7/x86_64 [x] Package functions as described. [-] Scriptlets must be sane, if used. [-] The placement of pkgconfig(.pc) files is correct. [-] File based requires are sane. ================ *** APPROVED *** ================
Since the original sources do not really have a license specified (but only include the standard COPYING file), I suggest to act according to http://fedoraproject.org/wiki/Licensing/FAQ ("Now, keep in mind that most upstreams are probably leaving the versioning out by accident. If you get to case 4, you definitely want to let upstream know that you are unable to determine the applicable version(s) of the license from the source and documentation. They'll almost certainly let you know what their intended license version is, and (hopefully) correct it in the upstream source. ").
(In reply to comment #7) > (In reply to comment #6) > > Could you please point me to other work you have done in fedora? Hi. Recently I focused on * creation of new package for Fedora - awesome wm bug 452427 https://bugzilla.redhat.com/show_bug.cgi?id=452427 and informal package reviewing * https://bugzilla.redhat.com/show_bug.cgi?id=450816#c1 Fixed build flags to be same as Fedora default. * https://bugzilla.redhat.com/show_bug.cgi?id=452842#c1 Monotorrent server/client library * https://bugzilla.redhat.com/show_bug.cgi?id=442233#c4 * https://bugzilla.redhat.com/show_bug.cgi?id=450466#c15 clive - recently approved Fedora package * https://bugzilla.redhat.com/show_bug.cgi?id=452663#c1 * https://bugzilla.redhat.com/show_bug.cgi?id=438609#c1 Minor comments. * https://bugzilla.redhat.com/show_bug.cgi?id=446102#c2 ...which are you already aware of (this BZ). Michal (Sorry for posting it here but was not able to deliver it via email for few days.)
New Package CVS Request ======================= Package Name: xdialog Short Description: X11 drop in replacement for cdialog Owners: pertusus Branches: F-8 F-9 EL-4 EL-5 InitialCC: Cvsextras Commits: yes
cvs done.
xdialog-2.3.1-3.fc9 has been submitted as an update for Fedora 9
xdialog-2.3.1-3.fc8.1 has been submitted as an update for Fedora 8
Thanks both of you for the review. I'll have a look at awesome over the week end and I'll sponsor you is awesome review is completed.
(In reply to comment #16) > I'll have a look at awesome over the week end and I'll sponsor you > is awesome review is completed. Actually I've been sponsored for my work on khmeros-fonts (bug 454078), anyway I'll still appreciate awesome review.
xdialog-2.3.1-3.fc8.1 has been pushed to the Fedora 8 stable repository. If problems still persist, please make note of it in this bug report.
xdialog-2.3.1-3.fc9 has been pushed to the Fedora 9 stable repository. If problems still persist, please make note of it in this bug report.
Package Change Request ====================== Package Name: xdialog New Branches: el6 Owners: fantom InitialCC: Xdialog is a dependancy of easybashgui in el6 repository, I will import the srpm of fedora in the el6 branch and maintain it.
Git done (by process-git-requests).
xdialog-2.3.1-12.el6 has been submitted as an update for Fedora EPEL 6. https://admin.fedoraproject.org/updates/xdialog-2.3.1-12.el6
xdialog-2.3.1-12.el6 has been pushed to the Fedora EPEL 6 stable repository.