Bug 487737
Summary: | Review Request: slock - Simple X display locker | ||||||
---|---|---|---|---|---|---|---|
Product: | [Fedora] Fedora | Reporter: | Pavel Shevchuk <stlwrt> | ||||
Component: | Package Review | Assignee: | Alexey Torkhov <atorkhov> | ||||
Status: | CLOSED NEXTRELEASE | QA Contact: | Fedora Extras Quality Assurance <extras-qa> | ||||
Severity: | medium | Docs Contact: | |||||
Priority: | medium | ||||||
Version: | rawhide | CC: | atorkhov, fedora-package-review, notting | ||||
Target Milestone: | --- | Flags: | atorkhov:
fedora-review+
kevin: fedora-cvs+ |
||||
Target Release: | --- | ||||||
Hardware: | All | ||||||
OS: | Linux | ||||||
Whiteboard: | |||||||
Fixed In Version: | Doc Type: | Bug Fix | |||||
Doc Text: | Story Points: | --- | |||||
Clone Of: | Environment: | ||||||
Last Closed: | 2009-03-27 16:13:19 UTC | Type: | --- | ||||
Regression: | --- | Mount Type: | --- | ||||
Documentation: | --- | CRM: | |||||
Verified Versions: | Category: | --- | |||||
oVirt Team: | --- | RHEL 7.3 requirements from Atomic Host: | |||||
Cloudforms Team: | --- | Target Upstream Version: | |||||
Embargoed: | |||||||
Attachments: |
|
Description
Pavel Shevchuk
2009-02-27 17:55:42 UTC
This package does not use standard rpm compiler flags when building: https://fedoraproject.org/wiki/Packaging/Guidelines#Compiler_flags Now it does. * Sun Mar 08 2009 Pavel "Stalwart" Shevchuk <stlwrt> - 0.9-2 - Set fedora generic compiler flags Spec URL: http://rpm.scwlab.com/fedora/slock/slock.spec SRPM URL: http://rpm.scwlab.com/fedora/slock/slock-0.9-2.fc10.src.rpm Built RPMs for F10 i386 and x86_64: http://rpm.scwlab.com/fedora/slock/ Generated debuginfo is empty. It happens because it have "-s" flag when running ld: https://fedoraproject.org/wiki/Packaging/Debuginfo I'm also suggesting to use patch, not sed. * Sun Mar 15 2009 Pavel "Stalwart" Shevchuk <stlwrt> - 0.9-3 - Fixed debuginfo generation Spec URL: http://rpm.scwlab.com/fedora/slock/slock.spec SRPM URL: http://rpm.scwlab.com/fedora/slock/slock-0.9-3.fc10.src.rpm Built RPMs for F10 i386 and x86_64: http://rpm.scwlab.com/fedora/slock/ - Replace two seds with a patch. It will be easier to read. Moreover, if upstream decide to change flags in makefile in future, sed will silently continue with old values and patch will fail. - This is a GUI application, you should create .desktop file for it. See https://fedoraproject.org/wiki/Packaging/Guidelines#desktop This is NOT a gui application, it's expected to be ran by windowmanager, not user And there are standard way for window manager to choose locker? How it will know of slock existence? Light WMs like openbox and dwm (slock was written for) bound commands, not fd.o .desktop nonsense. How to create static patch for config.mk that will get arch-specific CFLAGS? I think sed expressions are pretty good options for one-line patching - my EFL/E17 specs have plenty of those. Created attachment 335317 [details] Patch for makefile to use rpm optflags (In reply to comment #8) > Light WMs like openbox and dwm (slock was written for) bound commands, > not fd.o .desktop nonsense. Ok then. Please add a comment to spec telling that it is not need .desktop file and brief explanation. > How to create static patch for config.mk that will get arch-specific CFLAGS? Very simple - use $RPM_OPT_FLAGS environment variable. I've made a patch - it is in attachment as an example. Programs with good makefiles do not require any patching at all - you can pass flags in make call just like "make CXXFLAGS=...", but this makefile written in bad way, so it'll not add includes to flags. > I think sed expressions are pretty good options for one-line patching - my > EFL/E17 specs have plenty of those. This is a bad practice in case of makefiles - you won't notice when upstream source changes. I think, sed is good in opposite cases - when patch would be ugly, or big, or when change made by sed if safe for changes, etc. * Mon Mar 23 2009 Pavel "Stalwart" Shevchuk <stlwrt> - 0.9-4 - Replaced sed expressions with a patch Spec URL: http://rpm.scwlab.com/fedora/slock/slock.spec SRPM URL: http://rpm.scwlab.com/fedora/slock/slock-0.9-4.fc10.src.rpm Built RPMs for F10 i386 and x86_64: http://rpm.scwlab.com/fedora/slock/ Thank you for a patch, i didn't know $RPM_OPT_FLAGS is set in build environment + rpmlint output: slock.x86_64: E: setuid-binary /usr/bin/slock root 04755 slock.x86_64: E: non-standard-executable-perm /usr/bin/slock 04755 3 packages and 0 specfiles checked; 2 errors, 0 warnings. Setuid binary is intended. Otherwise it can't check user password. It drops privileges soon after start. + The package is named according to the Package Naming Guidelines. + The spec file name matches the base package %{name}, in the format %{name}.spec. + The package is licensed with a Fedora approved license and meets the Licensing Guidelines. + The License field in the package spec file matches the actual license. + File, containing the text of the licenses for the package is included in %doc. + The spec file is written in American English. + The spec file for the package is legible. + The sources used to build the package matches the upstream source, as provided in the spec URL. df342ad129cf2c3b8eb8da9d9d0ab845 slock-0.9.tar.gz df342ad129cf2c3b8eb8da9d9d0ab845 slock-0.9.tar.gz + The package successfully compiles and builds into binary rpms on at least one primary architecture (x86_64). + All build dependencies are listed in BuildRequires. + No need to deal with locales. + Does not contain shared libraries. + The package does not designed to be relocatable. + A package owns all directories that it creates. + A package does list a file more than once in the spec %files listings. + Permissions on files are set properly. + The package has a %clean section, which contains rm -rf $RPM_BUILD_ROOT. + The package consistently uses macros. + The package contains code, or permissible content. + Does not contain large documentation files. + Includes only doc files in %doc. + No headers. + No static libraries. + The package does not contain pkgconfig(.pc) files. + The package does not contain library files with a suffix (e.g. libfoo.so.1.1). + No devel packages. + The package does not contain any .la libtool archives. + Does not need %{name}.desktop file. This is not really GUI application and does not need desktop file. But please add comment about this in spec before import. + The package does not own files or directories already owned by other packages. + At the beginning of %install, the package runs rm -rf $RPM_BUILD_ROOT. + All filenames in the package are valid UTF-8. + The package builds in mock. + The package functions as described. This package is APPROVED. New Package CVS Request ======================= Package Name: slock Short Description: Simple X display locker Owners: stalwart Branches: F-10 InitialCC: cvs done. Packages built, kudos to Alex and Kevin! |