Note: This is a public test instance of Red Hat Bugzilla. The data contained within is a snapshot of the live data so any changes you make will not be reflected in the production Bugzilla. Email is disabled so feel free to test any aspect of the site that you want. File any problems you find or give feedback at bugzilla.redhat.com.
Bug 984700 (sddm)
Summary: | Review Request: sddm - QML based X11 desktop manager | ||
---|---|---|---|
Product: | [Fedora] Fedora | Reporter: | Martin Bříza <mbriza> |
Component: | Package Review | Assignee: | Christopher Meng <i> |
Status: | CLOSED ERRATA | QA Contact: | Fedora Extras Quality Assurance <extras-qa> |
Severity: | medium | Docs Contact: | |
Priority: | medium | ||
Version: | rawhide | CC: | i, jones.peter.busi, kevin, mbriza, ndbecker2, notting, package-review, raphael.groner, rdieter |
Target Milestone: | --- | Flags: | i:
fedora-review+
gwync: fedora-cvs+ |
Target Release: | --- | ||
Hardware: | All | ||
OS: | Linux | ||
Whiteboard: | |||
Fixed In Version: | sddm-0.1.0-5.fc18 | Doc Type: | Bug Fix |
Doc Text: | Story Points: | --- | |
Clone Of: | Environment: | ||
Last Closed: | 2013-08-06 00:20:47 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: | |||
Bug Depends On: | |||
Bug Blocks: | 928937 |
Description
Martin Bříza
2013-07-15 17:49:01 UTC
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. 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 |