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

[zerous <zerous@nocebo.space>]

On Tue, Jun 11, 2019 at 12:17:25PM +0200, Leonardo Taccari wrote:
> Hello Naveen,
> Thank you for working on it!
> Some possible suggestions and questions directly inline!
> 
> 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! :)

Sure, I will do it from now :)

> 
> > +++ 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.

Ah yes. I will amend this.

> 
> > --- /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.

Sure. I will move it.

> 
> > 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?

I am not sure about this. Initially, I got some errors with tests
which is why I disabled it.  I will work on it and let you know how it
goes.

> 
> > +PKG_DESTDIR_SUPPORT=	user-destdir
> 
> Please remove it: it is not needed because it is already the default.
>

I shall remove it.

> > [...]
> > +NOT_PAX_MPROTECT_SAFE+=	bin/wine64
> > [...]
> 
> Is this a copypasto? (it seems there is no wine64 executable)

Yes, it was a copypasta. I have fixed it today.

> 
> > +.include "../../devel/ncurses/buildlink3.mk"
> 
> Can mk/curses.buildlink3.mk be used instead?  If not, why?
> 

I don't think it would work because I tried building wine-4.4 against
curses in base and it threw errors. I shall try regardless.

> 
> 
> Thank you!

Once again, I apologize for the delayed reply.

-- 
zer