On 2022/11/13 16:20:18 +0100, Alexander Klimov <[email protected]> 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,
