https://bugzilla.redhat.com/show_bug.cgi?id=1295144 --- Comment #11 from Michael Schwendt <bugs.michael@xxxxxxx> --- Let me recommend slowing down with the review a bit. > Please, remove: > %define checkout 20140302git During review, while it is good to point at the guidelines, in this case the guidelines don't forbid %define in any way. Just like several other parts of the guidelines, they aim at educating packagers about a pitfall related to %define. There are hundreds of packages that build correctly using %define for global definitions in the spec file. > you must use rpm macros No. A rule of thumb (and background for the guidelines) really is that macros should be used consistently, so e.g. the %build section uses the same macros than the %files section. If there were a mismatch, the package may either fail to build or install some files into wrong locations (which may not always lead to a build failure depending on how you include files and directory trees). If the build framework, such as Makefiles that contain hardcoded paths, doesn't accept any RPM spec file macros given to a configure script, it would also not make any sense to use the same macros anywhere in the spec file. You could hardcode the same paths in the spec file. However, if the build process can be configured to accept values such as %_libdir or %_datadir, it is usually better to use such macros everywhere, so if Fedora changed some locations globally, the package would change too during a mass-rebuild already (and would not need to be modified by the packager). > must be > Release: 0.1.%{checkout}%{?dist} Why that? There is a 0.3.0 release tarball at https://bitbucket.org/raymonad/xss-lock/downloads already, dated 2013-11-04, so a git checkout made on 20140302 is a post-release snapshot. > Requires: xcb-util libxcb glib2 https://fedoraproject.org/wiki/Packaging:Guidelines#Explicit_Requires > cmake .. \ https://fedoraproject.org/wiki/Packaging:Cmake https://fedoraproject.org/wiki/Packaging:Guidelines#Compiler_flags https://kojipkgs.fedoraproject.org//work/tasks/1921/12381921/build.log Also check whether using the %cmake macro turns on verbose build output already. Currently, the compiler/linker invocation in the build.log cannot be seen, which makes the build.log less helpful when tracking down build problems. > -DCMAKE_INSTALL_LIBEXEC="%_libexecdir" \ Odd, since it doesn't use %_libexecdir in the %files section. Does it search for something in that dir? > make install DESTDIR=%{buildroot} %{?_smp_mflags} Works, but SMP make flags during %install don't add much, especially not for a small program like this. Also, the guidelines recommend the %make_install macro for less typing. Also see: rpm -E %make_install > %{_datadir}/bash-completion/completions/xss-lock > %{_datadir}/zsh/site-functions/_xss-lock https://fedoraproject.org/wiki/Packaging:Guidelines#File_and_Directory_Ownership > %{_mandir}/man1/xss-lock.1.gz Works, but unless it's the build process of this program, which adds the gzip compression to this file, it is rpmbuild which adds it on-the-fly. In that case, it may change any time to a different compressor or be disabled by local configuration. You can observe reviewers suggesting the following wildcard: %{_mandir}/man1/xss-lock.1* > %changelog https://fedoraproject.org/wiki/FrequentlyMadePackagingMistakes When modifying the spec file during package review, bumping Release and updating %changelog is good practice. -- You are receiving this mail because: You are on the CC list for the bug. You are always notified about changes to this product and component _______________________________________________ package-review mailing list package-review@xxxxxxxxxxxxxxxxxxxxxxx https://admin.fedoraproject.org/mailman/listinfo/package-review