Please do not reply directly to this email. All additional comments should be made in the comments box of this bug.
Summary: Review Request: slock - Simple X display locker
https://bugzilla.redhat.com/show_bug.cgi?id=487737
Summary: Review Request: slock - Simple X display locker Product: Fedora Version: rawhide Platform: All OS/Version: Linux Status: NEW Severity: medium Priority: medium Component: Package Review AssignedTo: nobody@fedoraproject.org ReportedBy: stlwrt@gmail.com QAContact: extras-qa@fedoraproject.org CC: notting@redhat.com, fedora-package-review@redhat.com Estimated Hours: 0.0 Classification: Fedora
Spec URL: http://rpm.scwlab.com/fedora/slock/slock.spec SRPM URL: http://rpm.scwlab.com/fedora/slock/slock-0.9-1.fc10.src.rpm Built RPMs for F10 i386 and x86_64: http://rpm.scwlab.com/fedora/slock/
Description: slock is a simple X display locker. Really this is the simplest X screen locker we are aware of. It is stable and quite a lot people in this community are using it every day when they are out with friends or fetching some food from the local pub.
Please do not reply directly to this email. All additional comments should be made in the comments box of this bug.
https://bugzilla.redhat.com/show_bug.cgi?id=487737
Alexey Torkhov atorkhov@gmail.com changed:
What |Removed |Added ---------------------------------------------------------------------------- CC| |atorkhov@gmail.com
--- Comment #1 from Alexey Torkhov atorkhov@gmail.com 2009-03-08 11:24:56 EDT --- This package does not use standard rpm compiler flags when building: https://fedoraproject.org/wiki/Packaging/Guidelines#Compiler_flags
Please do not reply directly to this email. All additional comments should be made in the comments box of this bug.
https://bugzilla.redhat.com/show_bug.cgi?id=487737
--- Comment #2 from Pavel Shevchuk stlwrt@gmail.com 2009-03-08 12:08:45 EDT --- Now it does.
* Sun Mar 08 2009 Pavel "Stalwart" Shevchuk stlwrt@gmail.com - 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/
Please do not reply directly to this email. All additional comments should be made in the comments box of this bug.
https://bugzilla.redhat.com/show_bug.cgi?id=487737
--- Comment #3 from Alexey Torkhov atorkhov@gmail.com 2009-03-08 12:21:01 EDT --- 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.
Please do not reply directly to this email. All additional comments should be made in the comments box of this bug.
https://bugzilla.redhat.com/show_bug.cgi?id=487737
--- Comment #4 from Pavel Shevchuk stlwrt@gmail.com 2009-03-15 13:57:10 EDT --- * Sun Mar 15 2009 Pavel "Stalwart" Shevchuk stlwrt@gmail.com - 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/
Please do not reply directly to this email. All additional comments should be made in the comments box of this bug.
https://bugzilla.redhat.com/show_bug.cgi?id=487737
Alexey Torkhov atorkhov@gmail.com changed:
What |Removed |Added ---------------------------------------------------------------------------- Status|NEW |ASSIGNED AssignedTo|nobody@fedoraproject.org |atorkhov@gmail.com Flag| |fedora-review?
--- Comment #5 from Alexey Torkhov atorkhov@gmail.com 2009-03-15 17:08:39 EDT --- - 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
Please do not reply directly to this email. All additional comments should be made in the comments box of this bug.
https://bugzilla.redhat.com/show_bug.cgi?id=487737
--- Comment #6 from Pavel Shevchuk stlwrt@gmail.com 2009-03-15 17:13:52 EDT --- This is NOT a gui application, it's expected to be ran by windowmanager, not user
Please do not reply directly to this email. All additional comments should be made in the comments box of this bug.
https://bugzilla.redhat.com/show_bug.cgi?id=487737
--- Comment #7 from Alexey Torkhov atorkhov@gmail.com 2009-03-15 17:41:36 EDT --- And there are standard way for window manager to choose locker? How it will know of slock existence?
Please do not reply directly to this email. All additional comments should be made in the comments box of this bug.
https://bugzilla.redhat.com/show_bug.cgi?id=487737
--- Comment #8 from Pavel Shevchuk stlwrt@gmail.com 2009-03-15 19:13:36 EDT --- 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.
Please do not reply directly to this email. All additional comments should be made in the comments box of this bug.
https://bugzilla.redhat.com/show_bug.cgi?id=487737
--- Comment #9 from Alexey Torkhov atorkhov@gmail.com 2009-03-16 04:22:16 EDT --- Created an attachment (id=335317) --> (https://bugzilla.redhat.com/attachment.cgi?id=335317) 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.
Please do not reply directly to this email. All additional comments should be made in the comments box of this bug.
https://bugzilla.redhat.com/show_bug.cgi?id=487737
--- Comment #10 from Pavel Shevchuk stlwrt@gmail.com 2009-03-22 19:54:31 EDT --- * Mon Mar 23 2009 Pavel "Stalwart" Shevchuk stlwrt@gmail.com - 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
Please do not reply directly to this email. All additional comments should be made in the comments box of this bug.
https://bugzilla.redhat.com/show_bug.cgi?id=487737
Alexey Torkhov atorkhov@gmail.com changed:
What |Removed |Added ---------------------------------------------------------------------------- Flag|fedora-review? |fedora-review+
--- Comment #11 from Alexey Torkhov atorkhov@gmail.com 2009-03-23 16:13:41 EDT --- + 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.
Please do not reply directly to this email. All additional comments should be made in the comments box of this bug.
https://bugzilla.redhat.com/show_bug.cgi?id=487737
--- Comment #12 from Pavel Shevchuk stlwrt@gmail.com 2009-03-23 17:29:24 EDT --- New Package CVS Request ======================= Package Name: slock Short Description: Simple X display locker Owners: stalwart Branches: F-10 InitialCC:
Please do not reply directly to this email. All additional comments should be made in the comments box of this bug.
https://bugzilla.redhat.com/show_bug.cgi?id=487737
Pavel Shevchuk stlwrt@gmail.com changed:
What |Removed |Added ---------------------------------------------------------------------------- Flag| |fedora-cvs?
Please do not reply directly to this email. All additional comments should be made in the comments box of this bug.
https://bugzilla.redhat.com/show_bug.cgi?id=487737
Kevin Fenzi kevin@tummy.com changed:
What |Removed |Added ---------------------------------------------------------------------------- Flag|fedora-cvs? |fedora-cvs+
--- Comment #13 from Kevin Fenzi kevin@tummy.com 2009-03-24 13:16:30 EDT --- cvs done.
Please do not reply directly to this email. All additional comments should be made in the comments box of this bug.
https://bugzilla.redhat.com/show_bug.cgi?id=487737
Pavel Shevchuk stlwrt@gmail.com changed:
What |Removed |Added ---------------------------------------------------------------------------- Status|ASSIGNED |CLOSED Resolution| |NEXTRELEASE
--- Comment #14 from Pavel Shevchuk stlwrt@gmail.com 2009-03-27 12:13:19 EDT --- Packages built, kudos to Alex and Kevin!
package-review@lists.fedoraproject.org
-
Red Hat Bugzilla