Bug 233010 - x11/kitty: Cross-platform, fast, feature full, GPU based terminal emulator
Summary: x11/kitty: Cross-platform, fast, feature full, GPU based terminal emulator
Status: Open
Alias: None
Product: Ports & Packages
Classification: Unclassified
Component: Individual Port(s) (show other bugs)
Version: Latest
Hardware: Any Any
: --- Affects Some People
Assignee: freebsd-ports-bugs mailing list
URL:
Keywords: feature, needs-qa
Depends on: 230140
Blocks:
  Show dependency treegraph
 
Reported: 2018-11-06 02:42 UTC by Ben Widawsky
Modified: 2019-03-15 13:18 UTC (History)
6 users (show)

See Also:


Attachments
x11-kitty.diff (19.36 KB, patch)
2018-11-06 02:42 UTC, Ben Widawsky
no flags Details | Diff
fixed nits and added terminfo explanation (17.19 KB, patch)
2018-11-08 20:15 UTC, Ben Widawsky
no flags Details | Diff
v0.13.3 + dependency on termcap (15.04 KB, patch)
2019-02-02 06:56 UTC, Ben Widawsky
no flags Details | Diff
Small fixes + wayland support (17.81 KB, patch)
2019-02-19 02:17 UTC, Ben Widawsky
no flags Details | Diff
More minor fixes (18.08 KB, patch)
2019-03-07 22:39 UTC, Ben Widawsky
no flags Details | Diff
Latest round of improvements from jbeich (18.18 KB, patch)
2019-03-08 17:38 UTC, Ben Widawsky
no flags Details | Diff
INSTALLS_ICONS re-removed (18.19 KB, patch)
2019-03-09 17:19 UTC, Ben Widawsky
no flags Details | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Ben Widawsky freebsd_committer 2018-11-06 02:42:55 UTC
Created attachment 198996 [details]
x11-kitty.diff

In WantedPorts:  https://wiki.freebsd.org/WantedPorts.

Note that because Sphinx isn't yet at >= 1.7, the docs option is untestable and I didn't spend much time on it.
Comment 1 Nathan 2018-11-06 02:50:29 UTC
Comment on attachment 198996 [details]
x11-kitty.diff

BINARY_ALIAS=	python3=${PYTHON_CMD} 
How is that used?

DOCS_DESC=	build docs
Remove, DOCS already has DESC in framework

BELL=
Should be BELL_DESC=

+OPTIONS_DEFINE=	ICAT DOCS BELL NLS
+OPTIONS_DEFAULT= NLS
Goes at the start of OPTIONS, not at the end

DOCS_BUILD_DEPENDS=	${PYTHON_PKGNAMEPREFIX}sphinx>=1.7:textproc/py-sphinx
should be:
OCS_BUILD_DEPENDS=	${PYTHON_PKGNAMEPREFIX}sphinx>=1.7:textproc/py-sphinx@${PY_FLAVOR}
Comment 2 Ben Widawsky freebsd_committer 2018-11-06 06:14:28 UTC
(In reply to Nathan from comment #1)
The project's internal installation logic tries to call python3, which is worked around via ALIAS_BINARY.

All the other changes have been made locally. I will wait for some more feedback before uploading the patch.
Comment 3 Dave Cottlehuber freebsd_committer 2018-11-06 14:27:21 UTC
neat!

I pulled this in as a patch to https://github.com/skunkwerks/ports/tree/x11/kitty to run it in a clean build in poudriere, similar to what the FreeBSD packaging system would run. This includes DOCS built by default: 
 
===>   kitty-0.12.3 depends on package: py36-sphinx>=1.7 - not found
*** Error code 1

DOCS is a pretty common base option to have defined, so I suggest poking the maintainer for py-sphinx and seeing if that's an option.

Either py-sphinx gets updated to 1.8.1 (current release) so this works, or the patch should remove the docs completely pending py-sphinx becoming available.
Comment 4 Ben Widawsky freebsd_committer 2018-11-06 18:38:57 UTC
(In reply to Dave Cottlehuber from comment #3)
Yes. I pinged lwhsu@ about this who originally wrote the package before I uploaded this.

BTW, off topic, I did test this would poudriere and it seemed like default options would always add docs no matter what. Is there a way to have poudriere jail testing test all the different options?
Comment 5 Ben Widawsky freebsd_committer 2018-11-08 17:11:40 UTC
Note, terminfo handling is currently broken. There are two issues:
1. tic run at install time isn't actually adding the entry.
2. Even when the entry is manually added, curses can't seem to figure it out.
Comment 6 Ben Widawsky freebsd_committer 2018-11-08 19:01:58 UTC
For base ncurses which uses old TERMCAP (thanks Scott):

TERMCAP=xterm-kitty|KovIdTTY:am:hs:km:mi:ms:xn:co#80:it#8:li#24:AL=\E[%dL:DC=\E[%dP:DL=\E[%dM:DO=\E[%dB:IC=\E[%d@:K1=:K3=:K4=:K5=:LE=\E[%dD:RI=\E[%dC:SF=\E[%dS:SR=\E[%dT:UP=\E[%dA:ae=\E(B:al=\E[L:as=\E(0:bl=^G:bt=\E[Z:cd=\E[J:ce=\E[K:cl=\E[H\E[2J:cm=\E[%i%d;%dH:cr=\r:cs=\E[%i%d;%dr:ct=\E[3g:dc=\E[P:dl=\E[M:do=\n:ds=\E]2;\007:ec=\E[%dX:ei=\E[4l:fs=^G:ho=\E[H:im=\E[4h:k1=\EOP:k2=\EOQ:k3=\EOR:k4=\EOS:k5=\E[15~:k6=\E[17~:k7=\E[18~:k8=\E[19~:k9=\E[20~:kD=\E[3~:kI=\E[2~:kN=\E[6~:kP=\E[5~:kb=\177:kd=\EOB:ke=\E[?1l:kh=\EOH:kl=\EOD:kr=\EOC:ks=\E[?1h:ku=\EOA:le=^H:md=\E[1m:me=\E[0m:mh=\E[2m:mr=\E[7m:nd=\E[C:rc=\E8:sc=\E7:se=\E[27m:sf=\n:so=\E[7m:sr=\EM:st=\EH:ta=^I:te=\E[?1049l:ti=\E[?1049h:ts=\E]2;:ue=\E[24m:up=\E[A:us=\E[4m:vb=\E[?5h\E[?5l:ve=\E[?12l\E[?25h:vi=\E[?25l:vs=\E[?12;25h:
Comment 7 Ben Widawsky freebsd_committer 2018-11-08 20:15:56 UTC
Created attachment 199085 [details]
fixed nits and added terminfo explanation
Comment 8 Ben Widawsky freebsd_committer 2018-11-08 21:02:56 UTC
The TERMCAP method doesn't seem to properly set colors...
Comment 9 Ben Widawsky freebsd_committer 2018-11-08 22:29:48 UTC
You need to add "tc=xterm-new:" to the termcap entry.
Comment 10 Li-Wen Hsu freebsd_committer 2018-12-13 05:37:46 UTC
BTW, if we cannot get textproc/py-sphinx updated soon, disabling building doc might be another option...
Comment 11 Kubilay Kocak freebsd_committer freebsd_triage 2018-12-14 05:30:55 UTC
Reporter is Committer, assign accoringly
Comment 12 Ben Widawsky freebsd_committer 2019-02-02 00:50:57 UTC
(In reply to Kubilay Kocak from comment #11)
I am not a ports committer.
Comment 13 Ben Widawsky freebsd_committer 2019-02-02 06:56:04 UTC
Created attachment 201630 [details]
v0.13.3 + dependency on termcap
Comment 15 Ben Widawsky freebsd_committer 2019-02-02 18:28:46 UTC
(In reply to Greg V from comment #14)
Do you just want to take this over? I only did this because I wanted kitty a few months back and no port existed. I have no particular desire to maintain a port.
Comment 16 Ben Widawsky freebsd_committer 2019-02-19 02:17:11 UTC
Created attachment 202146 [details]
Small fixes + wayland support
Comment 17 Ben Widawsky freebsd_committer 2019-03-07 22:39:32 UTC
Created attachment 202705 [details]
More minor fixes
Comment 18 Jan Beich freebsd_committer 2019-03-08 01:04:43 UTC
Comment on attachment 202705 [details]
More minor fixes

> USES=	gl gmake pkgconfig:build python:3.5+ shebangfix terminfo

pkgconfig defaults to build-only dependency, so you can drop :build argument.

> BINARY_ALIAS=	python3=${PYTHON_CMD}
[...]
> post-patch-DOCS-on:
> 	${REINPLACE_CMD} -e 's|python3|${PYTHON_CMD}|g' ${WRKSRC}/docs/Makefile
> 	${REINPLACE_CMD} -e 's|sphinx-build|&-${PYTHON_VER}|' ${WRKSRC}/docs/Makefile

Why both approaches? And do you still need either given DOCS is commented out?

> do-build:
> 	cd ${WRKSRC} && CC=clang ${PYTHON_CMD} setup.py --prefix ${STAGEDIR}${PREFIX} linux-package

- Replace CC=clang with ${SETENV} ${MAKE_ENV} to avoid breaking GCC architectures (powerpc*, sparc64, mips*, riscv64*)
- Add USES=compiler:c11 (for -std=c11) to avoid ancient GCC 4.2 in base 

x11/kitty/files/patch-glfw_wl__init.c:
> --- glfw/wl_init.c.orig	2019-02-17 23:49:36 UTC
> +++ glfw/wl_init.c
> @@ -31,7 +31,7 @@
>  #include <assert.h>
>  #include <errno.h>
>  #include <limits.h>
> -#include <linux/input.h>
> +#include <dev/evdev/input.h>
>  #include <stdio.h>
>  #include <stdlib.h>
>  #include <string.h>

Drop in favor of BUILD_DEPENDS=evdev-proto>0:devel/evdev-proto. According to wulf@ (maintainer of <dev/evdev/*>):
"The rule is simple: always use <linux/*> except you are writing something FreeBSD-only like kernel drivers"
Comment 19 Ben Widawsky freebsd_committer 2019-03-08 17:38:23 UTC
Created attachment 202721 [details]
Latest round of improvements from jbeich
Comment 20 Ben Widawsky freebsd_committer 2019-03-09 17:19:26 UTC
Created attachment 202749 [details]
INSTALLS_ICONS re-removed
Comment 21 Jan Beich freebsd_committer 2019-03-15 13:18:53 UTC
Comment on attachment 202749 [details]
INSTALLS_ICONS re-removed

Fails to build with ICAT=on:
[00:00:02] Error: x11/kitty depends on nonexistent origin 'graphics/ImageMagick' (moved to graphics/ImageMagick6)

> COMMENT=	Cross-platform, fast, feature full, GPU based terminal emulator

feature full -> featureful? GPU based -> GPU-based? Why , (comma) before "GPU based" (cumulative adjective)?
Maybe also drop "Cross-platform" as it's ambiguous and doesn't include Windows or Android.
See https://repology.org/project/kitty/information for alternative summaries.

> BUILD_DEPENDS=	wayland-protocols>=0:graphics/wayland-protocols \
> 	evdev-proto>0:devel/evdev-proto

Consistently use either >0 or >=0 for both.

> LIB_DEPENDS=	libdbus-1.so:devel/dbus \
> 	libfontconfig.so:x11-fonts/fontconfig \
> 	libfreetype.so:print/freetype2 \
> 	libglfw.so:graphics/glfw \

Why is this necessary if the port builds bundled version?

> 	libharfbuzz.so:print/harfbuzz \
> 	libpng.so:graphics/png \
> 	libxkbcommon.so:x11/libxkbcommon \
> 	libncursesw.so:devel/ncurses \

Switch to USES=ncurses:port as libncursesw.so may match /usr/lib/libncursesw.so

> 	libdbus-1.so:devel/dbus \

Is there any reason to specify devel/dbus more than once?

> 	libwayland-egl.so:graphics/wayland \
> 	libwayland-client.so:graphics/wayland \

libwayland-egl.so is part of graphics/wayland after bug 227423, so drop libwayland-client.so line to avoid redundancy. There's no need to list every single library used from a dependency port.

> 	libepoll-shim.so:devel/libepoll-shim \
> 	libpng.so:graphics/png

Is there any reason to specify graphics/png more than once?
Can you sort *_DEPENDS based on port category? For one, in Emacs select region via C-SPC then C-u C-| sort -k2 -t:
Also try to keep indentation consistent as there's plenty of room before exceeding 80 columns.

> USE_XORG=	x11 xft xrandr xinerama xcb xcursor xi

Why xft is necessary? According to "poudriere testport" libXft is not NEEDED.
xi (libXi) can be dropped as like xxf86vm (libXxf86vm.so) it's only dlopen'd.

> BINARY_ALIAS=	python3=${PYTHON_CMD}

Doesn't appear to be necessary as do-build invokes ${PYTHON_CMD} explicitly.
post-patch-DOCS-on is enough otherwise.

> OPTIONS_DEFAULT=	NLS NOBELL #DOCS

NLS and DOCS are always enabled by default, see Mk/bsd.options.mk.

> OPTIONS_DEFINE=	ICAT NOBELL NLS #DOCS
[...]
> # XXX: docs cannot build because it relies on sphinx >= 1.7 which doesn't yet
> # exist in ports. When docs is capable of building, remove EXTRA_PATCHES and
> # uncomment all the DOCS_* lines.
> #DOCS_BUILD_DEPENDS=	${PYTHON_PKGNAMEPREFIX}sphinx>=1.7:textproc/py-sphinx@${PY_FLAVOR}
> #DOCS_EXTRA_PATCHES=	${PATCHDIR}/docs-on-patch-setup.py
> #DOCS_EXTRA_PATCHES_OFF=	${PATCHDIR}/docs-off-patch-setup.py

Don't leave unfinished code in. Better apply bug 230140, enable and test the code properly, fix issues then add OPTIONS_EXCLUDE=DOCS until the main tree catches up.

> ICAT_DESC=	tool to display images in the terminal
> NOBELL_DESC=	disable audio bell by default (can be changed in kitty.conf)

Capitalize the first letter in sentence.

> ICAT_LIB_DEPENDS=	libMagickCore-6.so:graphics/ImageMagick

Does it link against the library? If not convert to RUN_DEPENDS. That'd also make it easier to switch between ImageMagick6 and ImageMagick7.

> NLS_USES=	gettext

Can one actually disable NLS? If not make it unconditional.
If only the library is required switch to USES=gettext-runtime.

>	${FIND} ${STAGEDIR}${PREFIX} -name __pycache__ -type d -exec ${RM} -r {} +

Maybe define USE_PYTHON=py3kplist and add .pyo for each .py file in pkg-plist instead.

$ sed -i '' '/\.py$/ { p; s//&o/; }' pkg-plist