[Bug 915144] Review Request: rasmol - Molecular Graphics Visualization Tool
https://bugzilla.redhat.com/show_bug.cgi?id=915144
Björn besser82 Esser bjoern.es...@gmail.com changed:
What|Removed |Added
CC|package-review@lists.fedora |
|project.org |
Flags|needinfo?(bjoern.esser@gmai |
|l.com) |
--- Comment #31 from Björn besser82 Esser bjoern.es...@gmail.com ---
Sorry for the huge delay here. Your package has some small issues left.
#
Package Review
==
Legend:
[x] = Pass
[!] = Fail
[-] = Not applicable
[?] = Not evaluated
= 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.
[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.
--- License is fine.
[!]: License file installed when any subpackage combination is installed.
--- the -doc pkg misses some of the license-files. In fact the files
named NOTICE and RASLIC carry some imported notes on license, too.
[x]: If the package is under multiple licenses, the licensing breakdown must
be documented in the spec.
[!]: Package must own all directories that it creates.
Note: Directories without known owners: /usr/share/icons/hicolor,
/usr/share/icons/hicolor/48x48, /usr/share/icons/hicolor/48x48/apps
--- your package MUST Requires: hicolor-icon-theme
[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]: gtk-update-icon-cache is invoked in %postun and %posttrans if package
contains icons.
[x]: Useful -debuginfo package or justification otherwise.
[x]: Package is not known to require an ExcludeArch tag.
[!]: Large documentation must go in a -doc subpackage. Large could be size
(~1MB) or number of files.
Note: Documentation size is 481280 bytes in 20 files.
--- the ChangeLog-folder just contains split copies of ChangeLog.txt,
so you can purge it from being included. The whole %doc in the
arched pkg is about the same size as the binaries within. I'd
recommend to move all, but GPL, NOTICE, RASLIC, to the -doc pkg.
You will need to include NOTICE and RASLIC into the -doc, too,
because these files carry some important informations about
licensing/copyright as well.
[!]: Package complies to the Packaging Guidelines
--- issues are present
[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).
[x]: Package requires other packages for directories it uses.
[x]: Package does not own files or directories owned by other packages.
[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]: 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 or desktop-
file-validate if there is such a file.
[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
[Bug 915144] Review Request: rasmol - Molecular Graphics Visualization Tool
https://bugzilla.redhat.com/show_bug.cgi?id=915144 --- Comment #30 from Dmitrij S. Kryzhevich kr...@land.ru --- Ping ^2 ? -- 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=YHNlqRkveMa=cc_unsubscribe ___ package-review mailing list package-review@lists.fedoraproject.org https://admin.fedoraproject.org/mailman/listinfo/package-review
[Bug 915144] Review Request: rasmol - Molecular Graphics Visualization Tool
https://bugzilla.redhat.com/show_bug.cgi?id=915144 Dmitrij S. Kryzhevich kr...@land.ru changed: What|Removed |Added Flags||needinfo?(bjoern.esser@gmai ||l.com) --- Comment #29 from Dmitrij S. Kryzhevich kr...@land.ru --- Ping? -- 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=aEbhmcxqIta=cc_unsubscribe ___ package-review mailing list package-review@lists.fedoraproject.org https://admin.fedoraproject.org/mailman/listinfo/package-review
[Bug 915144] Review Request: rasmol - Molecular Graphics Visualization Tool
https://bugzilla.redhat.com/show_bug.cgi?id=915144 --- Comment #26 from Christopher Meng cicku...@gmail.com --- Not sure. Waiting for reviewer. -- 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=BTbwITupA4a=cc_unsubscribe ___ package-review mailing list package-review@lists.fedoraproject.org https://admin.fedoraproject.org/mailman/listinfo/package-review
[Bug 915144] Review Request: rasmol - Molecular Graphics Visualization Tool
https://bugzilla.redhat.com/show_bug.cgi?id=915144 --- Comment #27 from Dmitrij S. Kryzhevich kr...@land.ru --- If you don't mind, I would like to left both of them. -- 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=y1A3wQwY0Ha=cc_unsubscribe ___ package-review mailing list package-review@lists.fedoraproject.org https://admin.fedoraproject.org/mailman/listinfo/package-review
[Bug 915144] Review Request: rasmol - Molecular Graphics Visualization Tool
https://bugzilla.redhat.com/show_bug.cgi?id=915144 --- Comment #28 from Michael Schwendt bugs.mich...@gmx.net --- Which type to remove, txt or html? Neither one. ;-p Keep'em all. Nothing wrong with that. -- 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=KM69FU6Hp3a=cc_unsubscribe ___ package-review mailing list package-review@lists.fedoraproject.org https://admin.fedoraproject.org/mailman/listinfo/package-review
[Bug 915144] Review Request: rasmol - Molecular Graphics Visualization Tool
https://bugzilla.redhat.com/show_bug.cgi?id=915144 --- Comment #23 from Dmitrij S. Kryzhevich kr...@land.ru --- Spec URL: http://krege.fedorapeople.org/rasmol/rasmol.spec SRPM URL: http://krege.fedorapeople.org/rasmol/rasmol-2.7.5-9.fc19.src.rpm %changelog * Thu Jul 25 2013 Dmitrij S. Kryzhevich 2.7.5-9 - Some missed -p key for install command. - Added missed entry to the prev changelog section (man file). * Mon Jul 22 2013 Dmitrij S. Kryzhevich 2.7.5-8 - Fixed bogus dates. - Added GPL file to the -doc subpackage. - Added -p key to install command. - Fixed man file. Added patch for it. -- 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=fpNXIB1xdua=cc_unsubscribe ___ package-review mailing list package-review@lists.fedoraproject.org https://admin.fedoraproject.org/mailman/listinfo/package-review
[Bug 915144] Review Request: rasmol - Molecular Graphics Visualization Tool
https://bugzilla.redhat.com/show_bug.cgi?id=915144 Christopher Meng cicku...@gmail.com changed: What|Removed |Added CC||cicku...@gmail.com --- Comment #24 from Christopher Meng cicku...@gmail.com --- I think you don't need to include a file's 3 different types. See %doc %doc ChangeLog %doc ChangeLog.html ChangeLog.txt GPL NOTICE PROJECTS RASLIC README.html README.txt TODO.html TODO.txt history.html Please remove redundant files. -- 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=Okm9jcJSc7a=cc_unsubscribe ___ package-review mailing list package-review@lists.fedoraproject.org https://admin.fedoraproject.org/mailman/listinfo/package-review
[Bug 915144] Review Request: rasmol - Molecular Graphics Visualization Tool
https://bugzilla.redhat.com/show_bug.cgi?id=915144 --- Comment #25 from Dmitrij S. Kryzhevich kr...@land.ru --- (In reply to Christopher Meng from comment #24) What are 3 types? I see only 2. Which type to remove, txt or html? -- 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=NZrIyxfYXna=cc_unsubscribe ___ package-review mailing list package-review@lists.fedoraproject.org https://admin.fedoraproject.org/mailman/listinfo/package-review
[Bug 915144] Review Request: rasmol - Molecular Graphics Visualization Tool
https://bugzilla.redhat.com/show_bug.cgi?id=915144 --- Comment #21 from Dmitrij S. Kryzhevich kr...@land.ru --- Sorry for the delay, again. (In reply to Björn Esser from comment #18) - bogus date in %changelog: Wed Mar 14 2013 Dmitrij S. Kryzhevich 2.7.5-7 - bogus date in %changelog: Thu Mar 13 2013 Dmitrij S. Kryzhevich 2.7.5-6 - bogus date in %changelog: Thu Mar 13 2013 Dmitrij S. Kryzhevich 2.7.5-5 --- fix this, please Done. - manual-page-warning rasmol.1x.gz 1: warning: macro `PU' not defined - manual-page-warning rasmol.1x.gz 4119: warning: macro `false',' not defined --- please fix this, e.g. with patch I'm trying to understang what is called one after another in Makefile. rasmol tar contain formated file AND man file AND compressed man file. - rasmol.x86_64: E: incorrect-fsf-address /usr/share/doc/rasmol-2.7.5/GPL --- please inform upstream and ask for including recent rev. of that document Done (it was done before). --- LDFLAGS are ommitted on linking Done. [!]: License file installed when any subpackage combination is installed. --- add %doc GPL to -doc-pkg Done. [!]: Large documentation must go in a -doc subpackage. Note: Documentation size is 481280 bytes in 20 files. --- you should move all %doc, but GPL, to doc-pkg Hm... Changelog and readme? All doc is already in -doc. What do you mean? [!]: Packages should try to preserve timestamps of original installed files. --- add -p switch to install-commands, please. Done. Spec URL: http://krege.fedorapeople.org/rasmol/rasmol.spec SRPM URL: http://krege.fedorapeople.org/rasmol/rasmol-2.7.5-8.fc18.src.rpm -- 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=3BofGdc6O5a=cc_unsubscribe ___ package-review mailing list package-review@lists.fedoraproject.org https://admin.fedoraproject.org/mailman/listinfo/package-review
[Bug 915144] Review Request: rasmol - Molecular Graphics Visualization Tool
https://bugzilla.redhat.com/show_bug.cgi?id=915144 --- Comment #22 from Dmitrij S. Kryzhevich kr...@land.ru --- Ah, this version contain fix for man file too, it is fixed. That was old remark. -- 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=gUZkqnZidfa=cc_unsubscribe ___ package-review mailing list package-review@lists.fedoraproject.org https://admin.fedoraproject.org/mailman/listinfo/package-review
[Bug 915144] Review Request: rasmol - Molecular Graphics Visualization Tool
https://bugzilla.redhat.com/show_bug.cgi?id=915144 --- Comment #20 from Björn Esser bjoern.es...@gmail.com --- Any new progress here? -- 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=p8xNDhOHZ8a=cc_unsubscribe ___ package-review mailing list package-review@lists.fedoraproject.org https://admin.fedoraproject.org/mailman/listinfo/package-review
[Bug 915144] Review Request: rasmol - Molecular Graphics Visualization Tool
https://bugzilla.redhat.com/show_bug.cgi?id=915144 --- Comment #19 from Dmitrij S. Kryzhevich kr...@land.ru --- Sorry for silence, I'l be ready for work with rasmol at about 10th of July. -- 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=kyVOcJVY0ea=cc_unsubscribe ___ package-review mailing list package-review@lists.fedoraproject.org https://admin.fedoraproject.org/mailman/listinfo/package-review
[Bug 915144] Review Request: rasmol - Molecular Graphics Visualization Tool
https://bugzilla.redhat.com/show_bug.cgi?id=915144
Björn Esser bjoern.es...@gmail.com changed:
What|Removed |Added
Status|NEW |ASSIGNED
CC||bjoern.es...@gmail.com
Assignee|nob...@fedoraproject.org|bjoern.es...@gmail.com
Flags||fedora-review?
--- Comment #18 from Björn Esser bjoern.es...@gmail.com ---
Package has issues, see below.
#
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 rasmol, rasmol-gtk
See: http://fedoraproject.org/wiki/Packaging:ScriptletSnippets#Icon_Cache
--- false positive: no mime-type in desktop-file
- bogus date in %changelog: Wed Mar 14 2013 Dmitrij S. Kryzhevich 2.7.5-7
- bogus date in %changelog: Thu Mar 13 2013 Dmitrij S. Kryzhevich 2.7.5-6
- bogus date in %changelog: Thu Mar 13 2013 Dmitrij S. Kryzhevich 2.7.5-5
--- fix this, please
- manual-page-warning rasmol.1x.gz 1: warning: macro `PU' not defined
- manual-page-warning rasmol.1x.gz 4119: warning: macro `false',' not defined
--- please fix this, e.g. with patch
- rasmol.x86_64: E: incorrect-fsf-address /usr/share/doc/rasmol-2.7.5/GPL
--- please inform upstream and ask for including recent rev.
of that document
- more issues to be found in inline-comments of report
= 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.
[!]: %build honors applicable compiler flags or justifies otherwise.
--- LDFLAGS are ommitted on linking
[x]: Package contains no bundled libraries without FPC exception.
[!]: Changelog in prescribed format.
--- bogus dates, see above
[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.
Note: No Requires: %{name}%{?_isa} = %{version}-%{release} in rasmol-doc
--- false positve: doc-subpkg is noarch and should not require binaries
[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:
LGPL (v2 or later) (with incorrect FSF address), Unknown or
generated. 97 files have unknown license. Detailed output of
licensecheck in
/home/bjoern.esser/fedora/review/915144-rasmol/licensecheck.txt
[!]: License file installed when any subpackage combination is installed.
--- add %doc GPL to -doc-pkg
[x]: Package consistently uses macro is (instead of hard-coded directory
names).
[x]: If the package is under multiple licenses, the licensing breakdown must
be documented in the spec.
[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]: gtk-update-icon-cache is invoked when required
Note: icons in rasmol
[x]: Useful -debuginfo package or justification otherwise.
[!]: Large documentation must go in a -doc subpackage.
Note: Documentation size is 481280 bytes in 20 files.
--- you should move all %doc, but GPL, to doc-pkg
[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
[Bug 915144] Review Request: rasmol - Molecular Graphics Visualization Tool
https://bugzilla.redhat.com/show_bug.cgi?id=915144 --- Comment #17 from Dmitrij S. Kryzhevich kr...@land.ru --- Michael, could you (and wish) performe the review? -- 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=ixUChaEMXGa=cc_unsubscribe ___ package-review mailing list package-review@lists.fedoraproject.org https://admin.fedoraproject.org/mailman/listinfo/package-review
[Bug 915144] Review Request: rasmol - Molecular Graphics Visualization Tool
Product: Fedora https://bugzilla.redhat.com/show_bug.cgi?id=915144 --- Comment #13 from Michael Schwendt mschwe...@gmail.com --- update-desktop-database is not required. Both main and -gtk subpackage call update-desktop-database as both of them have .desktop files. That's contradictory. So, you run the tool although that's not required? That's harmless, but superfluous. qt package check paths in the same way. So it looks to be safe. Qt does it to adjust install paths. You do it to patch a header file that influences compilation. -- 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=CHYzdNJvMra=cc_unsubscribe ___ package-review mailing list package-review@lists.fedoraproject.org https://admin.fedoraproject.org/mailman/listinfo/package-review
[Bug 915144] Review Request: rasmol - Molecular Graphics Visualization Tool
Product: Fedora https://bugzilla.redhat.com/show_bug.cgi?id=915144 --- Comment #14 from Dmitrij S. Kryzhevich kr...@land.ru --- Spec URL: http://krege.fedorapeople.org/rasmol/rasmol.spec SRPM URL: http://krege.fedorapeople.org/rasmol/rasmol-2.7.5-6.fc18.src.rpm (In reply to comment #13) update-desktop-database is not required. Both main and -gtk subpackage call update-desktop-database as both of them have .desktop files. That's contradictory. So, you run the tool although that's not required? That's harmless, but superfluous. Oh. What a... Hm. Shame for me. It is only now when I understand what you both talking about. Sorry. Sure, it is not required and they are removed. qt package check paths in the same way. So it looks to be safe. Qt does it to adjust install paths. You do it to patch a header file that influences compilation. But the check itself for paths looks to be the same. Anyway, are there better ideas? * Thu Mar 13 2013 Dmitrij S. Kryzhevich kr...@land.ru 2.7.5-6 - Remove update-desktop-database from post and postun scripts as .desktop files do not contain MimeType key. -- 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=hflC941xeWa=cc_unsubscribe ___ package-review mailing list package-review@lists.fedoraproject.org https://admin.fedoraproject.org/mailman/listinfo/package-review
[Bug 915144] Review Request: rasmol - Molecular Graphics Visualization Tool
Product: Fedora
https://bugzilla.redhat.com/show_bug.cgi?id=915144
--- Comment #15 from Michael Schwendt mschwe...@gmail.com ---
How about evaluating %__isa_bits instead?
%if %{?__isa_bits} == 64
# ...
%else
# ...
%endif
--
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=k9lAXRpoiJa=cc_unsubscribe
___
package-review mailing list
package-review@lists.fedoraproject.org
https://admin.fedoraproject.org/mailman/listinfo/package-review
[Bug 915144] Review Request: rasmol - Molecular Graphics Visualization Tool
Product: Fedora
https://bugzilla.redhat.com/show_bug.cgi?id=915144
--- Comment #16 from Dmitrij S. Kryzhevich kr...@land.ru ---
Spec URL: http://krege.fedorapeople.org/rasmol/rasmol.spec
SRPM URL: http://krege.fedorapeople.org/rasmol/rasmol-2.7.5-7.fc18.src.rpm
(In reply to comment #15)
How about evaluating %__isa_bits instead?
%if %{?__isa_bits} == 64
# ...
%else
# ...
%endif
Had found that in qt.spec, didn't have time to update yestersay.
* Wed Mar 14 2013 Dmitrij S. Kryzhevich kr...@land.ru 2.7.5-7
- Let apply 64b patch according to the __isa_bits macros.
--
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=GlRYyXNYkMa=cc_unsubscribe
___
package-review mailing list
package-review@lists.fedoraproject.org
https://admin.fedoraproject.org/mailman/listinfo/package-review
[Bug 915144] Review Request: rasmol - Molecular Graphics Visualization Tool
Product: Fedora
https://bugzilla.redhat.com/show_bug.cgi?id=915144
--- Comment #8 from Antonio Trande trp...@katamail.com ---
(In reply to comment #7)
- Package consistently uses macro is (instead of hard-coded directory
names).
%%{name} macro can be used more frequently in .spec file.
May be. Not sure.
Hardcoded system dir names are not present.
'install -D -m 755 %{SOURCE4} %{buildroot}%{_bindir}/rasmol' becomes
'install -D -m 755 %{SOURCE4} %{buildroot}%{_bindir}/%{name}'
or
'cp data/* %{buildroot}%{_datadir}/rasmol/data/' becomes
'cp data/* %{buildroot}%{_datadir}/%{name}/data/'
or again
'Patch1: rasmol-2.7.5-23Jul09-gtk.patch' becomes
'Patch1: %{name}-2.7.5-23Jul09-gtk.patch'
- update-desktop-database is invoked when required
Note: desktop file(s) in rasmol, rasmol-gtk
See: http://fedoraproject.org/wiki/Packaging:ScriptletSnippets#Icon_Cache
Icon cache updated in rasmol package, no more icons for rasmol-gtk.
http://fedoraproject.org/wiki/Packaging:Guidelines#desktop-file-
install_usage was used for .desktop files.
Again, what is wrong?
Probably it refers to missing icon in gtk sub-package but indicated by
grasmol.desktop file. I'm not sure.
--
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=GZj0Y8ppNCa=cc_unsubscribe
___
package-review mailing list
package-review@lists.fedoraproject.org
https://admin.fedoraproject.org/mailman/listinfo/package-review
[Bug 915144] Review Request: rasmol - Molecular Graphics Visualization Tool
Product: Fedora
https://bugzilla.redhat.com/show_bug.cgi?id=915144
--- Comment #9 from Dmitrij S. Kryzhevich kr...@land.ru ---
(In reply to comment #8)
'install -D -m 755 %{SOURCE4} %{buildroot}%{_bindir}/rasmol' becomes
'install -D -m 755 %{SOURCE4} %{buildroot}%{_bindir}/%{name}'
or
'cp data/* %{buildroot}%{_datadir}/rasmol/data/' becomes
'cp data/* %{buildroot}%{_datadir}/%{name}/data/'
or again
'Patch1: rasmol-2.7.5-23Jul09-gtk.patch' becomes
'Patch1: %{name}-2.7.5-23Jul09-gtk.patch'
Let it be as is. Putting %{name} wherever it could be put - I think it is
wrong.
Probably it refers to missing icon in gtk sub-package but indicated by
grasmol.desktop file. I'm not sure.
No icons in rasmol-gtk is that what suppose to be. It is not a realy issue I
think.
--
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=cBseGVeUI8a=cc_unsubscribe
___
package-review mailing list
package-review@lists.fedoraproject.org
https://admin.fedoraproject.org/mailman/listinfo/package-review
[Bug 915144] Review Request: rasmol - Molecular Graphics Visualization Tool
Product: Fedora
https://bugzilla.redhat.com/show_bug.cgi?id=915144
--- Comment #10 from Michael Schwendt mschwe...@gmail.com ---
'Patch1: rasmol-2.7.5-23Jul09-gtk.patch' becomes
'Patch1: %{name}-2.7.5-23Jul09-gtk.patch'
Not necessarily a good idea to do that. These tags refer to files with a
hardcoded file name, because they have been checked into git with those file
names. If %name changes, the file names don't change automatically. But if the
patches still apply afterwards, there is no need to rename them.
- update-desktop-database is invoked when required
But is it required?
$ grep -i mime *.desktop
$
http://fedoraproject.org/wiki/Packaging:ScriptletSnippets#desktop-database
Icon cache updated in rasmol package, no more icons for rasmol-gtk.
What does that mean? Have you added the scriptlets in release -4? If so, please
mention such details in the %changelog. It isn't easy to follow this review
when you mention changes in bugzilla, which don't match the %changelog. The
previous src.rpm is gone already, too, and as I don't have a copy of it,
rpmdiff cannot be used.
So, with regard to update-desktop-database and gtk-update-icon-cache I can only
guess that there is some confusion.
Requires: %{name} = %{version}-%{release}
https://fedoraproject.org/wiki/Packaging:Guidelines#Requiring_Base_Package
%package doc
Requires: %{name} = %{version}-%{release}
Please don't add a base dep to -doc subpackages. Documentation packages ought
to stay free of such dependencies. Imagine someone who wants to peruse the
documentation without being forced to install the application (and possibly
lots of dependencies).
In case you worry about ownership of %{_datadir}/%{name}/, having multiple
packages own the same dir is okay with current packaging guidelines.
https://fedoraproject.org/wiki/Packaging:Guidelines#File_and_Directory_Ownership
# 64b version require one more definition
%if %{?_lib} == lib64
%patch2 -p1
Dangerous. This assumes that every 64-bit platform uses lib64, even if it may
not support multilib.
# Fix harcoded vars
sed -i -e 's|PKGDIR = $(HOME)|PKGDIR = /usr|' -e 's|/usr/local|/usr|g'
-e 's|RASMOLDIR = $(USRLIBDIR)/rasmol/|RASMOLDIR = %{_datadir}/rasmol/|' \
Without checking each of these substitutions, please consider adding guards
(e.g. with a grep before or afterwards). Alternatively, add a comment that
explains whether the %files section guards against failing substitutions
already. Sed substitutions are fragile. If a match fails, because the target
file has changed, no substitution would be performed. That's a case that may
cause the build to fail later (e.g. due to the non-matching %files section,
which would be good) or succeed with installing files in wrong locations or
building the software with wrong built-in paths (which would be bad).
--
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=DtBPP90DvHa=cc_unsubscribe
___
package-review mailing list
package-review@lists.fedoraproject.org
https://admin.fedoraproject.org/mailman/listinfo/package-review
[Bug 915144] Review Request: rasmol - Molecular Graphics Visualization Tool
Product: Fedora
https://bugzilla.redhat.com/show_bug.cgi?id=915144
--- Comment #11 from Dmitrij S. Kryzhevich kr...@land.ru ---
Spec URL: http://krege.fedorapeople.org/rasmol/rasmol.spec
SRPM URL: http://krege.fedorapeople.org/rasmol/rasmol-2.7.5-5.fc18.src.rpm
(In reply to comment #10)
Icon cache updated in rasmol package, no more icons for rasmol-gtk.
What does that mean? Have you added the scriptlets in release -4? If so,
please mention such details in the %changelog.
I didn't. It was already in relese -1.
So, with regard to update-desktop-database and gtk-update-icon-cache I can
only guess that there is some confusion.
update-desktop-database is not required. gtk-update-icon-cache is used to for
installed icon. That icon is installed in main package and no more icons are
installed in subpackages so (as I thinks) subpackages does not require to call
gtk-update-icon-cache. Both main and -gtk subpackage call
update-desktop-database as both of them have .desktop files.
Requires: %{name} = %{version}-%{release}
https://fedoraproject.org/wiki/Packaging:Guidelines#Requiring_Base_Package
%{?_isa} added.
%package doc
Requires: %{name} = %{version}-%{release}
Please don't add a base dep to -doc subpackages. Documentation packages
ought to stay free of such dependencies. Imagine someone who wants to peruse
the documentation without being forced to install the application (and
possibly lots of dependencies).
I think you are right. Removed. %files fixed.
# 64b version require one more definition
%if %{?_lib} == lib64
%patch2 -p1
Dangerous. This assumes that every 64-bit platform uses lib64, even if it
may not support multilib.
Example was taken from crossplatform cmake package which runs on at least ppc
and arm. Both put 64b libs in /usr/lib64, right. But in Fedora at all main
platforms 64b libs are in /usr/lib64. Sorry, I can't handle all possibilites. I
just suppose that %{?_lib} is not an arch (as 'ia64' and 'ppc64' are not
'lib64' obviously) so it would be work correctly anywhere.
But moreover it assumes that every 64-bit platform requires that definition in
patch. Documentation say it is required. I could test only x64_64/i686 and not
even arm.
# Fix harcoded vars
sed -i -e 's|PKGDIR = $(HOME)|PKGDIR = /usr|' -e 's|/usr/local|/usr|g'
-e 's|RASMOLDIR = $(USRLIBDIR)/rasmol/|RASMOLDIR = %{_datadir}/rasmol/|' \
Without checking each of these substitutions, please consider adding guards
(e.g. with a grep before or afterwards). Alternatively, add a comment that
explains whether the %files section guards against failing substitutions
already. Sed substitutions are fragile. If a match fails, because the target
file has changed, no substitution would be performed. That's a case that may
cause the build to fail later (e.g. due to the non-matching %files section,
which would be good) or succeed with installing files in wrong locations or
building the software with wrong built-in paths (which would be bad).
1) All substitutions beside RASMOLDIR. It is what could brake compilation and
compilation only.
2) RASMOLDIR. It is handle where place PDB and help files. If they (files)
whould be in some other place, /usr/lib/rpm/check-files will provide an error.
That comment was placed in spec.
%changelog
* Thu Mar 13 2013 Dmitrij S. Kryzhevich kr...@land.ru 2.7.5-5
- Dependency for -gtk subpackage use ?_isa macros.
- Drop main package dependency for -doc subpackage.
- Fix files section for -doc according new deps.
- sed substitutions were commented.
- Fix previouse changelog (added last string).
* Mon Mar 04 2013 Dmitrij S. Kryzhevich kr...@land.ru 2.7.5-4
- doc subpackage is noarch now.
- Some cleanups.
- Fix encoding for doc/itrasmol2721.hlp.
--
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=dFuFYAe0PBa=cc_unsubscribe
___
package-review mailing list
package-review@lists.fedoraproject.org
https://admin.fedoraproject.org/mailman/listinfo/package-review
[Bug 915144] Review Request: rasmol - Molecular Graphics Visualization Tool
Product: Fedora
https://bugzilla.redhat.com/show_bug.cgi?id=915144
--- Comment #12 from Dmitrij S. Kryzhevich kr...@land.ru ---
I just suppose that %{?_lib} is not an arch (as 'ia64' and
'ppc64' are not 'lib64' obviously) so it would be work correctly anywhere.
And one more: qt package check paths in the same way. So it looks to be safe.
--
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=MQgO5W12UGa=cc_unsubscribe
___
package-review mailing list
package-review@lists.fedoraproject.org
https://admin.fedoraproject.org/mailman/listinfo/package-review
[Bug 915144] Review Request: rasmol - Molecular Graphics Visualization Tool
Product: Fedora
https://bugzilla.redhat.com/show_bug.cgi?id=915144
Antonio Trande trp...@katamail.com changed:
What|Removed |Added
CC||trp...@katamail.com
--- Comment #6 from Antonio Trande trp...@katamail.com ---
Hi Dmitrij.
I'm not a official Fedora packager so let me try to review your package. My
review could contain some (my) evaluation errors, please you consider all that
as a test for myself. :)
- gzip can be omitted
https://fedoraproject.org/wiki/Packaging:Guidelines?rd=Packaging/Guidelines#Exceptions_2
- Another license (LGPLv2+) has been detected for 'eggfileformatchooser.h'
file. You should consider to add a multiple licensing:
https://fedoraproject.org/wiki/Packaging:LicensingGuidelines#Multiple_Licensing_Scenarios
- Patches link to upstream bugs/comments/lists or are otherwise justified.
These info are missing, add them if possible.
- Package uses nothing in %doc for runtime.
-doc sub-package contains '%{_datadir}/%{name}/data' directory with all
*.pdb* files; .pdb files are Protein Data Bank data files used from
application.
Why they are excluded from main package ? If they were not, -doc sub-package
could be packaged as 'noarch' and could contain only document files.
- Fully versioned dependency in subpackages, if present.
I don't know if Epochs is strictly necessary for rasmol; if no,
'Requires: %{name}%{?_isa} = %{version}-%{release}' is better.
- Package consistently uses macro is (instead of hard-coded directory names).
%%{name} macro can be used more frequently in .spec file.
- rasmol-doc.x86_64: W: file-not-utf8
/usr/share/doc/rasmol-doc-2.7.5/doc/itrasmol2721.hlp
This warning seems can be fixed by using something like that:
iconv --from=ISO-8859-1 --to=UTF-8 doc/itrasmol2721.hlp
doc/itrasmol2721_new.hlp
mv doc/itrasmol2721_new.hlp doc/itrasmol2721.hlp
http://fedoraproject.org/wiki/Packaging_tricks#Convert_encoding_to_UTF-8
Package Review
==
Key:
[x] = Pass
[!] = Fail
[-] = Not applicable
[?] = Not evaluated
[ ] = Manual review needed
Issues:
===
- All build dependencies are listed in BuildRequires, except for any that are
listed in the exceptions section of Packaging Guidelines.
Note: These BR are not needed: gzip
See: http://fedoraproject.org/wiki/Packaging/Guidelines#Exceptions_2
- update-desktop-database is invoked when required
Note: desktop file(s) in rasmol, rasmol-gtk
See: http://fedoraproject.org/wiki/Packaging:ScriptletSnippets#Icon_Cache
= 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:
[?]: Package is licensed with an open-source compatible license and meets
other legal requirements as defined in the legal section of Packaging
Guidelines.
[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
[-]: Package requires other packages for directories it uses.
[!]: Patches link to upstream bugs/comments/lists or are otherwise justified
[x]: Package is not known to require ExcludeArch.
[!]: Fully versioned dependency in subpackages, if present.
Note: No Requires: %{name}%{?_isa} = %{version}-%{release} in rasmol-gtk
, rasmol-doc
[x]: Package complies to the Packaging Guidelines
[?]: 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.
[?]: License field in the package spec file matches the actual license.
Note: Checking patched sources after %prep for licenses. Licenses found:
LGPL (v2 or later) (with incorrect FSF address), Unknown or
generated. 2 files have unknown license. Detailed output of licensecheck
in /home/sagitter/rpmbuild/SRPMS/915144-rasmol/licensecheck.txt
[x]: License file installed when any subpackage combination is installed.
[!]: 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.
[!]: 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]:
[Bug 915144] Review Request: rasmol - Molecular Graphics Visualization Tool
Product: Fedora
https://bugzilla.redhat.com/show_bug.cgi?id=915144
--- Comment #7 from Dmitrij S. Kryzhevich kr...@land.ru ---
Spec URL: http://krege.fedorapeople.org/rasmol/rasmol.spec
SRPM URL: http://krege.fedorapeople.org/rasmol/rasmol-2.7.5-4.fc18.src.rpm
Thanks.
- gzip can be omitted
Done.
- Another license (LGPLv2+) has been detected for 'eggfileformatchooser.h'
file. You should consider to add a multiple licensing:
Right. Missed that.
- Patches link to upstream bugs/comments/lists or are otherwise justified.
These info are missing, add them if possible.
Didn't catch that.
Add comment on pathes but bot sure it is what you mean.
- Package uses nothing in %doc for runtime.
-doc sub-package contains '%{_datadir}/%{name}/data' directory with all
*.pdb* files; .pdb files are Protein Data Bank data files used from
application.
Why they are excluded from main package ? If they were not, -doc sub-package
could be packaged as 'noarch' and could contain only document files.
noarch added.
data dir is a dir with examples, their are not needed for rasmol to work
actualy.
- Fully versioned dependency in subpackages, if present.
I don't know if Epochs is strictly necessary for rasmol; if no,
'Requires: %{name}%{?_isa} = %{version}-%{release}' is better.
Why not? Fixed.
- Package consistently uses macro is (instead of hard-coded directory names).
%%{name} macro can be used more frequently in .spec file.
May be. Not sure.
Hardcoded system dir names are not present.
- rasmol-doc.x86_64: W: file-not-utf8
/usr/share/doc/rasmol-doc-2.7.5/doc/itrasmol2721.hlp
This warning seems can be fixed by using something like that:
iconv --from=ISO-8859-1 --to=UTF-8 doc/itrasmol2721.hlp
doc/itrasmol2721_new.hlp
mv doc/itrasmol2721_new.hlp doc/itrasmol2721.hlp
ISO-8859-1? How could I gues that...
- update-desktop-database is invoked when required
Note: desktop file(s) in rasmol, rasmol-gtk
See: http://fedoraproject.org/wiki/Packaging:ScriptletSnippets#Icon_Cache
Icon cache updated in rasmol package, no more icons for rasmol-gtk.
http://fedoraproject.org/wiki/Packaging:Guidelines#desktop-file-install_usage
was used for .desktop files.
Again, what is wrong?
rasmol.x86_64: E: incorrect-fsf-address /usr/share/doc/rasmol-2.7.5/GPL
This is semipositive issue.
--
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=yN2mpWLpeFa=cc_unsubscribe
___
package-review mailing list
package-review@lists.fedoraproject.org
https://admin.fedoraproject.org/mailman/listinfo/package-review
[Bug 915144] Review Request: rasmol - Molecular Graphics Visualization Tool
Product: Fedora https://bugzilla.redhat.com/show_bug.cgi?id=915144 Dmitrij S. Kryzhevich kr...@land.ru changed: What|Removed |Added Blocks||505154 (FE-SCITECH) -- 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=S1Sho8EgIba=cc_unsubscribe ___ package-review mailing list package-review@lists.fedoraproject.org https://admin.fedoraproject.org/mailman/listinfo/package-review
[Bug 915144] Review Request: rasmol - Molecular Graphics Visualization Tool
Product: Fedora https://bugzilla.redhat.com/show_bug.cgi?id=915144 --- Comment #4 from Dmitrij S. Kryzhevich kr...@land.ru --- Spec URL: http://krege.fedorapeople.org/rasmol/rasmol.spec SRPM URL: http://krege.fedorapeople.org/rasmol/rasmol-2.7.5-3.fc18.src.rpm Move some examples and docs to -doc subpackage. Warning. Source codes are not available on official site at the moment with 403 error. I wrote a letter to the webmaster, waiting for the reply. -- 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=swSbkTFG7La=cc_unsubscribe ___ package-review mailing list package-review@lists.fedoraproject.org https://admin.fedoraproject.org/mailman/listinfo/package-review
[Bug 915144] Review Request: rasmol - Molecular Graphics Visualization Tool
Product: Fedora https://bugzilla.redhat.com/show_bug.cgi?id=915144 --- Comment #5 from Dmitrij S. Kryzhevich kr...@land.ru --- Warning droppped. Sources available now. -- 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=Qwd9RUk89Xa=cc_unsubscribe ___ package-review mailing list package-review@lists.fedoraproject.org https://admin.fedoraproject.org/mailman/listinfo/package-review
[Bug 915144] Review Request: rasmol - Molecular Graphics Visualization Tool
Product: Fedora https://bugzilla.redhat.com/show_bug.cgi?id=915144 --- Comment #3 from Dmitrij S. Kryzhevich kr...@land.ru --- Spec URL: http://krege.fedorapeople.org/rasmol/rasmol.spec SRPM URL: http://krege.fedorapeople.org/rasmol/rasmol-2.7.5-2.fc18.src.rpm An error in script: missed -e option to the xterm. -- 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=jdFxD6gspha=cc_unsubscribe ___ package-review mailing list package-review@lists.fedoraproject.org https://admin.fedoraproject.org/mailman/listinfo/package-review
[Bug 915144] Review Request: rasmol - Molecular Graphics Visualization Tool
Product: Fedora https://bugzilla.redhat.com/show_bug.cgi?id=915144 Dmitrij S. Kryzhevich kr...@land.ru changed: What|Removed |Added CC||c...@five-ten-sg.com --- Comment #1 from Dmitrij S. Kryzhevich kr...@land.ru --- *** Bug 541462 has been marked as a duplicate of this bug. *** -- 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=kGhAL9QFtIa=cc_unsubscribe ___ package-review mailing list package-review@lists.fedoraproject.org https://admin.fedoraproject.org/mailman/listinfo/package-review
[Bug 915144] Review Request: rasmol - Molecular Graphics Visualization Tool
Product: Fedora https://bugzilla.redhat.com/show_bug.cgi?id=915144 --- Comment #2 from Dmitrij S. Kryzhevich kr...@land.ru --- Issue 2: https://fedoraproject.org/wiki/Common_Rpmlint_issues#incorrect-fsf-address I do not allowed to change this file, need to provide this as is. -- 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=8FB7ejpGlEa=cc_unsubscribe ___ package-review mailing list package-review@lists.fedoraproject.org https://admin.fedoraproject.org/mailman/listinfo/package-review
[Bug 915144] Review Request: rasmol - Molecular Graphics Visualization Tool
Product: Fedora https://bugzilla.redhat.com/show_bug.cgi?id=915144 Dmitrij S. Kryzhevich kr...@land.ru changed: What|Removed |Added Alias||rasmol -- 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=gOWR0OhYR5a=cc_unsubscribe ___ package-review mailing list package-review@lists.fedoraproject.org https://admin.fedoraproject.org/mailman/listinfo/package-review
