Re: wip/glusterfs to pkgsrc filesystems/glusterfs

[Jonathan Schleifer <js%NetBSD.org@localhost>]

Am 27.02.23 um 22:18 schrieb Alexander Schreiber:

> Hi,
>
> so I've updated pkgsrc wip/glusterfs to 10.3 and didn't get any complaints
> after announcing that here and on pkgsrc-users (I hope at least someone
> else tried it).
>
> How to I get this from wip to actual pkgsrc?

The best is to send a patch against what's currently in pkgsrc proper. E.g.:

 $ diff -Nru --exclude=CVS /usr/pkgsrc/filesystems/glusterfs wip/glusterfs

Doing this, there's a few things that I see:

> -DISTNAME=      glusterfs-8.2
> -PKGREVISION=   8
> +DISTNAME=      glusterfs-10.3
> +PKGREVISION=   9

If you update the version, PKGREVISION should be reset.

> +BROKEN_ON_PLATFORM+=    *-*-earm*
> +BROKEN_ON_PLATFORM+=    *-*-hppa
> +BROKEN_ON_PLATFORM+=    *-*-i386
> +BROKEN_ON_PLATFORM+=    *-*-m68000
> +BROKEN_ON_PLATFORM+=    *-*-m68k
> +BROKEN_ON_PLATFORM+=    *-*-mipseb
> +BROKEN_ON_PLATFORM+=    *-*-mipsel
> +BROKEN_ON_PLATFORM+=    *-*-powerpc
> +BROKEN_ON_PLATFORM+=    *-*-riscv32
> +BROKEN_ON_PLATFORM+=    *-*-sparc
> +BROKEN_ON_PLATFORM+=    *-*-vax

Maybe ONLY_FOR_PLATFORM would be better here? This smells like the code 
isn't written portably, so probably will fail on anything but amd64 and 
aarch64 anyway?

If there's users of the old package for which this update drops support, 
we might need to make this a versioned package.

> -SUBST_FILES.etc+=      libglusterfs/src/logging.c
> -SUBST_FILES.etc+=      doc/glusterfsd.8
> +SUBST_FILES.etc=       libglusterfs/src/logging.c
> +SUBST_FILES.etc=       doc/glusterfsd.8

Why are you removing the + here? Especially, with your version, 
substitution is only performed in doc/glusterfsd.8 and not in logging.c.

> +TOOL_DEPENDS+= automake>=1.13:../../devel/automake

This shouldn't be needed, as you already add it to USE_TOOLS.

> -$NetBSD: patch-configure,v 1.5 2020/09/27 01:13:11 manu Exp $
> +$NetBSD$

I know the current package already does this, but it's really bad 
practice. Only patch configure.ac and then regenerate. Do not patch the 
generated file.

patch-libglusterfs-src-glusterfs-dict.h is a bit sketchy. How about 
checking for __LP64__ instead?

I believe for the removed patches you have verified they have indeed 
been upstreamed and made it into the release?

> I'm happy to take over pkgsrc filesystems/glusterfs, given that:
>   - it looks abandoned, since it is stuck at 8.2 (from 2020-09-16)
>   - I'm actually running glusterfs storage so I'm interested in keeping
>     this up to date (and yes, I've seen that 11.0 is out now)
>   - I'm working on getting my two portability patches merged into
>     upstream glusterfs (1/2 done)
>
> I've been happy pkgsrc user for many years, now I'd like to contribute
> as well.
>
> So, how do I got about getting this done?

I would propose to address the issues above and then I'm happy to import 
it. Feel free to set yourself as MAINTAINER. Also feel free to send 
further changes my way.

> Kind regards,
>             Alex.

--
Jonathan