Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

gui-libs/greetd: login daemon for wayland + gtk greeter #16223

Closed
wants to merge 5 commits into from

Conversation

epsilon-0
Copy link
Contributor

@epsilon-0 epsilon-0 commented Jun 13, 2020

This is the first wayland only greeter where you don't need to run the display manager on X11 and still get a GUI.

Its a bit rough around the edges, so needs some small manual configurations after installation.

Security features include privilege separation using an unprivileged dedicated daemon runner and only needing access to video group for displaying gui.

Overall I think this is an essential addition for a wayland only desktop environment.


Upstream: https://git.sr.ht/~kennylevinsen/greetd

@gentoo-bot
Copy link

Pull Request assignment

Submitter: @epsilon-0
Areas affected: ebuilds
Packages affected: acct-group/greetd, acct-user/greetd, gui-apps/gtkgreet, gui-libs/greetd

acct-group/greetd: @gentoo/proxy-maint (new package)
acct-user/greetd: @gentoo/proxy-maint (new package)
gui-apps/gtkgreet: @gentoo/proxy-maint (new package)
gui-libs/greetd: @gentoo/proxy-maint (new package)

Linked bugs

No bugs to link found. If your pull request references any of the Gentoo bug reports, please add appropriate GLEP 66 tags to the commit message and request reassignment.

New packages

This Pull Request appears to be introducing new packages only. Due to limited manpower, adding new packages is considered low priority. This does not mean that your Pull Request will not receive any attention, however, it might take quite some time for it to be reviewed. In the meantime, your new ebuild might find a home in the GURU project repository: the ebuild repository maintained collaboratively by Gentoo users. GURU offers your ebuild a place to be reviewed and improved by other Gentoo users, while making it easy for Gentoo users to install it and enjoy the software it adds.


In order to force reassignment and/or bug reference scan, please append [please reassign] to the pull request title.

Docs: Code of ConductCopyright policy (expl.) ● DevmanualGitHub PRsProxy-maint guide

@gentoo-bot gentoo-bot added new package The PR is adding a new package. self-maintained The PR changes only packages that are maintained by the submitter (i.e. no need to ask anybody else) assigned PR successfully assigned to the package maintainer(s). labels Jun 13, 2020
@epsilon-0
Copy link
Contributor Author

@flappyports this an interesting wayland only display and login manager.
Its integration with compositors is bit rough but seems to be working.
No rush to get it in and thanks a lot!

@gentoo-repo-qa-bot
Copy link
Collaborator

Pull request CI report

Report generated at: 2020-06-13 15:52 UTC
Newest commit scanned: b2f053e
Status: ✅ good

There are existing issues already. Please look into the report to make sure none of them affect the packages in question:
https://qa-reports.gentoo.org/output/gentoo-ci/6d9e076314/output.html

@gentoo-repo-qa-bot
Copy link
Collaborator

Pull request CI report

Report generated at: 2020-06-13 21:38 UTC
Newest commit scanned: c956508
Status: ✅ good

There are existing issues already. Please look into the report to make sure none of them affect the packages in question:
https://qa-reports.gentoo.org/output/gentoo-ci/f9048ad269/output.html

@gentoo-repo-qa-bot
Copy link
Collaborator

Pull request CI report

Report generated at: 2020-06-30 01:58 UTC
Newest commit scanned: 3fccf3c
Status: ✅ good

There are existing issues already. Please look into the report to make sure none of them affect the packages in question:
https://qa-reports.gentoo.org/output/gentoo-ci/c6e4da10bd/output.html

@FuzzyGophers
Copy link
Contributor

@epsilon-0 as this is a rust ebuild... I would recommend pinging one of those folks.

@epsilon-0
Copy link
Contributor Author

@epsilon-0 as this is a rust ebuild... I would recommend pinging one of those folks.

Good idea!

SRC_URI="https://git.sr.ht/~kennylevinsen/greetd/archive/${PV}.tar.gz -> ${P}.tar.gz
$(cargo_crate_uris ${CRATES})
"
RESTRICT="mirror"
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

RESTRICT="mirror" is mostly for overlays.
remove it, we allow and encourage crate mirroring.
like 80% of those crates already on the mirrors.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

done

LICENSE="Apache-2.0 BSD Boost-1.0 GPL-3 MIT Unlicense"
SLOT="0"
KEYWORDS="~amd64 ~x86"
IUSE="man"
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

you have man turned off here by default, but have turned on in other package.
be consistent, turn it on by default.
scdoc is cheap dependency with no weird revdeps (not sphinx lol), so it's going to be ok.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

done

DEPEND="
acct-user/greetd
sys-auth/pambase
"
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

you'll likely need pam dependency here.
pam-sys crate is a binding crate and links to system .so

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

use this rdepcheck.sh

for example rdepcheck.sh greetd (once it's installed)
it will tell if it links to pam.

#!/bin/bash

qlist -e ${1} \
        | xargs --no-run-if-empty scanelf -L -n -q -F '%n #F' \
        | tr , '\n' \
        | xargs --no-run-if-empty readlink -f \
        | uniq \
        | xargs --no-run-if-empty qfile -CS \
        | sort -u

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

done! added sys-libs/pam

dobin target/release/{agreety,fakegreet,greetd}

insinto /etc/greetd
insopts -m0644 -o greetd -g greetd
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

export INSOPTIONS=-m0644 is the default.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

also I read upstream docs and they recommend configuration owned by daemon.
this is bad practice.
it has to be readable by user.
configuration should be owned by root, period, unless something weird going on.
I think you should just use defaults, it will be readable by that user. but owned by root.

read
http://michael.orlitzky.com/articles/configuration_should_be_owned_and_writable_only_by_root.xhtml

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Removed the users/group in the config file install.
Its not needed afaik.


if use man; then
doman agreety.1 greetd.1 greetd.5 greetd-ipc.7
fi
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

use man && doman agreety.1 greetd.1 greetd.5 greetd-ipc.7 ?
up to you, just a nit.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I like verbosity so I'll keep this 😸

}

pkg_postint() {
optfeature "eye-candy gtk based greeter" gui-apps/gtkgreet
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

eutils.eclass and optfeature should be avoided if possible. just print post-inst message.
optfeature is generally needed if you have a lot of optional deps.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Also added tuigreet so I'll keep this.

Copy link
Member

@gyakovlev gyakovlev left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

if you add a user you need to open a pull request to

https://github.com/gentoo/api-gentoo-org/blob/master/files/uid-gid.txt

and have it merged before or at the same time this package is merged.
look for closed PRs as an example.

$(cargo_crate_uris ${CRATES})"
S="${WORKDIR}/${PN}-${COMMIT}"

RESTRICT="mirror"
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

ditto, remove restrict.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

done

DESCRIPTION="TUI greeter for greetd login manager"
HOMEPAGE="https://github.com/apognu/tuigreet"

COMMIT="034f44155e35a9c0d131f50c57a7436bff97b42a"
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

how do you know that this commit is 0.1.0 ?
if author tags 0.0.1, it will be older that this ebuild.
version it it 0.0.0_pre2020xxxx
rust follows semantic versioning, this will guarantee that anything author tags will be newer.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Author had made a release 0.1.0 but removed it afterwards (for some trivial bug issues).
I've changed it to 0_pre20200702 (there was a small commit today.

SLOT="0"
KEYWORDS="~amd64"

DEPEND="gui-libs/greetd"
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

is it really depend?
I think it uses IPC to communicate, hence RDEPEND only.
unless it's used for tests, in that case move behind test? condition.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Moved to RDEPEND

virtual/pkgconfig
man? ( app-text/scdoc )
"

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

gtk based, but does not depend on gtk?
sounds fishy.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Oooof, had totally forgotten to add them.
Added now 😸

Signed-off-by: Aisha Tammy <gentoo@aisha.cc>
Signed-off-by: Aisha Tammy <gentoo@aisha.cc>
Package-Manager: Portage-2.3.99, Repoman-2.3.22
Signed-off-by: Aisha Tammy <gentoo@aisha.cc>
@epsilon-0
Copy link
Contributor Author

if you add a user you need to open a pull request to

https://github.com/gentoo/api-gentoo-org/blob/master/files/uid-gid.txt

and have it merged before or at the same time this package is merged.
look for closed PRs as an example.

Create PR - gentoo/api-gentoo-org#287

@epsilon-0 epsilon-0 requested a review from gyakovlev July 2, 2020 12:28
@gentoo-repo-qa-bot
Copy link
Collaborator

Pull request CI report

Report generated at: 2020-07-02 12:38 UTC
Newest commit scanned: 800e4d8
Status: ✅ good

There are existing issues already. Please look into the report to make sure none of them affect the packages in question:
https://qa-reports.gentoo.org/output/gentoo-ci/74895cabc3/output.html

IUSE="+layershell +man"

DEPEND="
dev-libs/json-c
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

missing subslot op :=

you can see if you need subslot deps by running eshowkw json-c
you see, it has 3 subslots currently. if a new one introduced or user installs 0/5 instead of 0/4 you'd likely need a rebuild.


Keywords for dev-libs/json-c:
           |                             |   u     |  
           | a   a     p s     a   r     |   n     |  
           | m   r h   p p   s l i i m m | e u s   | r
           | d a m p p c a x 3 p a s 6 i | a s l   | e
           | 6 r 6 p p 6 r 8 9 h 6 c 8 p | p e o   | p
           | 4 m 4 a c 4 c 6 0 a 4 v k s | i d t   | o
-----------+-----------------------------+---------+-------
   0.12    | + + + + + + + + + ~ ~ o o ~ | 5 o 0/2 | gentoo
-----------+-----------------------------+---------+-------
 0.13.1-r1 | + + + + + + + + + ~ ~ ~ o ~ | 6 o 0/4 | gentoo
-----------+-----------------------------+---------+-------
[I]0.14-r3 | + + + + + + + + + ~ ~ o o ~ | 7 o 0/5 | gentoo
   9999    | o o o o o o o o o o o o o o | 7 o     | gentoo

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Like I had a cursory glance in the slots, forgot to check subslots 😿
Would be nice if pkgcheck scan could tell this !

In any case, I've fixed it.

DEPEND="
dev-libs/json-c
gui-libs/gtk-layer-shell
x11-libs/gtk+:3=
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

don't need = here

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

done


DEPEND="
dev-libs/json-c
gui-libs/gtk-layer-shell
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

btw, gtk-layer-shell missing gtk dep as well lol.
it should depend on x11-libs/gtk+:3[wayland]

not your package, just sayin.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

lol

Package-Manager: Portage-2.3.99, Repoman-2.3.22
Signed-off-by: Aisha Tammy <gentoo@aisha.cc>
Package-Manager: Portage-2.3.101, Repoman-2.3.22
Signed-off-by: Aisha Tammy <gentoo@aisha.cc>
@gentoo-repo-qa-bot
Copy link
Collaborator

Pull request CI report

Report generated at: 2020-07-02 20:43 UTC
Newest commit scanned: 02e3a75
Status: ✅ good

There are existing issues already. Please look into the report to make sure none of them affect the packages in question:
https://qa-reports.gentoo.org/output/gentoo-ci/423e1f3bf5/output.html

@gentoo-bot gentoo-bot closed this in 4b392d7 Jul 2, 2020
gentoo-bot pushed a commit that referenced this pull request Jul 2, 2020
PR: #16223
Package-Manager: Portage-2.3.103, Repoman-2.3.23
Signed-off-by: Georgy Yakovlev <gyakovlev@gentoo.org>
gentoo-bot pushed a commit that referenced this pull request Jul 2, 2020
PR: #16223
Package-Manager: Portage-2.3.103, Repoman-2.3.23
Signed-off-by: Georgy Yakovlev <gyakovlev@gentoo.org>
gentoo-bot pushed a commit that referenced this pull request Jul 2, 2020
PR: #16223
Package-Manager: Portage-2.3.103, Repoman-2.3.23
Signed-off-by: Georgy Yakovlev <gyakovlev@gentoo.org>
@gyakovlev
Copy link
Member

I've committed couple keywords and fixes I've discovered while build testing.
tuigreet was missing a crate, probably you did not re-run cargo-ebuild after referencing new commit sha.
and I've added QA_FLAGS_IGNORED, because rust does not support CFLAGS ofc, portage was screaming that flags are ignored.

@epsilon-0
Copy link
Contributor Author

epsilon-0 commented Jul 2, 2020

I've committed couple keywords and fixes I've discovered while build testing.
tuigreet was missing a crate, probably you did not re-run cargo-ebuild after referencing new commit sha.

OMG, I totally forgot, sorry about this one.

and I've added QA_FLAGS_IGNORED, because rust does not support CFLAGS ofc, portage was screaming that flags are ignored.

I didn't even know this was supposed to be done.
Rust is a very.... interesting thing to learn, for lack of a nicer term.

Thanks a lot btw.

NeddySeagoon pushed a commit to NeddySeagoon/gentoo-arm64 that referenced this pull request Jul 4, 2020
Package-Manager: Portage-2.3.101, Repoman-2.3.22
Signed-off-by: Aisha Tammy <gentoo@aisha.cc>
Closes: gentoo#16223
Signed-off-by: Georgy Yakovlev <gyakovlev@gentoo.org>
NeddySeagoon pushed a commit to NeddySeagoon/gentoo-arm64 that referenced this pull request Jul 4, 2020
PR: gentoo#16223
Package-Manager: Portage-2.3.103, Repoman-2.3.23
Signed-off-by: Georgy Yakovlev <gyakovlev@gentoo.org>
NeddySeagoon pushed a commit to NeddySeagoon/gentoo-arm64 that referenced this pull request Jul 4, 2020
PR: gentoo#16223
Package-Manager: Portage-2.3.103, Repoman-2.3.23
Signed-off-by: Georgy Yakovlev <gyakovlev@gentoo.org>
NeddySeagoon pushed a commit to NeddySeagoon/gentoo-arm64 that referenced this pull request Jul 4, 2020
PR: gentoo#16223
Package-Manager: Portage-2.3.103, Repoman-2.3.23
Signed-off-by: Georgy Yakovlev <gyakovlev@gentoo.org>
@epsilon-0 epsilon-0 deleted the greetd branch August 26, 2020 03:51
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
assigned PR successfully assigned to the package maintainer(s). new package The PR is adding a new package. self-maintained The PR changes only packages that are maintained by the submitter (i.e. no need to ask anybody else)
Projects
None yet
5 participants