Re: [PATCH] sysutils/screenfetch

Index: Makefile
===================================================================
RCS file: /cvs/ports/sysutils/screenfetch/Makefile,v
retrieving revision 1.2
diff -u -p -r1.2 Makefile
--- Makefile 11 Nov 2017 08:07:20 -0000 1.2
+++ Makefile 1 Aug 2018 19:24:36 -0000
@@ -1,31 +1,32 @@
# $OpenBSD: Makefile,v 1.2 2017/11/11 08:07:20 bentley Exp $

COMMENT = display system information in the terminal for screenshots
+CATEGORIES = sysutils misc

-V = 3.8.0
-PKGNAME = screenfetch-$V
-REVISION = 0
+PKGNAME = ${DISTNAME:L}
+REVISION = 1

GH_ACCOUNT = KittyKatt
GH_PROJECT = screenFetch
-GH_TAGNAME = v$V
+GH_TAGNAME = v3.8.0

-CATEGORIES = sysutils
+MAINTAINER = Charlene Wendling <julianaito@posteo.jp>

# GPLv3+
PERMIT_PACKAGE_CDROM = Yes

-RUN_DEPENDS = graphics/scrot \
- net/curl \
- shells/bash
+RUN_DEPENDS = graphics/scrot \
+ net/curl \
+ shells/bash

-NO_BUILD = Yes
-NO_TEST = Yes
+NO_BUILD = Yes
+NO_TEST = Yes

-PKG_ARCH = *
+PKG_ARCH = *

+# Upstream has no Makefile.
do-install:
${INSTALL_SCRIPT} ${WRKSRC}/screenfetch-dev ${PREFIX}/bin/screenfetch
- ${INSTALL_SCRIPT} ${WRKSRC}/screenfetch.1 ${PREFIX}/man/man1
+ ${INSTALL_MAN} ${WRKSRC}/screenfetch.1 ${PREFIX}/man/man1

.include <bsd.port.mk>
Index: patches/patch-screenfetch-dev
===================================================================
RCS file: /cvs/ports/sysutils/screenfetch/patches/patch-screenfetch-dev,v
retrieving revision 1.1
diff -u -p -r1.1 patch-screenfetch-dev
--- patches/patch-screenfetch-dev 11 Nov 2017 08:07:20 -0000 1.1
+++ patches/patch-screenfetch-dev 1 Aug 2018 19:24:36 -0000
@@ -1,10 +1,32 @@
$OpenBSD: patch-screenfetch-dev,v 1.1 2017/11/11 08:07:20 bentley Exp $
-Fix RAM output.
-From upstream 77613487a23fd5a12b70945cf8075c2773553643.
+Fix "awk: cannot open /proc/fb"
+From upstream dc72b5932e86ba9c4e36110408690abeb2004070
+Fix "unary operator expected" when no GPU is detected
+From upstream 8346a75068323692243805fa702d02ec7538f3b9
+Fix RAM output
+From upstream 77613487a23fd5a12b70945cf8075c2773553643
Index: screenfetch-dev
--- screenfetch-dev.orig
+++ screenfetch-dev
-@@ -1421,7 +1421,7 @@ detectmem () {
+@@ -1332,13 +1332,15 @@ detectgpu () {
+ # Detect Intel GPU #works in dash
+ # Run it only on Intel Processors if GPU is unknown
+ DetectIntelGPU() {
+- gpu=$(awk '{print $2}' /proc/fb)
++ if [ -r /proc/fb ]; then
++ gpu=$(awk '{print $2}' /proc/fb)
++ fi
+
+ case $gpu in
+ *intel*) gpu=intel ;;
+ esac
+
+- if [ $gpu = intel ]; then
++ if [ "$gpu" = "intel" ]; then
+ #Detect CPU
+ local CPU=$(uname -p | awk '{print $3}')
+ CPU=${CPU#*'-'}; #Detect CPU number
+@@ -1421,7 +1423,7 @@ detectmem () {
usedmem=$(($used_mem / 1024^2 ))
elif [ "$distro" == "OpenBSD" ]; then
totalmem=$(($(sysctl -n hw.physmem) / 1024 / 1024))
Hi,

I'm joining a new diff after Brian's comments.

On Wed, 1 Aug 2018 02:17:37 -0400
Brian Callahan wrote:

> Hi Charlène --
>
> On 07/30/18 14:22, Charlène wrote:
> > Hi,
> >
> > I'm proposing several changes to this port:
> >
> > Makefile:
> >
> > * cleaned extra tabs after '='
> > * use INSTALL_MAN instead of INSTALL_SCRIPT for the manpage
> > * added myself as MAINTAINER, currently it's "orphaned"
> > * incremented REVISION
> > * added braces for variables
>
> Some of these I'm OK with, some I'm not super sure about.
> Yes, I think it's worthwhile to use INSTALL_MAN.
> Glad to see you taking MAINTAINER.
> Glad to see the updated patches.
>
> Braces around variables are not strictly needed in this case; it
> becomes a matter of style in this particular instance. But it might
> be even better to change PKGNAME to ${DISTNAME:L} and change
> GH_TAGNAME to v3.8.0 -- that would completely eliminate the need for
> a V variable in the first place.

I changed to what you recommend. You made me finally understand
that bsd.port.mk is a great source of documentation by itself,
thanks!

> I'm more inclined to leave extra tabs in once a port has been
> imported, unless you're otherwise changing the line anyway, which
> you're not here. Especially in this case, where things are aligned
> anyway, I'm not sure removing the extra tabs improves readability.
> But I guess if you're going to do this, now would be the time.

I was advised to do so for sysutils/neofetch, so i applied the same
thing for this port. I entirely agree about the fact that it doesn't
improve (or worsen) readability here. I've let it as-is for this time,
but i take note for future works.

Charlène.

> ~Brian
>
> > patches, already in upstream's master branch but not in a release:
> >
> > * fixed "awk: cannot open /proc/fb" [1]
> > * fixed "unary operator expected" when no GPU is detected
> > [2]
> >
> > Testing:
> >
> > It has been successfully tested using proot, but you'll need to
> > copy /var/run/dmesg.boot in your chroot to check the output of the
> > program, as it uses this file for OS detection.
> >
> > Comments and OK are welcome!
> >
> > Charlène.
> >
> > [1]
> > https://github.com/KittyKatt/screenFetch/commit/dc72b5932e86ba9c4e36110408690abeb2004070
> > [2]
> > https://github.com/KittyKatt/screenFetch/commit/8346a75068323692243805fa702d02ec7538f3b9
> >
> >
>