Initial review:

1. "BuildRequires:  gcc-c++" is not needed, you can remove it.

2. You can simplify "%setup -q -n sddm-%{version}" to "%setup -q"

3. Systemd units files are handled incorrectly IMO.

Ref: http://fedoraproject.org/wiki/Packaging:ScriptletSnippets#Systemd

4. Release tag should be 1%{?dist}, after the fix should be 2%{?dist}

Spec URL: http://mbriza.fedorapeople.org/sddm/sddm.spec
SRPM URL: http://mbriza.fedorapeople.org/sddm/sddm-0.1.0-2.fc19.src.rpm
Thanks for the review, Christopher, I fixed the following problems:
- Removed unneeded BuildRequires.
- Fixed systemd scriptlets.
- Fixed release.
- Simplified setup.
- Added Requires needed for basic function.
- Added Provides for graphical login.

Hmmm...

Why do we need "BuildRequires:  systemd-devel"? Isn't it "BuildRequires:  systemd"?

Not if this links to one of the C libraries that are part of systemd.

Hi,

1. This morning I tried a mock build but failed with:

Executing(%build): /bin/sh -e /var/tmp/rpm-tmp.e633jH
+ umask 022
+ cd /builddir/build/BUILD
+ cd sddm-0.1.0
+ mkdir -p i686-redhat-linux-gnu
+ sed -i s/-march=native// CMakeLists.txt
+ pushd i686-redhat-linux-gnu
~/build/BUILD/sddm-0.1.0/i686-redhat-linux-gnu ~/build/BUILD/sddm-0.1.0
+ '%{cmake}' ..
/var/tmp/rpm-tmp.e633jH: line 35: fg: no job control
error: Bad exit status from /var/tmp/rpm-tmp.e633jH (%build)
    Bad exit status from /var/tmp/rpm-tmp.e633jH (%build)
RPM build errors:
Child return code was: 1
EXCEPTION: Command failed. See logs for output.
 # ['bash', '--login', '-c', 'rpmbuild -bb --target i686 --nodeps builddir/build/SPECS/sddm.spec']
Traceback (most recent call last):
  File "/usr/lib/python2.7/site-packages/mockbuild/trace_decorator.py", line 70, in trace
    result = func(*args, **kw)
  File "/usr/lib/python2.7/site-packages/mockbuild/util.py", line 359, in do
    raise mockbuild.exception.Error, ("Command failed. See logs for output.\n # %s" % (command,), child.returncode)
Error: Command failed. See logs for output.
 # ['bash', '--login', '-c', 'rpmbuild -bb --target i686 --nodeps builddir/build/SPECS/sddm.spec']
LEAVE do --> EXCEPTION RAISED

Something is wrong in the cmake, I see two periods.

2. And one more question about your spec, I can see these lines in %files:

%config   %{_sysconfdir}/pam.d/sddm
%config   %{_sysconfdir}/sddm.conf

Can these be %config(noreplace)?

3. I think you should choose tarball https://github.com/sddm/sddm/archive/0.1.0.tar.gz

as Source0 as this software will grow bigger and bigger.

> + '%{cmake}' ..
> /var/tmp/rpm-tmp.e633jH: line 35: fg: no job control

Missing "BuildRequires: cmake".

("fg: no job control" is usually the result of an undefined RPM macro. RPM then leaves the macro unexpanded, so bash tries to interpret it as some job control thingy, which obviously fails.)

> Something is wrong in the cmake, I see two periods.

No, that is perfectly fine cmake command-line syntax. (CMake takes a directory on the command line, .. is the parent directory, which is what you want if you're building in a subdirectory.)

> 3. I think you should choose tarball
> https://github.com/sddm/sddm/archive/0.1.0.tar.gz
>
> as Source0 as this software will grow bigger and bigger.

tar.gz or zip doesn't make much difference compression-wise, they're both using deflate as the algorithm. The tar.gz might be slightly smaller because it is always a solid archive (it first tars up everything and then compresses the whole thing as one file), but I don't expect a big difference. A tar.xz or at least tar.bz2 would be helpful size-wise, if github can deliver that (but I wouldn't recompress the upstream tarball if they only ship tar.gz, doing that is frowned upon because it makes it harder to verify authenticity).

That said, I'd pick the tar.gz over the zip for other practical reasons, e.g. better support for unixy stuff such as symlinks, special permissions etc.

Spec URL: http://mbriza.fedorapeople.org/sddm/sddm.spec
SRPM URL: http://mbriza.fedorapeople.org/sddm/sddm-0.1.0-2.fc19.src.rpm

(In reply to Christopher Meng from comment #3)
> Why do we need "BuildRequires:  systemd-devel"? Isn't it "BuildRequires: 
> systemd"?

You're right, I thought the systemd pkg-config module is in the -devel package. Fixed.

(In reply to Christopher Meng from comment #5)
> Something is wrong in the cmake, I see two periods.

Missing BuildRequires: cmake; fixed.

> 2. And one more question about your spec, I can see these lines in %files:
> 
> %config   %{_sysconfdir}/pam.d/sddm
> %config   %{_sysconfdir}/sddm.conf
> 
> Can these be %config(noreplace)?

Fixed.

> 3. I think you should choose tarball
> https://github.com/sddm/sddm/archive/0.1.0.tar.gz
> 
> as Source0 as this software will grow bigger and bigger.

Changed as per Kevin's reasoning in comment #6. Unfortunately, there isn't an option to download xz or bz2 packages.

SRPM URL: http://mbriza.fedorapeople.org/sddm/sddm-0.1.0-3.fc19.src.rpm
...

No problems here.

Only 1 thing, you should add README and CONTRIBUTORS and especially COPYING as %doc.

Once you upload the -4 spec, I'll approve.

Spec URL: http://mbriza.fedorapeople.org/sddm/sddm.spec
SRPM URL: http://mbriza.fedorapeople.org/sddm/sddm-0.1.0-4.fc19.src.rpm

(In reply to Christopher Meng from comment #9)
> Only 1 thing, you should add README and CONTRIBUTORS and especially COPYING
> as %doc.

Done.

Thank you and sorry for the delay.

Alright, approved.

I'd like to test sddm on f19.  I have built/installed 0.1.0-4.  I did not see any instructions on how to test - hints?

Neal, 
using the following command (as root):
 systemctl enable --force sddm.service
will replace your current DM with SDDM. On the next reboot, you'll get SDDM.
If you won't want to reboot, switch to an other VT ([Ctrl + Alt + F6] for example), log in as root and use the following commands:
 systemctl stop display-manager.service # this will switch you to VT1, most probably, so then switch to VT6 again with [Alt + F6]
 systemctl start sddm.service
I have described some basic steps on how to test the package on https://fedoraproject.org/wiki/Changes/SDDMinsteadOfKDM . However, many of the steps don't apply, as some needed features aren't implemented yet in this version.

Well that failed.
2 notes on my config:
* I'm using kde
* I'm using nvidia blob

Jul 22 08:13:18 nbecker1 sddm[27151]: DAEMON: Initializing...
Jul 22 08:13:18 nbecker1 sddm[27151]: DAEMON: Adding new display ":0" ...
Jul 22 08:13:18 nbecker1 sddm[27151]: DAEMON: Starting...
Jul 22 08:13:18 nbecker1 sddm[27151]: DAEMON: Generating cookie...
Jul 22 08:13:18 nbecker1 sddm[27151]: DAEMON: Adding cookie to "/var/run/xauth/A:0-CiEIvq"
Jul 22 08:13:18 nbecker1 sddm[27151]: /usr/bin/xauth:  file /var/run/xauth/A:0-CiEIvq does not exist
Jul 22 08:13:18 nbecker1 sddm[27151]: DAEMON: Display server starting...
Jul 22 08:13:18 nbecker1 sddm[27151]: DAEMON: Display server started.
Jul 22 08:13:18 nbecker1 sddm[27151]: DAEMON: Socket server starting...
Jul 22 08:13:18 nbecker1 sddm[27151]: DAEMON: Socket server started.
Jul 22 08:13:18 nbecker1 sddm[27151]: DAEMON: Greeter starting...
Jul 22 08:13:18 nbecker1 sddm[27151]: DAEMON: Greeter started.
Jul 22 08:13:20 nbecker1 sddm[27151]: DAEMON: Message received from greeter: Connect
Jul 22 08:13:26 nbecker1 sddm[27151]: DAEMON: Message received from greeter: Login
Jul 22 08:13:26 nbecker1 systemd-logind[397]: New session 52 of user nbecker.
Jul 22 08:13:26 nbecker1 sddm[27151]: DAEMON: User session started.
Jul 22 08:13:28 nbecker1 fprintd[26972]: ** Message: No devices in use, exit
Jul 22 08:13:29 nbecker1 sddm[27151]: DAEMON: User session ended.
Jul 22 08:13:29 nbecker1 sddm[27151]: DAEMON: Greeter stopping...
Jul 22 08:13:29 nbecker1 sddm[27151]: DAEMON: Greeter stopped.
Jul 22 08:13:29 nbecker1 sddm[27151]: DAEMON: Socket server stopping...
Jul 22 08:13:29 nbecker1 sddm[27151]: DAEMON: Socket server stopped.
Jul 22 08:13:29 nbecker1 sddm[27151]: DAEMON: Display server stopping...
Jul 22 08:13:31 nbecker1 systemd[1]: Started Getty on tty6.
Jul 22 08:13:31 nbecker1 abrt[27346]: Saved core dump of pid 27154 (/usr/bin/Xorg) to /var/tmp/abrt/ccpp-2013-07-22-08:13:31-27154 (23814144 bytes)
Jul 22 08:13:31 nbecker1 abrtd: Directory 'ccpp-2013-07-22-08:13:31-27154' creation detected
Jul 22 08:13:31 nbecker1 abrtd: Generating core_backtrace
Jul 22 08:13:31 nbecker1 abrtd: Generating backtrace
Jul 22 08:13:31 nbecker1 abrtd: Backtrace is generated, 8355 bytes
Jul 22 08:13:31 nbecker1 abrtd: Cannot disassemble function at 0x0, not computing fingerprint
Jul 22 08:13:31 nbecker1 abrtd: dwarf_next_cfi failed for /usr/lib64/nvidia-304xx/libnvidia-glcore.so.304.88: invalid DWARF
Jul 22 08:13:31 nbecker1 abrtd: Failed to read .eh_frame function ranges from /usr/lib64/nvidia-304xx/libnvidia-glcore.so.304.88
Jul 22 08:13:31 nbecker1 abrtd: Core backtrace is generated and saved, 1337 bytes

Per

Jul 22 08:13:18 nbecker1 sddm[27151]: DAEMON: Adding cookie to
"/var/run/xauth/A:0-CiEIvq"
Jul 22 08:13:18 nbecker1 sddm[27151]: /usr/bin/xauth:  file
/var/run/xauth/A:0-CiEIvq does not exist

Looks like you'll want to create/own this dir at least, though it may be wise to configure the defaults to use something sddm-specific, like:
/var/run/sddm/
instead.

Anyway, see lightdm for an example of how to do this,
http://pkgs.fedoraproject.org/cgit/lightdm.git/tree/lightdm.spec
http://pkgs.fedoraproject.org/cgit/lightdm.git/tree/lightdm-tmpfiles.conf

New Package SCM Request
=======================
Package Name: sddm
Short Description: QML based X11 desktop manager
Owners: mbriza rdieter kkofler ltinkl dvratil jgrulich
Branches: f18 f19
InitialCC:

Rex, thank you, I'll address it when I push the package into the repository.

Git done (by process-git-requests).

Back to the xauth problem - The directory is being created on runtime, so I'll just change the path to /var/run/sddm to not complicate things for now.
Message about the nonexistent file is benign.
I'm just pushing the sources and will build the package.

Neal could you please report a bug against sddm with the backtrace? We'll discuss the matter further on in there.
Thank you.

sddm-0.1.0-5.fc19 has been submitted as an update for Fedora 19.
https://admin.fedoraproject.org/updates/sddm-0.1.0-5.fc19

sddm-0.1.0-5.fc18 has been submitted as an update for Fedora 18.
https://admin.fedoraproject.org/updates/sddm-0.1.0-5.fc18

sddm-0.1.0-5.fc19 has been pushed to the Fedora 19 testing repository.

sddm-0.1.0-5.fc19 has been pushed to the Fedora 19 stable repository.

sddm-0.1.0-5.fc18 has been pushed to the Fedora 18 stable repository.

Can someone please remove the alias? It's not possible to generally search for sddm bugs. I am not allowed to edit this bug.

(In reply to Raphael Groner (DAASI International) from comment #25)
> Can someone please remove the alias? It's not possible to generally search
> for sddm bugs. I am not allowed to edit this bug.

please report bug to bugzilla administrators.

I would prefer the alias remain