Bug 866901

Some initial comments after having quickly looked at the spec file:

- No %doc on the sources?
- Some lines are really long...
- Check if libfoo-devel requires libfoo, that would reduce the list of BuildRequires
- Check the guidelines for install .desktop files
- Check the guidelines regarding the icons (in %post and %postun)
- There is no comment if the patch has been submitted upstream, has it?

Hi Pierre-Yves,

- I have fill the %doc
- I have cut the long lignes :)
- I have look the requires of libxslt-devel, libxml2-devel
- I have change the .desktop install same in guidelines
- I have add the icons %post and %postun same in guidelines
- Upstream has been informed about the patch http://sourceforge.net/users/jouty/

Hi,

I'm trying to run "fedora-review", but I got this message:

----------------
$ fedora-review -b 866901
Processing review bug : 866901
Getting .spec and .srpm Urls from bug report : 866901
no SRPM file URL found in bug #866901
Cannot find any .spec and .srpm URLs in bugreport
----------------

Could you please add the new links in the ticket?

Thanks!

Hi Florancia,

Yes of course i add the new links  :

SPEC : http://jouty.fedorapeople.org/gogui.spec
SRPM : http://jouty.fedorapeople.org/gogui-1.4.6-1.fc17.src.rpm

Hi Christophe, 

Thanks for the update.
I run “fedora-review” and there are some tests that failed. Could you please check them?

----------------------------------
$ cat 866901/gogui-review.txt 

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

Key:
- = N/A
x = Pass
! = Fail
? = Not evaluated

==== Generic ====
[!]: MUST Package contains a properly installed %{name}.desktop using desktop-
     file-install file if it is a GUI application.
[!]: MUST Package consistently uses macros (instead of hard-coded directory
     names).
     Note: Using both %{buildroot} and $RPM_BUILD_ROOT
[!]: MUST Rpmlint output is silent.
[!]: MUST Sources used to build the package match the upstream source, as
     provided in the spec URL.
/home/Florencia/866901/gogui-1.4.6-src.tar.gz :
  MD5SUM this package     : a7cce6b4e314d0048f5e569e5fd43b73
  MD5SUM upstream package : 90da61b841b47a1c655c2a01205d2459

==== Java ====
[!]: MUST Javadocs are placed in %{_javadocdir}/%{name} (no -%{version}
     symlink)
     Note: No /usr/share/javadoc/gogui found

==== Maven ====
Issues:
[!]: MUST Javadocs are placed in %{_javadocdir}/%{name} (no -%{version}
     symlink)
     Note: No /usr/share/javadoc/gogui found
[!]: MUST Package contains a properly installed %{name}.desktop using desktop-
     file-install file if it is a GUI application.
[!]: MUST Package consistently uses macros (instead of hard-coded directory
     names).
     Note: Using both %{buildroot} and $RPM_BUILD_ROOT
[!]: MUST Rpmlint output is silent.
[!]: MUST Sources used to build the package match the upstream source, as
     provided in the spec URL.
----------------------------------

Complete fedora-review output:

-----------------
$ cat 866901/gogui-review.txt 

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

Key:
- = N/A
x = Pass
! = Fail
? = Not evaluated



==== Generic ====
[ ]: MUST Package is licensed with an open-source compatible license and meets
     other legal requirements as defined in the legal section of Packaging
     Guidelines.
[x]: MUST Package successfully compiles and builds into binary rpms on at
     least one supported primary architecture.
[x]: MUST All build dependencies are listed in BuildRequires, except for any
     that are listed in the exceptions section of Packaging Guidelines.
[x]: MUST Buildroot is not present
     Note: Unless packager wants to package for EPEL5 this is fine
[ ]: MUST Package contains no bundled libraries.
[ ]: MUST Changelog in prescribed format.
[x]: MUST Package has no %clean section with rm -rf %{buildroot} (or
     $RPM_BUILD_ROOT)
     Note: Clean would be needed if support for EPEL is required
[ ]: MUST Sources contain only permissible code or content.
[x]: MUST Each %files section contains %defattr if rpm < 4.4
     Note: Note: defattr macros not found. They would be needed for EPEL5
[ ]: MUST Macros in Summary, %description expandable at SRPM build time.
[!]: MUST Package contains a properly installed %{name}.desktop using desktop-
     file-install file if it is a GUI application.
[ ]: MUST Package requires other packages for directories it uses.
[ ]: MUST Package uses nothing in %doc for runtime.
[ ]: MUST Package is not known to require ExcludeArch.
[x]: MUST Permissions on files are set properly.
[x]: MUST Package does not contain duplicates in %files.
[x]: MUST Spec file lacks Packager, Vendor, PreReq tags.
[x]: MUST Package does not run rm -rf %{buildroot} (or $RPM_BUILD_ROOT) at the
     beginning of %install.
     Note: rm -rf would be needed if support for EPEL5 is required
[x]: MUST 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.
[ ]: MUST License field in the package spec file matches the actual license.
[ ]: MUST License file installed when any subpackage combination is installed.
[!]: MUST Package consistently uses macros (instead of hard-coded directory
     names).
     Note: Using both %{buildroot} and $RPM_BUILD_ROOT
[ ]: MUST Package meets the Packaging Guidelines.
[x]: MUST Package is named according to the Package Naming Guidelines.
[ ]: MUST Package does not generates any conflict.
[ ]: MUST Package obeys FHS, except libexecdir and /usr/target.
[ ]: MUST Package must own all directories that it creates.
[ ]: MUST Package does not own files or directories owned by other packages.
[ ]: MUST Package installs properly.
[ ]: MUST Requires correct, justified where necessary.
[!]: MUST Rpmlint output is silent.

rpmlint gogui-1.4.6-1.fc19.noarch.rpm

gogui.noarch: I: enchant-dictionary-not-found fr_FR
gogui.noarch: W: non-standard-group Unspecified
1 packages and 0 specfiles checked; 0 errors, 1 warnings.


rpmlint gogui-javadoc-1.4.6-1.fc19.noarch.rpm

gogui-javadoc.noarch: I: enchant-dictionary-not-found fr_FR
gogui-javadoc.noarch: W: non-standard-group Unspecified
1 packages and 0 specfiles checked; 0 errors, 1 warnings.


rpmlint gogui-1.4.6-1.fc19.src.rpm

gogui.src: I: enchant-dictionary-not-found fr_FR
gogui.src: W: non-standard-group Unspecified
gogui.src:103: E: files-attr-not-set
gogui.src:104: E: files-attr-not-set
gogui.src:105: E: files-attr-not-set
gogui.src:106: E: files-attr-not-set
gogui.src:107: E: files-attr-not-set
gogui.src:108: E: files-attr-not-set
gogui.src:109: E: files-attr-not-set
gogui.src:112: E: files-attr-not-set
gogui.src: W: no-cleaning-of-buildroot %install
gogui.src: W: no-cleaning-of-buildroot %clean
gogui.src: W: no-buildroot-tag
gogui.src: W: no-%clean-section
1 packages and 0 specfiles checked; 8 errors, 5 warnings.


[!]: MUST Sources used to build the package match the upstream source, as
     provided in the spec URL.
/home/Florencia/866901/gogui-1.4.6-src.tar.gz :
  MD5SUM this package     : a7cce6b4e314d0048f5e569e5fd43b73
  MD5SUM upstream package : 90da61b841b47a1c655c2a01205d2459

[ ]: MUST Spec file is legible and written in American English.
[x]: MUST Spec file name must match the spec package %{name}, in the format
     %{name}.spec.
[ ]: MUST Package contains a SysV-style init script if in need of one.
[x]: MUST File names are valid UTF-8.
[x]: SHOULD Reviewer should test that the package builds in mock.
[ ]: SHOULD 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]: SHOULD Dist tag is present.
[ ]: SHOULD No file requires outside of /etc, /bin, /sbin, /usr/bin,
     /usr/sbin.
[ ]: SHOULD Final provides and requires are sane (rpm -q --provides and rpm -q
     --requires).
[ ]: SHOULD Package functions as described.
[ ]: SHOULD Package does not include license text files separate from
     upstream.
[ ]: SHOULD Patches link to upstream bugs/comments/lists or are otherwise
     justified.
[ ]: SHOULD Scriptlets must be sane, if used.
[!]: SHOULD SourceX / PatchY prefixed with %{name}.
     Note: Patch0: manifest.patch (manifest.patch)
[x]: SHOULD SourceX is a working URL.
[ ]: SHOULD Description and summary sections in the package spec file contains
     translations for supported Non-English languages, if available.
[ ]: SHOULD Package should compile and build into binary rpms on all supported
     architectures.
[ ]: SHOULD %check is present and all tests pass.
[ ]: SHOULD Packages should try to preserve timestamps of original installed
     files.
[x]: SHOULD Spec use %global instead of %define.


==== Java ====
[ ]: MUST If source tarball includes bundled jar/class files these need to be
     removed prior to building
[x]: MUST Packages have proper BuildRequires/Requires on jpackage-utils
[x]: MUST Fully versioned dependency in subpackages, if present.
[x]: MUST Javadoc documentation files are generated and included in -javadoc
     subpackage
[x]: MUST Javadoc subpackages have Requires: jpackage-utils
[!]: MUST Javadocs are placed in %{_javadocdir}/%{name} (no -%{version}
     symlink)
     Note: No /usr/share/javadoc/gogui found
[ ]: SHOULD Package has BuildArch: noarch (if possible)
[ ]: SHOULD Package uses upstream build method (ant/maven/etc.)


==== Maven ====
[x]: MUST Old add_to_maven_depmap macro is not being used
[ ]: MUST If package contains pom.xml files install it (including depmaps)
     even when building with ant

Issues:
[!]: MUST Javadocs are placed in %{_javadocdir}/%{name} (no -%{version}
     symlink)
     Note: No /usr/share/javadoc/gogui found
[!]: MUST Package contains a properly installed %{name}.desktop using desktop-
     file-install file if it is a GUI application.
[!]: MUST Package consistently uses macros (instead of hard-coded directory
     names).
     Note: Using both %{buildroot} and $RPM_BUILD_ROOT
[!]: MUST Rpmlint output is silent.

rpmlint gogui-1.4.6-1.fc19.noarch.rpm

gogui.noarch: I: enchant-dictionary-not-found fr_FR
gogui.noarch: W: non-standard-group Unspecified
1 packages and 0 specfiles checked; 0 errors, 1 warnings.


rpmlint gogui-javadoc-1.4.6-1.fc19.noarch.rpm

gogui-javadoc.noarch: I: enchant-dictionary-not-found fr_FR
gogui-javadoc.noarch: W: non-standard-group Unspecified
1 packages and 0 specfiles checked; 0 errors, 1 warnings.


rpmlint gogui-1.4.6-1.fc19.src.rpm

gogui.src: I: enchant-dictionary-not-found fr_FR
gogui.src: W: non-standard-group Unspecified
gogui.src:103: E: files-attr-not-set
gogui.src:104: E: files-attr-not-set
gogui.src:105: E: files-attr-not-set
gogui.src:106: E: files-attr-not-set
gogui.src:107: E: files-attr-not-set
gogui.src:108: E: files-attr-not-set
gogui.src:109: E: files-attr-not-set
gogui.src:112: E: files-attr-not-set
gogui.src: W: no-cleaning-of-buildroot %install
gogui.src: W: no-cleaning-of-buildroot %clean
gogui.src: W: no-buildroot-tag
gogui.src: W: no-%clean-section
1 packages and 0 specfiles checked; 8 errors, 5 warnings.


[!]: MUST Sources used to build the package match the upstream source, as
     provided in the spec URL.
/home/Florencia/866901/gogui-1.4.6-src.tar.gz :
  MD5SUM this package     : a7cce6b4e314d0048f5e569e5fd43b73
  MD5SUM upstream package : 90da61b841b47a1c655c2a01205d2459



Generated by fedora-review 0.1.2
External plugins:
-----------------

Running fedora-review by itself without going through the checklist at the end and check the point that are remaining (and making sure there are no false positive/negative) is pretty much useless.

Please don't do that, either do a full review (helped with feora-review) or just report the error but don't just paste the output of fedora-review without checking it.

That said, pointing out mistakes is always good to do, we are all subject at making some ;-)

Hi Florencia, Pierre-Yves

[!]: MUST Package contains a properly installed %{name}.desktop using desktop-
     file-install file if it is a GUI application.
=> The desktop file name follow now the guidelines %{name}.desktop

[!]: MUST Package consistently uses macros (instead of hard-coded directory
     names).
     Note: Using both %{buildroot} and $RPM_BUILD_ROOT
=> I have solved this issue from the install desktop entry and change hard path from the %post and %postun too.

[!]: MUST Rpmlint output is silent.
=> I have just the enchant-dictionary-not-found fr_FR which is normal

[!]: MUST Sources used to build the package match the upstream source, as
     provided in the spec URL.
/home/Florencia/866901/gogui-1.4.6-src.tar.gz :
  MD5SUM this package     : a7cce6b4e314d0048f5e569e5fd43b73
  MD5SUM upstream package : 90da61b841b47a1c655c2a01205d2459
=> The Source0 url has been changed according sourceforge's url in the guidelines 

==== Java ====
[!]: MUST Javadocs are placed in %{_javadocdir}/%{name} (no -%{version}
     symlink)
     Note: No /usr/share/javadoc/gogui found
=>[root@pollux gogui]# pwd
/var/lib/mock/fedora-rawhide-x86_64/root/builddir/build/BUILDROOT/gogui-1.4.6-2.fc19.x86_64/usr/share/javadoc/gogui
[root@pollux gogui]# ll
total 812
drwxr-xr-x. 4 builder mock   4096 23 oct.  16:55 .
drwxr-xr-x. 3 builder mock   4096 23 oct.  16:55 ..
-rw-r--r--. 1 builder mock  25487 23 oct.  16:55 allclasses-frame.html
-rw-r--r--. 1 builder mock  21667 23 oct.  16:55 allclasses-noframe.html
-rw-r--r--. 1 builder mock   8523 23 oct.  16:55 constant-values.html
-rw-r--r--. 1 builder mock   4235 23 oct.  16:55 deprecated-list.html
-rw-r--r--. 1 builder mock   8030 23 oct.  16:55 help-doc.html
-rw-r--r--. 1 builder mock 598749 23 oct.  16:55 index-all.html
-rw-r--r--. 1 builder mock   1497 23 oct.  16:55 index.html
drwxr-xr-x. 3 builder mock   4096 23 oct.  16:55 net
-rw-r--r--. 1 builder mock   2611 23 oct.  16:55 overview-frame.html
-rw-r--r--. 1 builder mock   7662 23 oct.  16:55 overview-summary.html
-rw-r--r--. 1 builder mock  45091 23 oct.  16:55 overview-tree.html
-rw-r--r--. 1 builder mock    403 23 oct.  16:55 package-list
drwxr-xr-x. 2 builder mock   4096 23 oct.  16:55 resources
-rw-r--r--. 1 builder mock  45659 23 oct.  16:55 serialized-form.html
-rw-r--r--. 1 builder mock  11139 23 oct.  16:55 stylesheet.css

The directory /usr/share/javadoc/gogui is here 

=> I have correct the directory ownership in %files section
and update %changelog section
=> I have change the patch name and comment it in spec file with upstream url

New link :

SPEC : http://jouty.fedorapeople.org/gogui.spec
SRPM : http://jouty.fedorapeople.org/gogui-1.4.6-2.fc17.src.rpm

I have change the spec file and rebuilt due to :
http://jouty.fedorapeople.org/upstream-info

changing xgd to install command (see changelogs)

here new links :

SPEC : http://jouty.fedorapeople.org/gogui.spec
SRPM : http://jouty.fedorapeople.org/gogui-1.4.6-3.fc17.src.rpm

For info some change have been made due to :

http://jouty.fedorapeople.org/upstream-info

I think we can still improve this:

install -d $RPM_BUILD_ROOT%{_datadir}/thumbnailers
cat config/gogui.thumbnailer | sed "s;/usr/bin/gogui-thumbnailer;$PREFIX/bin/gogui-thumbnailer;" \
> $RPM_BUILD_ROOT%{_prefix}/share/thumbnailers/gogui.thumbnailer

- consistent use of %{_datadir}
- put the sed in %prep
- use install to install the file as you did for the others

- The /usr/share has been changed with the macro %{_datadir}
- The sed has been push in %prep section
- The installation for thumbnailer is now with install command

New links :

SPEC : http://jouty.fedorapeople.org/gogui.spec
SRPM : http://jouty.fedorapeople.org/gogui-1.4.6-4.fc17.src.rpm

Hello,

Some informal comments:

1)  You can use %{name}.desktop instead of gogui.desktop.

2)  Regarding rpmlint output, just some comments, no action needed:

--------------
gogui.noarch: I: enchant-dictionary-not-found fr_FR
gogui.noarch: W: non-standard-group Unspecified
--------------

Details:
===>>> gogui.noarch: I: enchant-dictionary-not-found fr_FR 
A dictionary for the Enchant spell checking library is not available for the
language given in the info message.  Spell checking will proceed with
rpmlint's built-in implementation for localized tags in this language.

===>>> gogui.noarch: W: non-standard-group Unspecified
All current versions of Fedora (and their respective RPM versions) treat the Group tag as optional. Packages may include a Group: field for compatibility with EPEL, but are not required to do so.
https://fedoraproject.org/wiki/Packaging:Guidelines?rd=Packaging/Guidelines#Group_tag

3) rpmlint output for .src.rpm is:

-------------
gogui.src: I: enchant-dictionary-not-found fr_FR
gogui.src: W: non-standard-group Unspecified
gogui.src:115: E: files-attr-not-set
gogui.src:116: E: files-attr-not-set
gogui.src:117: E: files-attr-not-set
gogui.src:118: E: files-attr-not-set
gogui.src:119: E: files-attr-not-set
gogui.src:120: E: files-attr-not-set
gogui.src:121: E: files-attr-not-set
gogui.src:122: E: files-attr-not-set
gogui.src:123: E: files-attr-not-set
gogui.src:124: E: files-attr-not-set
gogui.src:125: E: files-attr-not-set
gogui.src:128: E: files-attr-not-set
gogui.src: W: no-cleaning-of-buildroot %install
gogui.src: W: no-cleaning-of-buildroot %clean
gogui.src: W: no-buildroot-tag
gogui.src: W: no-%clean-section
1 packages and 0 specfiles checked; 12 errors, 5 warnings.
-------------

Details:

===>>> gogui.src:115: E: files-attr-not-set
This is now the default and no longer necessary to explicitly include.
https://fedoraproject.org/wiki/Packaging:Guidelines?rd=Packaging/Guidelines#File_Permissions


===>>> gogui.src: W: no-cleaning-of-buildroot %install
It’s only necessary for F-12 and below or EPEL 5.
https://fedoraproject.org/wiki/Packaging:Guidelines?rd=Packaging/Guidelines#.25clean

===>>> gogui.src: W: no-buildroot-tag
The BuildRoot tag isn't used in your spec. It must be used in order to allow
building the package as non root on some systems. For some rpm versions (e.g.
rpm.org >= 4.6) the BuildRoot tag is not necessary in specfiles and is ignored
by rpmbuild; if your package is only going to be built with such rpm versions
you can ignore this warning.

Thanks Florencia for the informal comments :

1) I have change some gogui with the %{name} macro (see changelog)

2) The dictionary message is normal, the group tag isn't need
https://fedoraproject.org/wiki/Packaging:Guidelines?rd=Packaging/Guidelines#Group_tag

3) I haven't %defattr in the spec file about the files-attr-not-set error
my rpmlint ( rpmlint-1.4-10.fc17.src.rpm) doesn't put this output error
Concerning the other warnings there are only used with EPEL.

New links :

SPEC : http://jouty.fedorapeople.org/gogui.spec
SRPM : http://jouty.fedorapeople.org/gogui-1.4.6-5.fc17.src.rpm

Some comments too:

1) as I told you recently on IRC, sentences shouldn't be used in Summary; "Graphical user interface to programs that play the board game Go" may be enough.

2) On the contrary, use real sentences to describe the role of gogui in Description; by the way, here is an improved French description (for example, don't literally translate "Go Text", it's a standard: http://fr.wikipedia.org/wiki/Go_Text_Protocol):

%description
Gogui is a graphical interface to programs that play
the game of Go and use the Go Text Protocol (GTP), 
such as GNU Go. GoGui has special features 
that are useful for Go program developers.

%description -l fr_FR
Gogui est une interface graphique pour les programmes de go implémentant le protocal Go Text Protocol (GTP), tels que GNU Go. Gogui présente des fonctionnalités utiles aux concepteurs de programmes Go.

3) You don't have to specify BuildRequires in sub-packages (see javadoc subpackage). BuildRequires must only be specified on the main package header section.

4) DON'T pipe sed through cat, it's useless, unreadable and considered bad practice. Simply use sed only instead:

for FILE in bin/*; do
	if [ -f $FILE -a -x $FILE ]; then
		sed -e "s;^GOGUI_LIB=.*;GOGUI_LIB=\"%{_javadir}/%{name}\";" \
		-e "s;^JAVA_DEFAULT=.*;JAVA_DEFAULT=\"$JAVA_DEFAULT\";" \
		> $RPM_BUILD_ROOT%{_prefix}/$FILE
		chmod a+x $RPM_BUILD_ROOT%{_prefix}/$FILE
	fi
done

5) You don't have to "manually" install documentation in %install target; use the %doc macro instead in files:
- Remove this line:
install -pm 644 doc/manual/html/*.html $RPM_BUILD_ROOT%{_defaultdocdir}/%{name}
- Install HTML documentation in %files as below:
%files
...
%doc COPYING.html README.html doc/manual/html/*.html

> 4) DON'T pipe sed through cat, it's useless, unreadable and considered bad
> practice. Simply use sed only instead:
> 
> for FILE in bin/*; do
> 	if [ -f $FILE -a -x $FILE ]; then
> 		sed -e "s;^GOGUI_LIB=.*;GOGUI_LIB=\"%{_javadir}/%{name}\";" \
> 		-e "s;^JAVA_DEFAULT=.*;JAVA_DEFAULT=\"$JAVA_DEFAULT\";" \
> 		> $RPM_BUILD_ROOT%{_prefix}/$FILE
> 		chmod a+x $RPM_BUILD_ROOT%{_prefix}/$FILE
> 	fi
> done
Mistake, use this instead:

 for FILE in bin/*; do
 	if [ -f $FILE -a -x $FILE ]; then
 		sed -e "s;^GOGUI_LIB=.*;GOGUI_LIB=\"%{_javadir}/%{name}\";" \
 		-e "s;^JAVA_DEFAULT=.*;JAVA_DEFAULT=\"$JAVA_DEFAULT\";" $FILE \
 		> $RPM_BUILD_ROOT%{_prefix}/$FILE
 		chmod a+x $RPM_BUILD_ROOT%{_prefix}/$FILE
 	fi
 done

Tanks for the help Mohamed

- I have change the Summary and Description as your recommendations
- The Buildrequires have been delete from javadoc subpackage
- The sed syntax has been change without the cat
- The manual files are installed through the %doc section and not manually anymore

New links :

SPEC : http://jouty.fedorapeople.org/gogui.spec
SRPM : http://jouty.fedorapeople.org/gogui-1.4.6-6.fc17.src.rpm

>- The sed syntax has been change without the cat
Almost: 
  cat config/%{name}.thumbnailer | sed "s;/usr/bin/%{name}-thumbnailer;$PREFIX/bin/%{name}-thumbnailer;"

I have change the sed in the prep section without cat too

New links :

SPEC : http://jouty.fedorapeople.org/gogui.spec
SRPM : http://jouty.fedorapeople.org/gogui-1.4.6-7.fc17.src.rpm

There is something missing in the javadoc package wrt to license ;-)

I have add the licence file to the subpackage

New links :

SPEC : http://jouty.fedorapeople.org/gogui.spec
SRPM : http://jouty.fedorapeople.org/gogui-1.4.6-8.fc17.src.rpm

Koji build :

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

While reading the log from the build, it seems that ant -p is missing a parameters (and thus uses a default), but it might be safer to add it.

Spec has been updated with your recommendations (see changelog)

New links :

SPEC : http://jouty.fedorapeople.org/gogui.spec
SRPM : http://jouty.fedorapeople.org/gogui-1.4.6-9.fc17.src.rpm

Spec has been updated with your recommendations (see changelog)

New links :

SPEC : http://jouty.fedorapeople.org/gogui.spec
SRPM : http://jouty.fedorapeople.org/gogui-1.4.6-10.fc17.src.rpm

Ok so here is the official review:

Basically, it looks all good to go, the only question is:
- Should gogui require gnugo?
One can use it without gnugo installed in a multiplayer setting but for single played gnugo would be one way to go.
Are there other program in Fedora with which one could play gogui in a single player mode?

What do you think?

Let's discuss this point and I'll approve the package and you as a packager.
Sorry for taking so long though :-s

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


===== 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]: Fully versioned dependency in subpackages, if present.
[x]: Package complies to the Packaging Guidelines
[x]: License field in the package spec file matches the actual license.
[x]: License file installed when any subpackage combination is installed.
[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.
[x]: Large documentation must go in a -doc subpackage.
[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]: Spec file lacks Packager, Vendor, PreReq tags.
[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]: 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: No rpmlint messages.

Java:
[x]: Packages have proper BuildRequires/Requires on jpackage-utils
[x]: Fully versioned dependency in subpackages, if present.
[x]: Javadoc documentation files are generated and included in -javadoc
     subpackage
[x]: Javadoc subpackages have Requires: jpackage-utils
[x]: Javadocs are placed in %{_javadocdir}/%{name} (no -%{version} symlink)
[x]: Bundled jar/class files should be removed before build

Maven:
[-]: If package contains pom.xml files install it (including depmaps) even
     when building with ant
[x]: Old add_to_maven_depmap macro is not being used

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

Generic:
[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]: 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.
[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.
[x]: %check is present and all tests pass.
[x]: Packages should try to preserve timestamps of original installed files.
[x]: update-mime-database is invoked as required
[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.

Java:
[x]: Package has BuildArch: noarch (if possible)
[x]: Package uses upstream build method (ant/maven/etc.)

===== 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: No rpmlint messages.
[x]: Spec file according to URL is the same as in SRPM.


Rpmlint
-------
Checking: gogui-1.4.6-10.fc17.noarch.rpm
          gogui-javadoc-1.4.6-10.fc17.noarch.rpm
gogui.noarch: I: enchant-dictionary-not-found fr_FR
2 packages and 0 specfiles checked; 0 errors, 0 warnings.

Rpmlint (installed packages)
----------------------------
# rpmlint gogui-javadoc gogui
gogui-javadoc.noarch: I: enchant-dictionary-not-found fr_FR
2 packages and 0 specfiles checked; 0 errors, 0 warnings.
# echo 'rpmlint-done:'



MD5-sum check
-------------
http://downloads.sourceforge.net/gogui/gogui-1.4.6.zip :
  CHECKSUM(SHA256) this package     : 0ca99fcb0957d077e691bd65861d17761bedf14144cc9820d2b2e71a096f9fe3
  CHECKSUM(SHA256) upstream package : 0ca99fcb0957d077e691bd65861d17761bedf14144cc9820d2b2e71a096f9fe3

Hi Pierre-Yves,

Gogui don't require gnugo 
You can use GoGui without any Go engine as a SGF viewer/editor or as a
local Go board for playing games between humans.
However its main use is as a GUI for Go engines so i have include a new patch who add a second desktop for gnugo if he is installed.
So user can use it or not.

New links :

SPEC : http://jouty.fedorapeople.org/gogui.spec
SRPM : http://jouty.fedorapeople.org/gogui-1.4.6-11.fc17.src.rpm

(In reply to comment #27)
> Gogui don't require gnugo 
> You can use GoGui without any Go engine as a SGF viewer/editor or as a
> local Go board for playing games between humans.
> However its main use is as a GUI for Go engines so i have include a new
> patch who add a second desktop for gnugo if he is installed.
I don't think this patch is a good idea, unless you explicitely set gnugo as a Requires. What would happen if I launched gogui through this new desktop entry, without having gnugo installed?
Such a patch is probably superfluous for gogui. The description of gogui is clear enough to imply it's GTP client, after all.

By the way, as I told you several weeks ago on IRC, please use macros provided by jpackage-utils to generate executable wrappers:
   https://fedoraproject.org/wiki/Packaging:Java#Wrapper_Scripts
To my opinion, it's safer and easier to maintain than patching the launcher scripts provided upstream with complicated sed queries.

More comments:
  >>> %description -l fr_FR
"fr_FR" is maybe a little bit too restrictive, "fr" may be enough. I don't think your package description is specific to France.

  >>> BuildRequires:	... libxslt-devel, libxml2-devel ...
Are those BR really required? They're quite strange for a Java program which doesn't build JNI bits.

(In reply to comment #28)
> (In reply to comment #27)
> > Gogui don't require gnugo 
> > You can use GoGui without any Go engine as a SGF viewer/editor or as a
> > local Go board for playing games between humans.
> > However its main use is as a GUI for Go engines so i have include a new
> > patch who add a second desktop for gnugo if he is installed.
> I don't think this patch is a good idea

I agree on this, I think providing the .desktop file as Source1 would be nicer.

> unless you explicitely set gnugo as
> a Requires. What would happen if I launched gogui through this new desktop
> entry, without having gnugo installed?

If my reading of the description of the TryExec key in the desktop file is correct, nothing should happen.

(source: http://standards.freedesktop.org/autostart-spec/autostart-spec-latest.html)

> By the way, as I told you several weeks ago on IRC, please use macros
> provided by jpackage-utils to generate executable wrappers:
>    https://fedoraproject.org/wiki/Packaging:Java#Wrapper_Scripts
> To my opinion, it's safer and easier to maintain than patching the launcher
> scripts provided upstream with complicated sed queries.

Not a blocker, but I clearly agree on this to :)

- Delete patch gnugo desktop because upstream would prefer gogui package alone
- Remove fr_FR and just let fr
- Delete BR libxslt-devel, libxml2-devel

New link :

SPEC : http://jouty.fedorapeople.org/gogui.spec
SRPM : http://jouty.fedorapeople.org/gogui-1.4.6-12.fc17.src.rpm

@Mohamed El Morabity

I have try the %jpackage_script but this hang on at startup 

Could you please look with me if i do something wrong?

Link with jpackage :


SPEC : http://jouty.fedorapeople.org/jpackage/gogui.spec
SRPM : http://jouty.fedorapeople.org/jpackage/gogui-1.4.6-12.fc17.src.rpm

[root@pollux bin]# ./gogui
/usr/bin/build-classpath: error: Could not find /usr/share/java/gogui/gogui.jar Java extension for this JVM
/usr/bin/build-classpath: error: Some specified jars were not found
Erreur : impossible de trouver ou charger la classe principale net.sf.gogui.gogui
[root@pollux bin]# ll /usr/share/java/gogui/gogui.jar
-rw-r--r--. 1 root root 964204  7 févr. 16:06 /usr/share/java/gogui/gogui.jar

Hi,

I have change the sed with the Wrapper scripts as your recommendations:
See changelogs

New link :

SPEC : http://jouty.fedorapeople.org/gogui.spec
SRPM : http://jouty.fedorapeople.org/gogui-1.4.6-13.fc17.src.rpm

Add rm for class files
See changelogs:

New link :

SPEC : http://jouty.fedorapeople.org/gogui.spec
SRPM : http://jouty.fedorapeople.org/gogui-1.4.6-14.fc17.src.rpm

Well this looks all good.

This package is APPROVED.

Christophe, I will now sponsor you in the packager group, congratulations!
You can now do formal review to approve packages from other packagers. You will not be able to approve new packagers though.

Use your power wisely and keep your package in good shape.

You may now follow the procedures from: 
https://fedoraproject.org/wiki/Join_the_package_collection_maintainers#Add_Package_to_Source_Code_Management_.28SCM.29_system_and_Set_Owner

New Package SCM Request
=======================
Package Name: gogui
Short Description: Graphical user interface to programs that play the board game Go
Owners: jouty
Branches: f17 f18

fas email address and bugzilla email address need to match.

Ah, that's why Christophe couldn't set the flag himself. Sorry I missed this.

New Package SCM Request
=======================
Package Name: gogui
Short Description: Graphical user interface to programs that play the board game Go
Owners: jouty
Branches: f17 f18

Git done (by process-git-requests).

gogui-1.4.6-14.fc17 has been submitted as an update for Fedora 17.
https://admin.fedoraproject.org/updates/gogui-1.4.6-14.fc17

gogui-1.4.6-14.fc18 has been submitted as an update for Fedora 18.
https://admin.fedoraproject.org/updates/gogui-1.4.6-14.fc18

gogui-1.4.6-14.fc17 has been pushed to the Fedora 17 testing repository.

gogui-1.4.6-14.fc17 has been pushed to the Fedora 17 stable repository.

gogui-1.4.6-14.fc18 has been pushed to the Fedora 18 stable repository.

gogui-1.4.7-1.fc18 has been submitted as an update for Fedora 18.
https://admin.fedoraproject.org/updates/gogui-1.4.7-1.fc18

gogui-1.4.7-1.fc17 has been submitted as an update for Fedora 17.
https://admin.fedoraproject.org/updates/gogui-1.4.7-1.fc17

gogui-1.4.7-1.fc19 has been submitted as an update for Fedora 19.
https://admin.fedoraproject.org/updates/gogui-1.4.7-1.fc19