Product: Fedora https://bugzilla.redhat.com/show_bug.cgi?id=853922 --- Comment #15 from Simone Caronni <negativo17@xxxxxxxxx> --- > [!]: Package requires other packages for directories it uses. > >>>> You are using directories owned by tomcat (e. g. the line 85 or 99). > >>>> You need to put Requires: tomcat to your spec file. That also makes > >>>> the creation of the folders redundant. Corrected. > [!]: Requires correct, justified where necessary. > >>>> The packages you mention in 'how to test' and packages owning files that > >>>> you link to (line 78+ in guacamole.spec) must be mentioned in Requires, > >>>> so that they are installed automatically. That is the point of having > >>>> things packaged in the first place. Please, put all the required packages > >>>> into Requires. Corrected, I've added log4j and tomcat-native (comment #6); while the Java requirements are already in the SPEC file. The "guacd" daemon can be on another host and there can be multiple web interfaces for one daemon. So it should not be included as Requirement. You can see the configuration in guacamole.properties. See also Package Description below, I added a note where to look for the centralized server. > [!]: Package obeys FHS, except libexecdir and /usr/target. > >>>> So, until the Web App Guidelines > >>>> are approved and in place, I hereby ask you to refrain from installing > >>>> anything in the /usr/share/webapps directory. Please use only > >>>> /var/lib/tomcat/webapps. Done; I also moved it up one level, see comment #2 and #3 from /usr/share/webapps/guacamole/guacamole/ to /var/lib/tomcat/webapps/guacamole/ > >>>> Also, I am not convinced that the file guacamole.properties should be > >>>> installed in /usr/share/tomcat/lib. For one, it's not a library, and > >>>> since stuff in /usr is read-only, it makes no sense to have a config > >>>> file in there. Having the file in /etc/guacamole should be enough. Done, I made a mistake by linking it 2 times in the SPEC file. It is now only located in /etc/guacamole/ and linked in /var/lib/tomcat/webapps/guacamole/WEB-INF/classes/ > [!]: If package contains pom.xml files install it (including depmaps) even > when building with ant > >>>> Pom.xml file is present, but is not installed. Please, install it. Done, I put only the pom.xml file as %add_maven_depmap throws an error. > [!]: Javadocs are placed in %{_javadocdir}/%{name} (no -%{version} symlink) > >>>> Line 96 should not copy the apidocs folder, but its contents: > >>>> cp -rp target/site/apidocs/* %{buildroot}%{javadocdir}/%{name}/ > >>>> Currently there is the unnecessary directory apidocs in the path. Corrected, but the Packaging Guidelines at https://fedoraproject.org/wiki/Packaging:Java#maven_3 suggest to copy directly the directory: mkdir -p $RPM_BUILD_ROOT%{_javadocdir}/%{name} cp -rp [javadoc directory] $RPM_BUILD_ROOT%{_javadocdir}/%{name} I need to change it also in guacamole-common and guacamole-ext. Is it correct? > [!]: Packages should try to preserve timestamps of original installed files. > >>>> You are copying files without the -p switch, which preserves timestamps. > >>>> It is not mandatory, but it would be nice having it. Corrected. > [ ]: Package Description > >>>> It would be nice if the description didn't start with the word Guacamole. Corrected, example taken from package kernel. Added note for centralized server. -- You are receiving this mail because: You are on the CC list for the bug. Unsubscribe from this bug https://bugzilla.redhat.com/token.cgi?t=OktDD46Vo3&a=cc_unsubscribe _______________________________________________ package-review mailing list package-review@xxxxxxxxxxxxxxxxxxxxxxx https://admin.fedoraproject.org/mailman/listinfo/package-review