https://bugzilla.redhat.com/show_bug.cgi?id=841335 Michael Schwendt <mschwendt@xxxxxxxxx> changed: What |Removed |Added ---------------------------------------------------------------------------- Flags| |fedora-review? --- Comment #2 from Michael Schwendt <mschwendt@xxxxxxxxx> --- * Latest upstream release has been updated to. Good. $ sha256sum gnusim8085-1.3.7.tar.gz e09b56089276eed91fb9df3c1e7e2aa4bf091859cfc62612521b45617167d525 gnusim8085-1.3.7.tar.gz * rpmlint output is clean. > License: GPLv2 "GPLv2+" according to the source file preambles. https://fedoraproject.org/wiki/Packaging:LicensingGuidelines#.22or_later_version.22_licenses $ find src -name \*.c|wc -l 31 $ grep "any later version" src/*.c|wc -l 31 $ find src -name \*.h|wc -l 31 $ grep "any later version" src/*.h|wc -l 31 > URL: http://gnusim8085.sourceforge.net | We have moved to new domain. Please update your bookmark. You should be | redirected to our new website in 10 seconds. If not please click here. -> http://gnusim8085.org/ > BuildRequires: automake libtool Apparently only needed because an autoconf recheck is triggered by the "sed" based change to configure.in. That made me curious. ;-) There has been a crash failing to find the documentation files: bug 542945 The fix is a brute-force sed substitution without any guard: > sed -i \ > "s|share/doc/\${PACKAGE}|share/doc/%{name}-%{version}|" \ > configure.in > sed -i "s|/usr/local/doc/GNUSim8085|%{_docdir}/%{name}-%{version}|" > src/callbacks.c One ought to be careful with such "sed" substitution, because if they don't match any longer, the command doesn't fail, and you don't notice. Hence it's superior to add a safety-check, such as a separate "grep". Anyway: The first sed modifies the line packagedocdir=share/doc/${PACKAGE} in configure.in, but I could not find any other place where this variable would be used. The second sed replaces a hardcoded string that is no longer present in version 1.3.7, but instead a different variable is used: g_string_append (tutorial_text, PACKAGE_DOC_DIR); and it is defined in src/Makefile.am as: $ grep PACKAGE_DOC_DIR src/Makefile.am -DPACKAGE_DOC_DIR=\"$(docdir)\"\ So, instead of the two sed substitutions, running configure like this redefined PACKAGE_DOC_DIR: %configure --docdir %{_datadir}/doc/%{name}-%{version} However, reading further the spec file, I found it to be dangerous. During %install, it does rm -rf %{buildroot}%{_docdir} to remove _any_ installed files in /usr/share/doc (whatever may have been installed there intentionally!), then it adds doc files manually via %doc in the %files section. Why is that dangerous? The default location for %doc files is %{_datadir}/doc/%{name}-%{version}/ which happily conflicts with any files already installed in there. Using %doc to install doc files overrides any files which are in that directory already. "make install" already installs all documentation except for the license file "COPYING". So, if that gets installed manually during %install, everything would be available, and using %doc is not necessary anymore. > make %{?_smp_mflags} CFLAGS="%{optflags} In your koji test build, I noticed the missing verbosity of compiler/linker output. Adding V=1 to the make invocation fixes that, so one can see all the build details. > mkdir -p %{buildroot}%{_mandir}/man1 Superfluous, as a man page is available in there already. Command can be removed. > Summary: A 8085 Simulator Not a blocker, but a little bit more verbose could be better: Summary: Graphical simulator for 8085 assembly language > %{_mandir}/man1/%{name}.1.gz Better: %{_mandir}/man1/%{name}.1* The on-the-fly compression to gzip could change eventually or be disabled/changed in a different build environment. * Patch for suggested changes will follow. * What do you think? -- You are receiving this mail because: You are on the CC list for the bug. _______________________________________________ package-review mailing list package-review@xxxxxxxxxxxxxxxxxxxxxxx https://admin.fedoraproject.org/mailman/listinfo/package-review