Page MenuHomeFreeBSD

x11-wm/hikari: Update to 1.0.0-alpha3
ClosedPublic

Authored by ports_xanderio.de on Feb 6 2020, 9:14 PM.

Diff Detail

Repository
rP FreeBSD ports repository
Lint
Automatic diff as part of commit; lint not applicable.
Unit
Automatic diff as part of commit; unit tests not applicable.

Event Timeline

There are a very large number of changes, so older changes are hidden. Show Older Changes
Makefile
38 ↗(On Diff #67895)

This can be removed. That's the default.

45 ↗(On Diff #67895)

I am not sure if that one is still needed. Maybe we can just use standard bsd.port.mk

pkg-descr
1 ↗(On Diff #67895)

s/wayland/Wayland/

4 ↗(On Diff #67895)

Missing WWW?

This revision now requires changes to proceed.Feb 6 2020, 9:29 PM
Makefile
4 ↗(On Diff #67895)

No, -alpha2 must stay in DISTVERSION, otherwise, the version of the package would be 1.0.0 and not 1.0.0.a2.

What goes into DISTVERSIONSUFFIX is something that is not part of the version, like a git commit hash.

Some places hardcode /usr/local:

$ rg /usr/local
Makefile
45:WAYLAND_PROTOCOLS ?= /usr/local/share/wayland-protocols

doc/example_hikari.conf
33:  c = "sakura --name cmus -c 150 -r 50 -e /usr/local/bin/cmus"
34:  f = "/usr/local/bin/firefox"
35:  m = "sakura -t mutt --name mutt -c 125 -r 37 -e /usr/local/bin/mutt"

Ignoring doc/example_hikari.conf as unlikely to be used "as is", /usr/local/share/wayland-protocols should be replaced with `pkg-config --variable pkgdatadir wayland-protocols`.

Nothing is installed by this port:

$ make check-plist
====> Checking for pkg-plist issues (check-plist)
===> Parsing plist
===> Checking for items in STAGEDIR missing from pkg-plist
Error: Orphaned: bin/hikari
Error: Orphaned: bin/hikari-unlocker
===> Checking for items in pkg-plist which are not in STAGEDIR
===> Error: Plist issues found.
*** Error code 1

Some files maybe missing:

$ hikari
unable to lock lockfile /var/run/foo/wayland-0.lock, maybe another compositor is running
/bin/sh: hikari-session: not found
^C

What provides hikari-session?

Makefile
9 ↗(On Diff #67895)

s/wayland/Wayland/. Wayland is the name of a protocol, not a generic noun or adjective.

See also:
https://en.wiktionary.org/wiki/Wayland
https://en.wikipedia.org/wiki/Wayland

14 ↗(On Diff #67895)

Maybe replace with USES=gnome + USE_GNOME=cairo

16 ↗(On Diff #67895)

Maybe replace with USES=gnome + USE_GNOME=glib20

18 ↗(On Diff #67895)

Maybe replace with USES=gnome + USE_GNOME=pango

19 ↗(On Diff #67895)

Maybe replace with USES=xorg + USE_XORG=pixman

24 ↗(On Diff #67895)

The extra newline also confuses portlint -C:

WARN: Makefile: "BUILD_DEPENDS" has to appear earlier.
WARN: Makefile: "USES" has to appear earlier.
29 ↗(On Diff #67895)

USES=gettext-runtime is an implementation detail of devel/glib20, nothing in hikari source references gettext. Instead define USE_GNOME=glib20 as described above.

36 ↗(On Diff #67895)

The whole conditional can be compressed into ALL_TARGET=${WITH_DEBUG:Ddebug}. If you do maybe also move the variable outside of options block e.g., put after USES but before PLIST_FILES.

42 ↗(On Diff #67895)

Not currently used in the port. See x11-wm/cage for a PLIST_FILES example. Note, MAYBE_SUID should be defined via _VARS if you use PLIST_FILES and _PLIST_SUB if you use pkg-plist.

51 ↗(On Diff #67895)

Needs to be added to PORTEXAMPLES, PLIST_FILES or pkg-plist. Otherwise, the port/package won't install the file. For some reason, make check-plist doesn't warn about this.

Makefile
33 ↗(On Diff #67895)

Only required if you have pkg-plist and don't use _PLIST_SUB option helper.

Some places hardcode /usr/local:

$ rg /usr/local
Makefile
45:WAYLAND_PROTOCOLS ?= /usr/local/share/wayland-protocols

Ignoring doc/example_hikari.conf as unlikely to be used "as is", `/usr/local/share/wayland-protocols` should be replaced with ``pkg-config --variable pkgdatadir wayland-protocols``.

Ah, thanks! I didn't know about that. I will patch this in alpha3.

Some files maybe missing:

$ hikari
unable to lock lockfile /var/run/foo/wayland-0.lock, maybe another compositor is running
/bin/sh: hikari-session: not found
^C

What provides hikari-session?

hikari-session is not necessary to run hikari it's still sort of undocumented because of the alpha-phase and will likely change into something that goes into hikari.conf.

@raichoo_googlemail.com, hikari source has #ifdef HAVE_XWAYLAND checks but nothing appears to define it. Is it incomplete? If not maybe switch to WLR_HAS_XWAYLAND from #include <wlr/config.h> like Wayfire.

However, Xwayland support hasn't caught up with wlroots 0.9 API.

src/output.c:182:26: error: assigning to 'struct wlr_box *' from incompatible type 'struct wlr_box'; take the
      address with &
    render_data.geometry = xwayland_unmanaged_view->geometry;
                         ^ ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
                           &
src/xwayland_view.c:88:48: error: too many arguments to function call, expected single argument 'view', have
      2 arguments
    hikari_view_commit_pending_operation(view, geometry);
    ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~       ^~~~~~~~
include/hikari/view.h:202:1: note: 'hikari_view_commit_pending_operation' declared here
void
^
src/xwayland_view.c:133:16: error: assigning to 'struct hikari_border' from incompatible type 'int'
  view->border = HIKARI_BORDER_INACTIVE;
               ^ ~~~~~~~~~~~~~~~~~~~~~~

XWayland support is currently incomplete and I was planning to take a look at it tonight. I didn't maintain it for a while since I ditched it entirely on my setup.

ports_xanderio.de added inline comments.
Makefile
51 ↗(On Diff #67895)

i forgot to add the pkg-plist file to the diff

pkg-descr
4 ↗(On Diff #67895)

Its just not in the diff

ports_xanderio.de marked 2 inline comments as done.

include the pkg-plist file in the diff and made change according to the code review

Makefile
45 ↗(On Diff #67895)

I'm not quite sure how you that this should be changed.

Changing this to just bsd.port.mk result in a bunch of make errors

Makefile
25 ↗(On Diff #67941)

this is wrong pixman should not be here. uploading a new diff

Makefile
24 ↗(On Diff #67941)

Sort USES alphabeticially. If unsure run portfmt from ports-mgmt/portfmt.

29 ↗(On Diff #67941)

OPTIONS_SUB is still unused as pkg-plist doesn't reference %%SUID%% or %%NO_SUID%%.

$ make -V PLIST_SUB:M\*SUID\*
SUID="" NO_SUID="@comment " MAYBE_SUID="@(,,4755) "
31 ↗(On Diff #67941)

Having an unconditional clause among conditional ones is confusing. Move the line before OPTIONS* block e.g., just after USE_* without adding a newline.

If unsure run portclippy from ports-mgmt/portfmt.

pkg-descr
1 ↗(On Diff #67941)

Wrap long lines (e.g., via fmt) to fix portlint -C warning:

WARN: x11-wm/hikari/pkg-descr: includes lines that exceed 80 characters.
pkg-plist
3 ↗(On Diff #67941)

hikari doesn't support system-wide config. Besides, @sample with 1 argument and no .sample suffix is nop. See /usr/ports/Keywords/sample.ucl.

$ poudriere testport -j 113i386 x11-wm/hikari
[...]
=>> Checking for extra files and directories
=>> Error: Files or directories left over:
%%PORTEXAMPLES%%@dir %%EXAMPLESDIR%%
@dir share/examples
pkg-plist
2 ↗(On Diff #67945)

%%MAYBE_SUID%% is lost after the last change.

With only 3 files maybe go back to PLIST_FILES then use PORTEXAMPLES for hikari.conf. Less likely to clobber via make makeplist and would also fix the following portlint -C warning:

WARN: Makefile: SUID is listed in OPTIONS_DEFINE, but no PORT_OPTIONS:MSUID appears.
Makefile
20 ↗(On Diff #67945)

Do you have a particular reason to put BUILD_DEPENDS after LIB_DEPENDS? The usual order is BUILD_DEPENDS then LIB_DEPENDS then RUN_DEPENDS. If unsure run portclippy from ports-mgmt/portfmt.

https://www.freebsd.org/doc/en/books/porters-handbook/porting-order-depends.html

45 ↗(On Diff #67895)

Changing this to just bsd.port.mk result in a bunch of make errors

Drop bsd.port.pre.mk then replace bsd.port.post.mk with bsd.port.mk.

pkg-plist
2 ↗(On Diff #67945)

If you're still confused try the following:

USES=		pkgconfig gnome xorg
USE_GNOME=	cairo glib20 pango
USE_XORG=	pixman
ALL_TARGET=	${WITH_DEBUG:Ddebug}
PLIST_FILES=	"${MAYBE_SUID}bin/${PORTNAME}" \
		"${MAYBE_SUID}bin/${PORTNAME}-unlocker"
PORTEXAMPLES=	${PORTNAME}.conf

OPTIONS_DEFINE=		EXAMPLES SUID
OPTIONS_DEFAULT=	SUID

SUID_DESC=	setuid bit on "hikari" and "hikari-unlocker" binary (required for DRM and PAM)
SUID_VARS=	MAYBE_SUID="@(,,4755) "
ports_xanderio.de added inline comments.
Makefile
29 ↗(On Diff #67941)

I accidentally remove it from the pkg-plist. Will add it back in the next revision

pkg-plist
3 ↗(On Diff #67941)

I'm not sure what you mean by this.

I also don't see the see the on my machine.

$ poudriere testport -j 12amd64 x11-wm/hikari
[...]
====> Running Q/A tests (stage-qa)
====> Checking for pkg-plist issues (check-plist)
===> Parsing plist
===> Checking for items in STAGEDIR missing from pkg-plist
===> Checking for items in pkg-plist which are not in STAGEDIR
===> No pkg-plist issues found (check-plist)
=>> Checking for staging violations... done
pkg-plist
3 ↗(On Diff #67941)

I also don't see the see the on my machine.

poudriere only warned about leftover when pkg-plist contained @sample. Sadly, Phabricator doesn't allow editing or removing bogus/obsolete/etc review comments and Done checkbox doesn't appear (for me) on that particular comment.

jbeich added inline comments.
Makefile
27 ↗(On Diff #67947)

Newline here isn't necessary but whether to keep it or not is *very subjective*. Some port maintainers abuse newlines so it's hard to read (due to vertical scrolling), others (e.g., me) compress almost everything into a single block.

38 ↗(On Diff #67947)

As @0mp said, this line can be removed but you have to adjust the last line.

https://www.freebsd.org/doc/en/books/porters-handbook/dads-after-port-mk.html

jbeich added inline comments.
Makefile
35 ↗(On Diff #67950)

Doesn't seem to fit into make config dialog window:

┌───────────────────────────── hikari-1.0.0.a2 ────────────────────────────────┐
│ ┌──────────────────────────────────────────────────────────────────────────┐ │
│ │ [x] EXAMPLES  Build and/or install examples                              │ │
│ │ [x] SUID      setuid bit on "hikari" and "hikari-unlocker" binary (requi)│ │
│ └──────────────────────────────────────────────────────────────────────────┘ │
├──────────────────────────────────────────────────────────────────────────────┤
│                       <  OK  >            <Cancel>                           │
└──────────────────────────────────────────────────────────────────────────────┘

Not sure how to reword other than drop the binary names e.g.,

SUID_DESC=		setuid bit for PAM unlocker and DRM session
ports_xanderio.de retitled this revision from x11-wm/hikari: Update to 1.0.0-alpha2 to x11-wm/hikari: Update to 1.0.0-alpha3.

Update to 1.0.0-alpha3

Makefile
33 ↗(On Diff #67960)

Maybe enable XWAYLAND by default to help new users adjust, especially those who migrate from a X11 window manager.

39 ↗(On Diff #67960)

wlroots already depends on Xwayland binary, hikari itself doesn't invoke it. If you want to abort build early (in depends stage) replace with:

XWAYLAND_BUILD_DEPENDS=	${LOCALBASE}/include/wlr/xwayland.h:x11-toolkits/wlroots

Maybe also rename XWAYLAND to X11 (then drop _DESC line) for consistency with x11-wm/sway and x11-wm/cage. X11 session (e.g., hikari on xorg-server) vs. Xwayland (e.g., emacs on hikari) distinction distinction isn't very useful for either new users (who want both) or hardcore users (who want neither).

Disabling XWayland was my call, since I don't have the best experience with the Xwayland server that is shipped with FreeBSD. It was quite unstable, and crashed on me several times. I haven't checked stability in quite a while though, I just found it to be near unusable last time I checked. I'm currently running a self compiled Xwayland server.

For the record, I think the last commit I've used for Xwayland was a026972776b303fa640f54e5e8f0fc1abf879071 from their HEAD. It turned out to be pretty stable for me. Can't recall it ever crashing on me.

ports_xanderio.de marked 9 inline comments as done.

I'm currently running a self compiled Xwayland server.

Then rP525681 should help. Until bug 244010 lands expect minor annoyance when building outside of poudriere.

I'm currently running a self compiled Xwayland server.

Then rP525681 should help. Until bug 244010 lands expect minor annoyance when building outside of poudriere.

This is one of the reasons why I wanted to have xwayland disabled in the port. It would lead to a binary package that would not be usable for daily use and people would need to dig out patches to make it work. I personally would prefer to have xwayland disabled by default until this is sorted out, until then I can only recommend against using xwayland with hikari on FreeBSD. If people would actually want to use an unstable build of hikari they can still compile it with xwayland turned on. I think it should be this way not the other way around.

It would lead to a binary package that would not be usable for daily use

What? I'm daily using x11-servers/xwayland-devel. Staying on snapshot also makes it easy to adopt https://gitlab.freedesktop.org/xorg/xserver/merge_requests/111 or similar. Wayland is only good when fresh. ;)

wayland-protocols dependency is build-only, so binary package users won't see conflict between wayland-protocols and wayland-protocols-devel ports. If anything binary package users are going to notice how buggy libepoll-shim version currently in ports is.

It would lead to a binary package that would not be usable for daily use

What? I'm daily using x11-servers/xwayland-devel. Staying on snapshot also makes it easy to adopt https://gitlab.freedesktop.org/xorg/xserver/merge_requests/111 or similar. Wayland is only good when fresh. ;)

If that works for you that's good. I'm talking about what gets shipped with FreeBSD packages. Until that is somewhat stable I won't recommend xwayland for daily use with hikari. Once it is brought up to date for everyone without the need for patching stuff it can be enabled by default.
I want people to be able to have a stable version of hikari by simply doing a pkg install hikari with the current state of the xwayland port I cannot guarantee that, that's why I would consider supporting it experimental. I honestly don't want to deal with issues people have because of an unstable version of xwayland.
If people want to mess with that they can build from ports by turning on the flag.

And yes epollshim is another issue. I've already talked to @zeising at the FreeBSD devsummit about it, and he is aware of this issue.

I'm talking about what gets shipped with FreeBSD packages.

xwayland-devel package should be available tomorrow or the day after. wlroots (that hikari uses) already depends on xwayland-devel. Kwin may follow soon, leaving xwayland package with no consumers. For example, pkg install sway will remove xwayland package (if installed) then install xwayland-devel package.

However, until libepoll-shim is updated none of the compositors in ports tree are stable (even if Xwayland is disabled) because libwayland itself uses libepoll-shim.

And yes epollshim is another issue. I've already talked to @zeising at the FreeBSD devsummit about it, and he is aware of this issue.

How much talking one needs to progress? Why do we even need a maintainer that doesn't dogfood, doesn't proactively update, doesn't provide in-depth review and relies on testing by others but goes ballistic when their privileges are infringed by "maintainer timeout" or "portmgr blanket"?

Look at how long it took to update libwayland.

xwayland-devel package should be available tomorrow or the day after. Kwin may follow soon, leaving xwayland package to bitrot on its own.
However, until libepoll-shim is updated none of the compositors in ports tree are stable (even if Xwayland is disabled) because libwayland itself uses libepoll-shim.

I'm currently running hikari with the shipped epollshim for testing reasons and it works okay enough with hikari. But yeah, it's a very iffy feeling and I wish it were a newer version.

How much talking one needs to progress? Why do we even need a maintainer that doesn't dogfood, doesn't proactively update, doesn't provide in-depth review and relies on testing by others but goes ballistic when their privileges are infringed by "maintainer timeout" or "portmgr blanket"?

Look at how long it took to update libwayland.

From what I've been told this is about time. But apparently there seems to be some history here of which I'm unaware and cannot comment on. The reasons why I'm so opposed to the xwayland support right now is because I've got bitten by it pretty badly during the development process of hikari.
Anyway, xwayland-devel is GREAT news and I'm very happy to learn that it exists (as of yesterday, i think). It seems you want xwayland support to be enabled to push things forward and I respect that. I'm just kind of careful because I don't want people to get the wrong impression about wayland and/or hikari stability
and want to enable features progressively as they become more stable and reliable.

Added x11-servers/xwayland-devel as BUILD_DEPENDS so that we use a more stable version of xwayland.

This will get removed when x11-servers/xwayland gets update to a more recent version.

As x11-servers/wayland-devel should not be dependencies.

Disabling the X11 option as upstream declared this feature unstable

As x11-servers/wayland-devel should not be dependencies.

Disabling the X11 option as upstream declared this feature unstable

Thanks, once the xwayland-devel packages land on my system and I had enough time for testing things I think this is a good position to be in for the alpha phase.

As x11-servers/wayland-devel should not be dependencies.

As mentioned in D23544#517781 x11-servers/xwayland-devel is already pulled by x11-toolkits/wlroots since rP525681 because of x11-wm/sway, x11-wm/wayfire, x11-wm/cage.

Disabling the X11 option as upstream declared this feature unstable

OK, especially since upstream uses FreeBSD. ;)

As x11-servers/wayland-devel should not be dependencies.

As mentioned in D23544#517781 x11-servers/xwayland-devel is already pulled by x11-toolkits/wlroots since rP525681 because of x11-wm/sway, x11-wm/wayfire, x11-wm/cage.

Disabling the X11 option as upstream declared this feature unstable

OK, especially since upstream uses FreeBSD. ;)

This is good news, let me test this and once we are pushing alpha4 or alpha5 we can enable it by default. Just let me dogfood on this for a day or two.

This revision is now accepted and ready to land.Feb 12 2020, 10:23 AM

Just let me dogfood on this for a day or two.

hikari haven't entered beta phase, so there's plenty of time. However, xwayland-devel is a moving target, so it can regress if my dogfooding doesn't cover a particular corner case. The rationale is simple:

  • Upstream release engineering is focused on X11 server that runs on hardware (aka -Dxorg=true)
  • Upstream development subsided as RedHat focuses on Wayland, so it's not clear when 1.21 will be released incorporating numerious Xwayland improvements.
  • Xwayland is a fast-evolving target. wlroots 0.10.0 already takes advantage of some features not in any xorg-server release.
  • FreeBSD patches for xorg-server are of questionable quality, so someone needs to refactor and upstream those.
  • xorg-server 1.20.* update succumbed to scope creep, skyrocketing QA complexity to WITH_NEW_XORG level.
  • I already have a few ports that track snapshots e.g., multimedia/aom updates are notorious for build churn. Bisecting regressions is easy as update windows are small.
This revision was automatically updated to reflect the committed changes.

FWIW, epoll-shim was updated in rP525983 just in time before the package cluster started to build hikari.