Friday, March 08, 2019

Re: [NEW/WIP] Qflow porting // [5/7] magic 8.1.224

Anthony, Stuart,

thanks for taking time to look into this.

On 07/03/2019 07:11, Anthony J. Bentley wrote:

> No strong opinion of that but if you push patches upstream (see below)
> it may be more practical to target the development branch for a while.

I would be reluctant to switch to the development branch, even
temporarily, since that isn't practical from user perspective.

>>> Upstream configure script is a wrapper around the one actually generated
>>> by autoconf, so I used:
>>>
>>> CONFIGURE_STYLE = simple
>
> A very thin wrapper. I'd rather set CONFIGURE_STYLE=gnu and
> WRKCONF=${WRKSRC}/scripts instead.

Done.

>>> and patched it adding -std=c89 and -Wno-parentheses to CFLAGS, in order
>>> to get rid of the huge amount of Wimplicit-function-declaration and
>>> Wno-parentheses warnings.
>
> My memory of magic is that it has some truly ancient ifdefs and code
> issues that might be worth fixing and shouldn't be too difficult.
>
>>> Other patches have essentially to do with termios functions; in
>>> particular, txGetTermState is not defined is OpenBSD; please carefully
>>> review what I made in patches/patch-textio_txInput_c, since I just tried
>>> to adapt some code found on the Internet used for similar situations;
>>> neither I really understand if those functions are actually needed when
>>> the Tcl/Tk version is used (this is the case for our port).
>
> -#if defined(SYSV) || defined(CYGWIN)
> +#if defined(SYSV) || defined(CYGWIN) || defined(__OpenBSD__)
>
> This is exactly what I'm talking about. If you check FreeBSD they have
> similar patches. When I see ifdefs like this in a codebase this old,
> I wonder: what systems even hit the else? I'd consider removing the
> pre-termios case completely and pushing it upstream.

That would be ideal; still, the termios code, as I said, was not there
at all, and my additions need review. Any feedback on that?

/usr/local/lib/magic/sys/scmos.tech>> post-extract:
>> mv ${WRKSRC}/lef/lefWrite.c.orig ${WRKSRC}/lef/lefWrite.c_orig
>
> PATCHORIG would remove the need for this.

Done.


> Can you kill the 2>&1 as well on the Makefile patches

Done.


Testing the port more deeply, I noticed that tech files were affected by
a bug similar to [1], but actually the clang preprocessor is behaving
differently (newlines seem to be removed). We could try to patch the sed
commands in scmos/Makefile and scmos/cif_template/Makefile, but this
would make the port unusable for archs with gcc in base.

I think it would be better to switch to ports-gcc as compiler,
considering also that it doesn't report all those
Wimplicit-function-declaration and Wparentheses messages (this isn't
necessarily a good thing, I know, but still...)

Attached an updated tarball.

[1] https://bugs.debian.org/cgi-bin/bugreport.cgi?bug=698760

--
Alessandro DE LAURENZIS
[mailto:just22@atlantide.t28.net]
Web: http://www.atlantide.t28.net
LinkedIn: https://www.linkedin.com/in/delaurenzis/

No comments:

Post a Comment