Product: Fedora https://bugzilla.redhat.com/show_bug.cgi?id=915144 --- Comment #11 from Dmitrij S. Kryzhevich <krege@xxxxxxx> --- 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 <krege@xxxxxxx> 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 <krege@xxxxxxx> 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=dFuFYAe0PB&a=cc_unsubscribe _______________________________________________ package-review mailing list package-review@xxxxxxxxxxxxxxxxxxxxxxx https://admin.fedoraproject.org/mailman/listinfo/package-review