Monday, July 11, 2022

Re: [NEW] archivers/zpaqfranz 55.1

Omar Polo <op@openbsd.org> wrote:
> Hello,
>
> tux0r <tux0r@rosaelefanten.org> wrote:
> > I ported what seems to be the only actively maintained ZPAQ implementation to OpenBSD (using 7.1 on AMD64).
> >
> > tar.gz archive:
> > https://cdn.tuxproject.de/openbsd-ports/zpaqfranz-55.1.tar.gz
>
> please send ports as attachment next time.
>
> > The developer added OpenBSD-specific code for this version and we both tested its basic functionality before I made the port. GitHub repository for the software itself: https://github.com/fcorbelli/zpaqfranz/
> >
> > I hope that I made no mistakes. This is my first port (but I plan to maintain it if you accept it into OpenBSD). :-)
> >
> > tux0r.
>
> It's a very good start, there are some minor nits:
>
> - the GH_* vars were indented with spaces instead of tabs
>
> - no need to specify DISTNAME, MASTER_SITES and DISTFILES in this case.
> There's only one tarball and GH_* sets up everything else. Same for
> PKGNAME, it defaults to ${GH_PROJECT}-${GH_TAGNAME} and it's fine in
> this case. HOMEPAGE also defaults to the github repo.
>
> - EPOCH is only added when a version goes backward; for new ports it's
> not specified.
>
> - COMPILER seems quite restrictive: ports-gcc at least should be there,
> but maybe it works with even with base-gcc? it doesn't link to
> anything other than the c++ stdlib.
>
> - The patch to the makefile is not needed, it's possible to override
> the vars by passing them as argument to make; so just add
>
> MAKE_FLAGS += CXX=${CXX}
>
> should do it. While here I'm also passing CXXFLAGS: the port
> should respect the cflags passed by the port infrastructure.
>
> - talking about the makefile, USE_GMAKE shouldn't be needed either. It
> seems a portable makefile (except for the CRLF that confuse our
> make(1), but that can be fixed upstream :-)
>
> here's a patch against your makefile that address these points and an
> updated tarball. I briefly tested following the examples on the readme
> and it seems to work well

I missed that this also has a jit inside and that's not gonna work on
!amd64. (in theory it should work on some x86 too)

Furthermore, the jit does:

p=(U8*)mmap(0, newsize, PROT_READ|PROT_WRITE|PROT_EXEC,
MAP_PRIVATE|MAP_ANON, -1, 0);

wich warrants a USE_WXNEEDED.

No comments:

Post a Comment