Bug 1295144 - Review Request: xss-lock - Use external locker as X screen saver
Summary: Review Request: xss-lock - Use external locker as X screen saver
Keywords:
Status: CLOSED ERRATA
Alias: None
Product: Fedora
Classification: Fedora
Component: Package Review
Version: rawhide
Hardware: All
OS: Linux
unspecified
medium
Target Milestone: ---
Assignee: Eduardo Mayorga
QA Contact: Fedora Extras Quality Assurance
URL:
Whiteboard:
Depends On:
Blocks:
TreeView+ depends on / blocked
 
Reported: 2016-01-02 20:19 UTC by Martin Ueding
Modified: 2016-07-29 09:42 UTC (History)
3 users (show)

Fixed In Version:
Doc Type: Bug Fix
Doc Text:
Clone Of:
Environment:
Last Closed: 2016-04-03 17:51:20 UTC
Type: ---
Embargoed:
e: fedora-review+


Attachments (Terms of Use)
spec file (2.45 KB, application/octet-stream)
2016-01-02 23:03 UTC, gil cattaneo
no flags Details

Description Martin Ueding 2016-01-02 20:19:17 UTC
I would like to include xss-lock in Fedora. This is my first Fedora package
and I am looking for a sponsor.

Koji build: http://koji.fedoraproject.org/koji/taskinfo?taskID=12381915

Spec URL: http://chaos.stw-bonn.de/users/mu/uploads/tmp/2016-01-02/xss-lock.spec
SRPM URL: http://chaos.stw-bonn.de/users/mu/uploads/tmp/2016-01-02/xss-lock-0.3.0-1.20140302git.fc23.src.rpm

Description:

*xss-lock* hooks up your favorite locker to the MIT screen saver extension for
X and also to systemd's login manager. The locker is executed in response to
events from these two sources:

- X signals when screen saver activation is forced or after a period of user
  inactivity (as set with `xset s TIMEOUT`). In the latter case, the notifier
  command, if specified, is executed first.

- The login manager can also request that the session be locked; as a result of
  `loginctl lock-sessions`, for example. Additionally, **xss-lock** uses the
  inhibition logic to lock the screen before the system goes to sleep.

*xss-lock* waits for the locker to exit -- or kills it when screen saver
deactivation or session unlocking is forced -- so the command should not fork.

Also, *xss-lock* manages the idle hint on the login session. The idle state of
the session is directly linked to user activity as reported by X (except when
the notifier runs before locking the screen). When all sessions are idle, the
login manager can take action (such as suspending the system) after a
preconfigured delay.


Fedora Account System Username: martinueding

Comment 1 gil cattaneo 2016-01-02 20:58:51 UTC
hi
Release:	1.%{checkout}%{?dist}
must be
Release:	0.1.%{checkout}%{?dist}
https://fedoraproject.org/wiki/Packaging:NamingGuidelines#Pre-Release_packages

If the source package includes the text of the license, then that file, containing the text of the license for the package is included in %license.

If the source package does not include license text, the packager SHOULD query upstream to include it.
https://fedoraproject.org/wiki/Packaging:LicensingGuidelines?rd=Packaging/LicensingGuidelines#License_Text

you must use rpm macros (rpm --showrc)

%files

/usr/bin/xss-lock
/usr/share/bash-completion/completions/xss-lock
%doc /usr/share/doc/xss-lock/NEWS
%doc /usr/share/doc/xss-lock/dim-screen.sh
%doc /usr/share/doc/xss-lock/transfer-sleep-lock-generic-delay.sh
%doc /usr/share/doc/xss-lock/transfer-sleep-lock-i3lock.sh
%doc /usr/share/doc/xss-lock/xdg-screensaver.patch
%doc /usr/share/man/man1/xss-lock.1.gz
/usr/share/zsh/site-functions/_xss-lock

%changelog

become

%files
%{_bindir}/xss-lock
%{_datadir}/bash-completion/completions/xss-lock
%{_datadir}/zsh/site-functions/_xss-lock
%{_mandir}/man1/xss-lock.1.gz
%doc NEWS

%doc dim-screen.sh transfer-sleep-lock-generic-delay.sh transfer-sleep-lock-i3lock.sh xdg-screensaver.patch

%changelog
* Sat Jan 02 2016 Martin Ueding <von.fedora> 0.3.0-0.1.20140302git
- initial rpm

Comment 2 gil cattaneo 2016-01-02 21:04:39 UTC
about hardcode path in SPEC https://fedoraproject.org/wiki/Packaging:Guidelines#Macros

Comment 3 gil cattaneo 2016-01-02 21:10:05 UTC
add a comment in SPEC how generate the taraball https://fedoraproject.org/wiki/Packaging:SourceURL?rd=Packaging/SourceURL#Using_Revision_Control

Comment 4 Martin Ueding 2016-01-02 21:34:04 UTC
Hi Gil,

thank you for the constructive feedback!

- I changed the `Release` to include the leading zero.

- Regarding the `%license` I am not so sure what I should exactly do with it.
  Is it correct to add this to the `%files` section as I did?

- And the changelog is populated with an item now.

- The way to create the tar archive is documented in the SPEC file now as well.

The new spec file (as of comment 5) is at:
http://chaos.stw-bonn.de/users/mu/uploads/tmp/2016-01-02/xss-lock-5.spec

Regards,

Martin

Comment 5 Eduardo Mayorga 2016-01-02 21:44:03 UTC
Could you please update the SRPM URL as well?

Comment 6 Martin Ueding 2016-01-02 21:50:04 UTC
Certainly! This is the new SRPM URL:

http://chaos.stw-bonn.de/users/mu/uploads/tmp/2016-01-02/xss-lock-0.3.0-0.1.20140302git.fc23.src.rpm

Comment 7 gil cattaneo 2016-01-02 22:41:51 UTC
(In reply to Martin Ueding from comment #4)
> Hi Gil,
> 
> thank you for the constructive feedback!
> 
> - I changed the `Release` to include the leading zero.
this is not a pre-release than you should use



%global commit0 1e158fb
%global shortcommit0 %(c=%{commit0}; echo ${c:0:11})
Name:		xss-lock

Release:	1%{?dist}

Source0:  https://bitbucket.org/raymonad/%{name}/get/%{commit0}.tar.gz#/%{name}-%{shortcommit0}.tar.gz

%changelog
* Sat Jan 02 2016 Martin Ueding <von.fedora> 0.3.0-1
- initial rpm

https://fedoraproject.org/wiki/Packaging:SourceURL?rd=Packaging/SourceURL#Git_Tags

> - Regarding the `%license` I am not so sure what I should exactly do with it.
>   Is it correct to add this to the `%files` section as I did?
yes, well done

Please, remove:
%define checkout 20140302git
or use (i don't understand what it is needed, for me should better use the above example)
%global checkout 20140302git

https://fedoraproject.org/wiki/PackagingDrafts/global_preferred_over_define

Comment 8 gil cattaneo 2016-01-02 22:44:21 UTC
... forgot, use
%prep
%setup -qn raymonad-%{name}-%{shortcommit0}

Comment 9 gil cattaneo 2016-01-02 23:03:49 UTC
Created attachment 1111087 [details]
spec file

Comment 10 gil cattaneo 2016-01-02 23:07:08 UTC
Ops ... you use a pre-relase (?). you must explain, in the SPEC, the why of this choice

Comment 11 Michael Schwendt 2016-01-03 04:42:55 UTC
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.

Comment 12 gil cattaneo 2016-01-03 11:29:22 UTC
(In reply to Michael Schwendt from comment #11)
> 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.
> 

Please, read this thread: https://lists.fedoraproject.org/archives/list/devel%40lists.fedoraproject.org/thread/AS35NKZSAWRIKY77IUYOVNFAT6AJQVAU/#GW7VY3RY76ZUH6X72FLOOEDCHURKWWI4

"%define" should be replaced, in each case (rare exceptions) by "%global" 

> 
> 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.
I see, comment#10

Comment 13 Martin Ueding 2016-01-04 09:13:47 UTC
In response to gil (#7):

> %global commit0 1e158fb
> %global shortcommit0 %(c=%{commit0}; echo ${c:0:11})

I only got it to work with the following:

    %global commit0 1e158fb20108
    %global shortcommit0 %(c=%{commit0}; echo ${c:0:7})

    Source0:	https://bitbucket.org/raymonad/%{name}/get/%{commit0}.tar.gz#/%{name}-%{shortcommit0}.tar.gz

    %setup -qn raymonad-%{name}-%{commit0}

The hash used in the URLs is also just seven characters long, perhaps they have
changed it? I had to deviate quite a bit from
[the wiki](https://fedoraproject.org/wiki/Packaging:SourceURL?rd=Packaging/SourceURL#Git_Hosting_Services)
so I would assume that the content in the wiki is a bit outdated. If so I would
happily update them.

----

In response to gil (#7) and later Michael (#11):

The `%define` was not chosen over `%global` with any particular thought from
me. I just searched for “rpm define variable” and stumbled accross [a SO
post](http://stackoverflow.com/a/10236104) where somebody used `%define`.
Having a C++ background I did not think that there could be another thing that
would be better.

I just want to reduce repetition (Don't Repeat Yourself) and introduced a
variable. Now it uses `%global`.

----

Then about the whole pre-release thing: The version that I use from git is a
couple commits *after* 0.3.0 and there is no more recent tag in the upstream
repository. Previously I have used xss-lock 0.3.0 on Ubuntu 15.04 and had a bug
with it, it would not exit when I logged out. The repository contains commits
which suggest that this bug has been addressed but not yet released.

I have packaged the latest version in git in my Open Build Service home project
and used for a week now, it seems fine.

In the sense of xss-lock this should be a post-release version.

In terms of the Fedora package I assumed that the Fedora package would be a
pre-release. In Debian they upload all the proposed packages with `Release: 0`
so to speak. As this is not an official package yet, I thought having `Release:
0.1` was the thing to do here.

----

In response to Michael (#11):

> [rpm macros]

I have copied the output from `rpmbuild` complaining about unpackaged files
there. In the wiki I read that the use of macros just has to be consistent but
is not mandatory. Should I now change it back to use the hardcoded paths?

>> Requires:	xcb-util libxcb glib2
> https://fedoraproject.org/wiki/Packaging:Guidelines#Explicit_Requires

I read the paragraph there but I am not sure what you want to tell me. Should I
add some explicit version there?

> [cmake compile step]

This part originated from an openSUSE package that I did for another project. I
did not really look into detail there and just assumed that it cannot be too
wrong as it works.

In the log I see that the old version gave the flags twice which seems to be
nonsensical. Also the compile commands are not visible which is not helpful.
And there are a couple warnings which are not that important. The one

    warning: 'atom' may be used uninitialized in this function

should be something to look into.

The `%cmake` already adds verbose output, so this should be fine now.

> [bash completion]

Quoting
[File and Directory
Ownership](https://fedoraproject.org/wiki/Packaging:Guidelines#File_and_Directory_Ownership):
> Solution: Both the `git` and `bzr` packages should own the
> `/etc/bash_completion.d` directory as `bash-completion` is optional
> functionality and the installation of git or bzr should not force the
> installation of `bash-completion`. 

So from this I think that this package should own the whole directory. I have
changed that.

> [man page]

Nice to know, I changed that.

> When modifying the spec file during package review, bumping Release and
> updating %changelog is good practice.

Okay, in Debian they demand that the `Release` only tracks uploads to the
actual distributions. Fixing a package in review is not considered a new
release. The one after these comments will be `Release: 2` and I filled in the
changelog.

----

Thank you for all the helpful comments!

Here are the current files, bumped to `Release: 2` and formatted such that
`fedora-review` should pick it up:

Spec URL: http://chaos.stw-bonn.de/users/mu/uploads/2016-01-04/xss-lock.spec
SRPM URL: http://chaos.stw-bonn.de/users/mu/uploads/2016-01-04/xss-lock-0.3.0-2.20140302git.fc23.src.rpm

Comment 14 Eduardo Mayorga 2016-01-05 07:40:34 UTC
(In reply to Martin Ueding from comment #13)
> In response to gil (#7):
> 
> > %global commit0 1e158fb
> > %global shortcommit0 %(c=%{commit0}; echo ${c:0:11})
> 
> I only got it to work with the following:
> 
>     %global commit0 1e158fb20108
>     %global shortcommit0 %(c=%{commit0}; echo ${c:0:7})
> 
>     Source0:
> https://bitbucket.org/raymonad/%{name}/get/%{commit0}.tar.gz#/%{name}-
> %{shortcommit0}.tar.gz
> 
>     %setup -qn raymonad-%{name}-%{commit0}
> 
> The hash used in the URLs is also just seven characters long, perhaps they
> have
> changed it? I had to deviate quite a bit from
> [the
> wiki](https://fedoraproject.org/wiki/Packaging:SourceURL?rd=Packaging/
> SourceURL#Git_Hosting_Services)
> so I would assume that the content in the wiki is a bit outdated. If so I
> would
> happily update them.

See the entire commit hash: https://bitbucket.org/raymonad/xss-lock/commits/1e158fb20108058dbd62bd51d8e8c003c0a48717. So %{commit0} should be 1e158fb20108058dbd62bd51d8e8c003c0a48717.

In order to make this work (as suggested in the wiki):
  %setup -qn OWNER-%{name}-%{shortcommit0}
%{shortcommit0} has to be defined to the first 12 characters of the commit hash. %setup works now just because %{commit0} is (wrongly) defined to those 12 characters.

So you wind up with:
%global commit0 1e158fb20108058dbd62bd51d8e8c003c0a48717
%global shortcommit0 %(c=%{commit0}; echo ${c:0:11})

> Then about the whole pre-release thing: The version that I use from git is a
> couple commits *after* 0.3.0 and there is no more recent tag in the upstream
> repository. Previously I have used xss-lock 0.3.0 on Ubuntu 15.04 and had a
> bug
> with it, it would not exit when I logged out. The repository contains commits
> which suggest that this bug has been addressed but not yet released.
> 
> I have packaged the latest version in git in my Open Build Service home
> project
> and used for a week now, it seems fine.
> 
> In the sense of xss-lock this should be a post-release version.
> 
> In terms of the Fedora package I assumed that the Fedora package would be a
> pre-release. In Debian they upload all the proposed packages with `Release:
> 0`
> so to speak. As this is not an official package yet, I thought having
> `Release:
> 0.1` was the thing to do here.

As Michael said in comment #11, this is a post-release snapshot because there was already a final 0.3.0 released, so you must follow the Naming Guidelines for Snapshot packages.

https://fedoraproject.org/wiki/Packaging:NamingGuidelines#Snapshot_packages

> I have copied the output from `rpmbuild` complaining about unpackaged files
> there. In the wiki I read that the use of macros just has to be consistent
> but
> is not mandatory. Should I now change it back to use the hardcoded paths?

Please no. Macros simplify things and there are several good reasons to use then over hardcoded paths.

> >> Requires:	xcb-util libxcb glib2
> > https://fedoraproject.org/wiki/Packaging:Guidelines#Explicit_Requires
> 
> I read the paragraph there but I am not sure what you want to tell me.
> Should I
> add some explicit version there?

rpmbuild adds those dependencies automatically so there is no need to add them manually in the spec file.

Note the rpmlint errors:
xss-lock.x86_64: E: explicit-lib-dependency glib2
xss-lock.x86_64: E: explicit-lib-dependency libxcb

Address these issues and I'll review it again.

Comment 15 Martin Ueding 2016-01-05 13:46:35 UTC
In order to get it to work I have to use characters 0 to 12:

    %global commit0 1e158fb20108058dbd62bd51d8e8c003c0a48717
    %global shortcommit0 %(c=%{commit0}; echo ${c:0:12})

As far as I understand the naming guidelines for snapshot packages the version
`0.3.0-2.20140302git` should be correct, isn't it? If not, what would be the
appropriate version?

The finding of runtime dependencies is nice! I removed the explicit libraries
from the `Requires:` part.

I ran `rpmlint -i` over the RPM and the SRPM, they are clean now (expect false
positives for spelling).

Spec URL: http://chaos.stw-bonn.de/users/mu/uploads/2016-01-05/xss-lock.spec
SRPM URL: http://chaos.stw-bonn.de/users/mu/uploads/2016-01-05/xss-lock-0.3.0-3.20140302git.fc23.src.rpm
RPM URL: http://chaos.stw-bonn.de/users/mu/uploads/2016-01-05/xss-lock-0.3.0-3.20140302git.fc23.x86_64.rpm

Comment 16 Eduardo Mayorga 2016-01-06 16:35:27 UTC
According to the current contributor agreement (which you have already signed), the default license for code contributions is MIT, so the copyright statement on the top of the spec looks just fine. Note that the Default License could change in the future, though. See: https://fedoraproject.org/wiki/Legal:Fedora_Project_Contributor_Agreement

Package Review
==============

Legend:
[x] = Pass, [!] = Fail, [-] = Not applicable, [?] = Not evaluated


Issues:
=======
- Directories without known owners: /usr/share/zsh
  The package must own this directory too. So, in the %files section,
    %{_datadir}/zsh/site-functions/
  becomes
    %{_datadir}/zsh/


===== MUST items =====

C/C++:
[x]: Package does not contain kernel modules.
[x]: Package contains no static executables.
[x]: Header files in -devel subpackage, if present.
[x]: Package does not contain any libtool archives (.la)
[x]: Rpath absent or only used for internal libs.

Generic:
[x]: Package is licensed with an open-source compatible license and meets
     other legal requirements as defined in the legal section of Packaging
     Guidelines.
[x]: License field in the package spec file matches the actual license.
     Note: Checking patched sources after %prep for licenses. Licenses
     found: "MIT/X11 (BSD like)", "Unknown or generated". 16 files have
     unknown license. Detailed output of licensecheck in
     /home/mayorga/1295144-xss-lock/licensecheck.txt
[x]: License file installed when any subpackage combination is installed.
[!]: Package must own all directories that it creates.
     Note: Directories without known owners: /usr/share/zsh
[x]: Package does not own files or directories owned by other packages.
     Note: Dirs in package are owned also by: /usr/share/zsh/site-
     functions(systemd, zsh, pulseaudio), /usr/share/bash-
     completion/completions(npm, kmod, rpmdevtools, python-pip, rpmlint,
     firewalld, bash-completion, subversion, gvfs-client, tracker, yum,
     libappstream-glib, python3-pip, glib2, git-core)
     -> This is for optional functionality, this is OK.
[x]: %build honors applicable compiler flags or justifies otherwise.
[x]: Package contains no bundled libraries without FPC exception.
[x]: Changelog in prescribed format.
[x]: Sources contain only permissible code or content.
[-]: Package contains desktop file if it is a GUI application.
[-]: Development files must be in a -devel package
[x]: Package uses nothing in %doc for runtime.
[x]: Package consistently uses macros (instead of hard-coded directory
     names).
[x]: Package is named according to the Package Naming Guidelines.
[x]: Package does not generate any conflict.
[x]: Package obeys FHS, except libexecdir and /usr/target.
[-]: If the package is a rename of another package, proper Obsoletes and
     Provides are present.
[x]: Requires correct, justified where necessary.
[x]: Spec file is legible and written in American English.
[-]: Package contains systemd file(s) if in need.
[x]: Useful -debuginfo package or justification otherwise.
[x]: Package is not known to require an ExcludeArch tag.
[-]: Large documentation must go in a -doc subpackage. Large could be size
     (~1MB) or number of files.
     Note: Documentation size is 20480 bytes in 5 files.
[!]: Package complies to the Packaging Guidelines
[x]: Package successfully compiles and builds into binary rpms on at least
     one supported primary architecture.
[x]: Rpmlint is run on all rpms the build produces.
     Note: There are rpmlint messages (see attachment).
[x]: If (and only if) the source package includes the text of the
     license(s) in its own file, then that file, containing the text of the
     license(s) for the package is included in %license.
[x]: Package requires other packages for directories it uses.
[x]: All build dependencies are listed in BuildRequires, except for any
     that are listed in the exceptions section of Packaging Guidelines.
[x]: Package uses either %{buildroot} or $RPM_BUILD_ROOT
[x]: Package does not run rm -rf %{buildroot} (or $RPM_BUILD_ROOT) at the
     beginning of %install.
[x]: Macros in Summary, %description expandable at SRPM build time.
[x]: Dist tag is present.
[x]: Package does not contain duplicates in %files.
[x]: Permissions on files are set properly.
[x]: Package use %makeinstall only when make install DESTDIR=... doesn't
     work.
[x]: Package is named using only allowed ASCII characters.
[x]: Package does not use a name that already exists.
[x]: Package is not relocatable.
[x]: Sources used to build the package match the upstream source, as
     provided in the spec URL.
[x]: Spec file name must match the spec package %{name}, in the format
     %{name}.spec.
[x]: File names are valid UTF-8.
[x]: Packages must not store files under /srv, /opt or /usr/local

===== SHOULD items =====

Generic:
[-]: If the source package does not include license text(s) as a separate
     file from upstream, the packager SHOULD query upstream to include it.
[x]: Final provides and requires are sane (see attachments).
[-]: Fully versioned dependency in subpackages if applicable.
     Note: No Requires: %{name}%{?_isa} = %{version}-%{release} in xss-
     lock-debuginfo
[x]: Package functions as described.
[x]: Latest version is packaged.
[x]: Package does not include license text files separate from upstream.
[-]: Description and summary sections in the package spec file contains
     translations for supported Non-English languages, if available.
[?]: Package should compile and build into binary rpms on all supported
     architectures.
[-]: %check is present and all tests pass.
[-]: Packages should try to preserve timestamps of original installed
     files.
[x]: Packager, Vendor, PreReq, Copyright tags should not be in spec file
[x]: Sources can be downloaded from URI in Source: tag
[x]: Reviewer should test that the package builds in mock.
[x]: Buildroot is not present
[x]: Package has no %clean section with rm -rf %{buildroot} (or
     $RPM_BUILD_ROOT)
[x]: No file requires outside of /etc, /bin, /sbin, /usr/bin, /usr/sbin.
[x]: Uses parallel make %{?_smp_mflags} macro.
[x]: SourceX is a working URL.
[x]: Spec use %global instead of %define unless justified.

===== EXTRA items =====

Generic:
[x]: Rpmlint is run on all installed packages.
[x]: Large data in /usr/share should live in a noarch subpackage if package
     is arched.
[x]: Spec file according to URL is the same as in SRPM.


Rpmlint
-------
Checking: xss-lock-0.3.0-3.20140302git.fc24.x86_64.rpm
          xss-lock-debuginfo-0.3.0-3.20140302git.fc24.x86_64.rpm
          xss-lock-0.3.0-3.20140302git.fc24.src.rpm
xss-lock.x86_64: W: spelling-error %description -l en_US systemd's -> system's, systemic's, systems
xss-lock.x86_64: W: spelling-error %description -l en_US xset -> set, x set, sextet
xss-lock.x86_64: W: spelling-error %description -l en_US loginctl -> login
xss-lock.x86_64: W: spelling-error %description -l en_US preconfigured -> reconfigured, p reconfigured, reconfigure
xss-lock.src: W: spelling-error %description -l en_US systemd's -> system's, systemic's, systems
xss-lock.src: W: spelling-error %description -l en_US xset -> set, x set, sextet
xss-lock.src: W: spelling-error %description -l en_US loginctl -> login
xss-lock.src: W: spelling-error %description -l en_US preconfigured -> reconfigured, p reconfigured, reconfigure
3 packages and 0 specfiles checked; 0 errors, 8 warnings.




Requires
--------
xss-lock (rpmlib, GLIBC filtered):
    libc.so.6()(64bit)
    libgio-2.0.so.0()(64bit)
    libglib-2.0.so.0()(64bit)
    libgobject-2.0.so.0()(64bit)
    libxcb-screensaver.so.0()(64bit)
    libxcb-util.so.1()(64bit)
    libxcb.so.1()(64bit)
    rtld(GNU_HASH)
    xcb-util

xss-lock-debuginfo (rpmlib, GLIBC filtered):



Provides
--------
xss-lock:
    xss-lock
    xss-lock(x86-64)

xss-lock-debuginfo:
    xss-lock-debuginfo
    xss-lock-debuginfo(x86-64)



Source checksums
----------------
https://bitbucket.org/raymonad/xss-lock/get/1e158fb20108058dbd62bd51d8e8c003c0a48717.tar.gz#/xss-lock-1e158fb20108.tar.gz :
  CHECKSUM(SHA256) this package     : 19c9e55fd14346e79a1cfaa400cb51ef467d9e3f482fb28a38ba2bbc2c972d91
  CHECKSUM(SHA256) upstream package : 19c9e55fd14346e79a1cfaa400cb51ef467d9e3f482fb28a38ba2bbc2c972d91

Comment 17 Martin Ueding 2016-01-06 17:01:38 UTC
> [!]: Package must own all directories that it creates.

That is fixed now as by your suggestion.

> [!]: Package complies to the Packaging Guidelines

This one does not help me that much. Is that implied by the previous point or still something I am not aware of?


Current version with `zsh` fixed:

Spec URL: http://chaos.stw-bonn.de/users/mu/uploads/2016-01-06/xss-lock.spec
SRPM URL: http://chaos.stw-bonn.de/users/mu/uploads/2016-01-06/xss-lock-0.3.0-4.20140302git.fc23.src.rpm

Comment 18 Eduardo Mayorga 2016-01-06 17:40:29 UTC
It looks fine.

PACKAGE APPROVED

I'll sponsor you but I want to see you involved in informal reviews first.

Comment 19 Eduardo Mayorga 2016-03-12 04:11:53 UTC
I have sponsored you in the Fedora packagers group. Welcome! Please follow the process from: 
https://fedoraproject.org/wiki/Join_the_package_collection_maintainers#Add_Package_to_Source_Code_Management_.28SCM.29_system_and_Set_Owner

If you have any questions, feel free to contact me via email or in the IRC channel #fedora-devel. My nick is mayorga.

Comment 20 Gwyn Ciesla 2016-03-14 13:11:09 UTC
Package request has been approved: https://admin.fedoraproject.org/pkgdb/package/xss-lock

Comment 21 Fedora Update System 2016-03-25 18:20:35 UTC
xss-lock-0.3.0-4.20140302git.fc23 has been submitted as an update to Fedora 23. https://bodhi.fedoraproject.org/updates/FEDORA-2016-bdb30b18c7

Comment 22 Fedora Update System 2016-03-25 18:40:02 UTC
xss-lock-0.3.0-4.20140302git.fc24 has been submitted as an update to Fedora 24. https://bodhi.fedoraproject.org/updates/FEDORA-2016-1e35732096

Comment 23 Fedora Update System 2016-03-26 23:49:07 UTC
xss-lock-0.3.0-4.20140302git.fc23 has been pushed to the Fedora 23 testing repository. If problems still persist, please make note of it in this bug report.
See https://fedoraproject.org/wiki/QA:Updates_Testing for
instructions on how to install test updates.
You can provide feedback for this update here: https://bodhi.fedoraproject.org/updates/FEDORA-2016-bdb30b18c7

Comment 24 Fedora Update System 2016-03-26 23:52:33 UTC
xss-lock-0.3.0-4.20140302git.fc24 has been pushed to the Fedora 24 testing repository. If problems still persist, please make note of it in this bug report.
See https://fedoraproject.org/wiki/QA:Updates_Testing for
instructions on how to install test updates.
You can provide feedback for this update here: https://bodhi.fedoraproject.org/updates/FEDORA-2016-1e35732096

Comment 25 Fedora Update System 2016-04-03 17:51:18 UTC
xss-lock-0.3.0-4.20140302git.fc23 has been pushed to the Fedora 23 stable repository. If problems still persist, please make note of it in this bug report.

Comment 26 Fedora Update System 2016-04-03 17:58:04 UTC
xss-lock-0.3.0-4.20140302git.fc24 has been pushed to the Fedora 24 stable repository. If problems still persist, please make note of it in this bug report.


Note You need to log in before you can comment on or make changes to this bug.