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
assuming it works; i haven't run-tested it.


Thanks,

Reply via email to