Changeset View
Standalone View
Makefile
# $FreeBSD$ | # $FreeBSD$ | ||||
PORTNAME= hikari | PORTNAME= hikari | ||||
DISTVERSION= 0.1.3 | DISTVERSION= 1.0.0-alpha2 | ||||
0mp: `-alpha2` could probably go to DISTVERSIONSUFFIX | |||||
Done Inline ActionsFrom Table 5.2: https://www.freebsd.org/doc/en_US.ISO8859-1/books/porters-handbook/makefile-naming.html#idp48666616 I think it's fine. lwhsu: From Table 5.2: https://www.freebsd.org/doc/en_US.ISO8859-1/books/porters-handbook/makefile… | |||||
Done Inline ActionsNo, -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. mat: No, -alpha2 must stay in `DISTVERSION`, otherwise, the version of the package would be 1.0.0… | |||||
PORTREVISION= 1 | |||||
CATEGORIES= x11-wm | CATEGORIES= x11-wm | ||||
MASTER_SITES= https://acmelabs.space/~raichoo/ | MASTER_SITES= https://acmelabs.space/~raichoo/ | ||||
MAINTAINER= alex@xanderio.de | MAINTAINER= ports@xanderio.de | ||||
COMMENT= Stacking window manager with tiling capabilities | COMMENT= Stacking Wayland compositor with tiling capabilities | ||||
Done Inline Actionss/wayland/Wayland/. Wayland is the name of a protocol, not a generic noun or adjective. See also: jbeich: s/wayland/Wayland/. Wayland is the name of a protocol, not a generic noun or adjective.
See… | |||||
LICENSE= BSD2CLAUSE | LICENSE= BSD2CLAUSE | ||||
LICENSE_FILE= ${WRKSRC}/LICENSE | LICENSE_FILE= ${WRKSRC}/LICENSE | ||||
LIB_DEPENDS= libxcb-ewmh.so:x11/xcb-util-wm \ | LIB_DEPENDS= libepoll-shim.so:devel/libepoll-shim \ | ||||
libxcb-icccm.so:x11/xcb-util-wm \ | libinput.so:x11/libinput \ | ||||
libxcb-keysyms.so:x11/xcb-util-keysyms | libucl.so:textproc/libucl \ | ||||
Done Inline ActionsMaybe replace with USES=gnome + USE_GNOME=glib20 jbeich: Maybe replace with `USES=gnome` + `USE_GNOME=glib20` | |||||
libwayland-server.so:graphics/wayland \ | |||||
libwlroots.so:x11-toolkits/wlroots \ | |||||
Done Inline ActionsMaybe replace with USES=gnome + USE_GNOME=pango jbeich: Maybe replace with `USES=gnome` + `USE_GNOME=pango` | |||||
libxkbcommon.so:x11/libxkbcommon | |||||
Done Inline ActionsMaybe replace with USES=xorg + USE_XORG=pixman jbeich: Maybe replace with `USES=xorg` + `USE_XORG=pixman` | |||||
BUILD_DEPENDS= evdev-proto>0:devel/evdev-proto \ | |||||
Done Inline ActionsDo 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 jbeich: Do you have a particular reason to put BUILD_DEPENDS **after** LIB_DEPENDS? The usual order is… | |||||
wayland-protocols>=1.14:graphics/wayland-protocols \ | |||||
wlroots>=0.9.0<0.11.0:x11-toolkits/wlroots | |||||
Done Inline ActionsMaybe replace with USES=gnome + USE_GNOME=cairo jbeich: Maybe replace with `USES=gnome` + `USE_GNOME=cairo` | |||||
USES= compiler:c11 localbase:ldflags xorg | USES= pkgconfig gnome | ||||
Done Inline ActionsThis empty line is not needed. Usually we avoid it. 0mp: This empty line is not needed. Usually we avoid it. | |||||
Done Inline ActionsThe extra newline also confuses portlint -C: WARN: Makefile: "BUILD_DEPENDS" has to appear earlier. WARN: Makefile: "USES" has to appear earlier. jbeich: The extra newline also confuses `portlint -C`:
```
WARN: Makefile: "BUILD_DEPENDS" has to… | |||||
jbeichUnsubmitted Done Inline ActionsSort USES alphabeticially. If unsure run portfmt from ports-mgmt/portfmt. jbeich: Sort USES alphabeticially. If unsure run `portfmt` from ports-mgmt/portfmt. | |||||
USE_XORG= x11 xcb | USE_GNOME= cairo glib20 pango pixman | ||||
ports_xanderio.deAuthorUnsubmitted Done Inline Actionsthis is wrong pixman should not be here. uploading a new diff ports_xanderio.de: this is wrong pixman should not be here. uploading a new diff | |||||
Done Inline ActionsNewline 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. jbeich: Newline here isn't necessary but whether to keep it or not is *very subjective*. Some port… | |||||
OPTIONS_DEFINE= DOCS | OPTIONS_DEFINE= SUID | ||||
OPTIONS_DEFAULT= SUID | |||||
Done Inline ActionsMaybe enable XWAYLAND by default to help new users adjust, especially those who migrate from a X11 window manager. jbeich: Maybe enable `XWAYLAND` by default to help new users adjust, especially those who migrate from… | |||||
OPTIONS_SUB= yes | |||||
Done Inline ActionsUSES=gettext-runtime is an implementation detail of devel/glib20, nothing in hikari source references gettext. Instead define USE_GNOME=glib20 as described above. jbeich: `USES=gettext-runtime` is an implementation detail of devel/glib20, nothing in hikari source… | |||||
jbeichUnsubmitted Not Done Inline ActionsOPTIONS_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) " jbeich: `OPTIONS_SUB` is still unused as pkg-plist doesn't reference `%%SUID%%` or `%%NO_SUID%%`.
```
$… | |||||
ports_xanderio.deAuthorUnsubmitted Done Inline ActionsI accidentally remove it from the pkg-plist. Will add it back in the next revision ports_xanderio.de: I accidentally remove it from the `pkg-plist`. Will add it back in the next revision | |||||
NO_WRKSUBDIR= yes | ALL_TARGET=${WITH_DEBUG:Ddebug} | ||||
jbeichUnsubmitted Done Inline ActionsHaving 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. jbeich: Having an unconditional clause among conditional ones is confusing. Move the line before… | |||||
Not Done Inline ActionsDoesn'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 jbeich: Doesn't seem to fit into `make config` dialog window:
```
┌───────────────────────────── hikari… | |||||
SUB_FILES= hikari.desktop | SUID_DESC= setuid bit on "hikari" and "hikari-unlocker" binary (required for DRM and PAM) | ||||
Done Inline ActionsOnly required if you have pkg-plist and don't use _PLIST_SUB option helper. jbeich: Only required if you have pkg-plist and don't use `_PLIST_SUB` option helper. | |||||
SUB_LIST= COMMENT="${COMMENT}" | SUID_PLIST_SUB= MAYBE_SUID="@(,,4755) " | ||||
Done Inline Actionswlroots 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). jbeich: wlroots already depends on `Xwayland` binary, hikari itself doesn't invoke it. If you want to… | |||||
PLIST_FILES= bin/hikari \ | SUID_PLIST_SUB_OFF= MAYBE_SUID="" | ||||
man/man1/hikari.1.gz \ | |||||
share/xsessions/hikari.desktop | |||||
PORTDATA= config.def.h | |||||
PORTDOCS= README.md | |||||
.include <bsd.port.pre.mk> | .include <bsd.port.pre.mk> | ||||
Done Inline ActionsAs @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: As @0mp said, this line can be removed but you have to adjust the last line.
https://www. | |||||
.if defined(WITH_DEBUG) | |||||
ALL_TARGET= debug | |||||
.else | |||||
ALL_TARGET= hikari | |||||
.endif | |||||
pre-everything:: | |||||
@${ECHO_MSG} "You can build hikari with your own config.h using the HIKARI_CONF knob:" | |||||
@${ECHO_MSG} "make HIKARI_CONF=/path/to/hikari/config.h install clean" | |||||
@${ECHO_MSG} "Note: Pre-${PORTVERSION} config.h files may not work." | |||||
post-patch: | |||||
.if defined(HIKARI_CONF) | |||||
@${ECHO_MSG} "Creating config.h from ${HIKARI_CONF}" | |||||
@${LN} -sf ${HIKARI_CONF} ${WRKSRC}/config.h | |||||
.else | |||||
@${LN} -sf ${WRKSRC}/config.def.h ${WRKSRC}/config.h | |||||
.endif | |||||
do-install: | do-install: | ||||
Done Inline ActionsThis can be removed. That's the default. 0mp: This can be removed. That's the default. | |||||
Done Inline ActionsI am not sure if that one is still needed. Maybe we can just use standard bsd.port.mk 0mp: I am not sure if that one is still needed. Maybe we can just use standard `bsd.port.mk` | |||||
Done Inline ActionsI'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 ports_xanderio.de: I'm not quite sure how you that this should be changed.
Changing this to just `bsd.port.mk`… | |||||
Done Inline Actions
Drop bsd.port.pre.mk then replace bsd.port.post.mk with bsd.port.mk. jbeich: > Changing this to just `bsd.port.mk` result in a bunch of make errors
Drop `bsd.port.pre.mk`… | |||||
Done Inline ActionsNot 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. jbeich: Not currently used in the port. See x11-wm/cage for a PLIST_FILES example. Note, `MAYBE_SUID`… | |||||
Done Inline ActionsThe 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. jbeich: The whole conditional can be compressed into `ALL_TARGET=${WITH_DEBUG:Ddebug}`. If you do maybe… | |||||
${INSTALL_PROGRAM} ${WRKSRC}/hikari ${STAGEDIR}${PREFIX}/bin/ | ${INSTALL_PROGRAM} ${WRKSRC}/hikari ${STAGEDIR}${PREFIX}/bin/ | ||||
${INSTALL_MAN} ${WRKSRC}/hikari.1 ${STAGEDIR}${MAN1PREFIX}/man/man1/ | ${INSTALL_PROGRAM} ${WRKSRC}/hikari-unlocker ${STAGEDIR}${PREFIX}/bin/ | ||||
@${MKDIR} ${STAGEDIR}${DATADIR} | @${MKDIR} ${STAGEDIR}${EXAMPLESDIR} | ||||
${INSTALL_DATA} ${WRKSRC}/${PORTDATA} ${STAGEDIR}${DATADIR} | ${INSTALL_MAN} ${WRKSRC}/doc/example_hikari.conf ${STAGEDIR}${EXAMPLESDIR}/hikari.conf | ||||
Done Inline ActionsNeeds 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. jbeich: Needs to be added to PORTEXAMPLES, PLIST_FILES or pkg-plist. Otherwise, the port/package won't… | |||||
Done Inline Actionsi forgot to add the pkg-plist file to the diff ports_xanderio.de: i forgot to add the pkg-plist file to the diff | |||||
@${MKDIR} ${STAGEDIR}${PREFIX}/share/xsessions | |||||
${INSTALL_DATA} ${WRKDIR}/hikari.desktop ${STAGEDIR}${PREFIX}/share/xsessions | |||||
post-install-DOCS-on: | |||||
@${MKDIR} ${STAGEDIR}${DOCSDIR} | |||||
${INSTALL_MAN} ${WRKSRC}/${PORTDOCS} ${STAGEDIR}${DOCSDIR} | |||||
.include <bsd.port.post.mk> | .include <bsd.port.post.mk> |
-alpha2 could probably go to DISTVERSIONSUFFIX