Re: wine32: Import Wine-4.4 (i386) as wip/wine32

[Leonardo Taccari <leot%NetBSD.org@localhost>]

Hello Naveen,
Thank you for working on it!
Some possible suggestions and questions directly inline!

Naveen Narayanan writes:
> Module Name:	pkgsrc-wip
> Committed By:	Naveen Narayanan <zerous@nocebo.space>
> Pushed By:	zerous
> Date:		Tue Jun 11 07:59:59 2019 +0200
> Changeset:	87b90e22db56647cd206135cbc74fb60555ea5aa
>
> Added Files:
> 	wine32/DESCR
> 	wine32/MESSAGE
> 	wine32/Makefile
> 	wine32/PLIST
> 	wine32/distinfo
> 	wine32/patches/patch-dlls_ntdll_thread.c
> 	wine32/patches/patch-dlls_wininet_http.c
>
> Log Message:
> wine32:	Import Wine-4.4 (i386) as wip/wine32
>

Minor suggestion: when importing package please always also include
their DESCR so people reading pkgsrc-wip-changes@ mailing list can
see what's the package is about by reading the commit log message! :)

> +++ b/wine32/DESCR
> @@ -0,0 +1,4 @@
> +Wine is a program which facilitates running Microsoft Windows Programs
> +on Unix. It acts as a compatibility layer by translating Windows API
> +calls into POSIX calls on the fly; thereby allowing you to run Windows
> +Applications on your UNIX desktop.

If it is only for i386, probably it would be better to add a newline and
add a comment about that in the DESCR too.

> --- /dev/null
> +++ b/wine32/MESSAGE
> @@ -0,0 +1,11 @@
> +===========================================================================
> +$NetBSD$
> +
> +If you are experiencing crashes, it may be due to a security measure
> +blocking the mapping of the NULL page.
> +
> +You can disable this (at a security cost) with the following commands:
> +
> + NetBSD:
> +	# sysctl -w vm.user_va0_disable=0
> +===========================================================================

This MESSAGE seems specific to NetBSD, please move it to MESSAGE.NetBSD
instead.

> diff --git a/wine32/Makefile b/wine32/Makefile
> new file mode 100644
> index 0000000000..451bda6240
> --- /dev/null
> +++ b/wine32/Makefile
> @@ -0,0 +1,35 @@
> [...]
> +CONFIGURE_ARGS+=	--disable-tests
> [...]

Why tests are disabled?  Can they be built and tested via TEST_TARGET?

> +PKG_DESTDIR_SUPPORT=	user-destdir

Please remove it: it is not needed because it is already the default.

> [...]
> +NOT_PAX_MPROTECT_SAFE+=	bin/wine64
> [...]

Is this a copypasto? (it seems there is no wine64 executable)

> +.include "../../devel/ncurses/buildlink3.mk"

Can mk/curses.buildlink3.mk be used instead?  If not, why?

> [...]


Thank you!