On 14.11.22 10:34, Omar Polo wrote:
On 2022/11/13 16:20:18 +0100, Alexander Klimov <grandmas...@al2klimov.de> wrote:
On 13.11.22 15:06, Omar Polo wrote:
[...]
one more nitpick regarding how you updated the call to pledge and
unveil;
+ if (unveil(args.dev_path, "r"))
+ err(1, "unveil");
I'd explicitly check that unveil returns -1 to error, not "anything
but 0". same for pledge.
[...]
Everything in patches/patch-utils_* is borrowed from upstream master.
patches/patch-utils_* will be obsolete with, say, v9.0.
ah! i didn't check if it was a backport, sorry!
[...]
Just one more comment regarding the Makefile patch: I'd add a MANDIR
variable set to ${PREFIX}/share/man/ and overwrite it in the port'
makefile
MAKE_ENV += MANDIR=${PREFIX}/man/
I think I understand and would be able to do this, but the author
doesn't like TOO OSspecific stuff in upstream (such as unveil(2),
pledge(2)) and this neither improves anything significantly nor adds
support for any OS. Let's think twice about this and maybe look for
something else to also adjust not to send more patches than necessary.
What I meant (and it's by no way a requirement; it's just a suggestion
to make your life easier maintaining the port) was something like
this: (diff against your repo)
diff --git a/Makefile b/Makefile
index c206c96..8e0acac 100644
--- a/Makefile
+++ b/Makefile
@@ -16,4 +16,6 @@ NO_TEST= Yes
BUILD_DEPENDS= devel/argp-standalone
+MAKE_FLAGS+= MANDIR=${PREFIX}/man
+
.include <bsd.port.mk>
diff --git a/patches/patch-Makefile b/patches/patch-Makefile
index 22cbba1..03316c3 100644
--- a/patches/patch-Makefile
+++ b/patches/patch-Makefile
@@ -1,16 +1,24 @@
Index: Makefile
--- Makefile.orig
+++ Makefile
-@@ -23,9 +23,9 @@ extra: $(EXTRA_TARGETS)
+@@ -5,6 +5,7 @@ TARGETS = f3write f3read
+ EXTRA_TARGETS = f3probe f3brew f3fix
+
+ PREFIX = /usr/local
++MANDIR = $(PREFIX)/share/man
+ INSTALL = install
+ LN = ln
+
+@@ -23,9 +24,9 @@ extra: $(EXTRA_TARGETS)
install: all
$(INSTALL) -d $(DESTDIR)$(PREFIX)/bin
$(INSTALL) -m755 $(TARGETS) $(DESTDIR)$(PREFIX)/bin
- $(INSTALL) -d $(DESTDIR)$(PREFIX)/share/man/man1
- $(INSTALL) -m644 f3read.1 $(DESTDIR)$(PREFIX)/share/man/man1
- $(LN) -sf f3read.1 $(DESTDIR)$(PREFIX)/share/man/man1/f3write.1
-+ $(INSTALL) -d $(DESTDIR)$(PREFIX)/man/man1
-+ $(INSTALL) -m644 f3read.1 $(DESTDIR)$(PREFIX)/man/man1
-+ $(LN) -sf f3read.1 $(DESTDIR)$(PREFIX)/man/man1/f3write.1
++ $(INSTALL) -d $(DESTDIR)$(MANDIR)/man1
++ $(INSTALL) -m644 f3read.1 $(DESTDIR)$(MANDIR)/man1
++ $(LN) -sf f3read.1 $(DESTDIR)$(MANDIR)/man1/f3write.1
install-extra: extra
$(INSTALL) -d $(DESTDIR)$(PREFIX)/bin
This way upstream' makefile gains a new variable MANDIR but otherwise
still behaves like before, and we can overwrite a variable instead of
patching. A patch like this I guess could be upstreamed easily.
Again, not a requirement, just a suggestion to make it easier to hack
on this port.
anyway, with the unveil/pledge == -1 bit fixed the port is ok to me
I've pushed it. (But I don't understand why it's an actual problem. It's
a convention like execpromises=NULL and no unveil(NULL,NULL), right?)
assuming it works; i haven't run-tested it.
Thanks,
