Re: [NEW/WIP] Qflow porting // [3/7] graywolf

Ciao Brian,

thanks for your prompt feedback.

On 08/11/18 16:41, Brian Callahan wrote:
[...]
> I would try with the latest version sooner rather than later.

I'll do that soon, I promise :-) The problem is that Qflow is... well, a
flow; and changing the version of one of the components requires double
checking the consistency of the entire sequence of steps, using some
test-cases; this takes a while.

[...]
> In patches/patch_CMakeLists_txt, you don't want to hardcode /usr/X11R6
> -- instead you should use ${OPENBSD_X11BASE}, which CMake understands.

Done

>> - adding "-Wno-return-type" to CFLAGS (otherwise the code doesn't
>> compile correctly with clang);
>>
>
> And I think more is warranted. This is really really not C99 (which
> clang defaults to), I would add
> # Really really not C99
> CONFIGURE_ARGS =        -DCMAKE_C_FLAGS="${CFLAGS} -std=gnu89"
>
> to give people a fighting chance to read the other warnings. I noticed
> some -Wformat warnings that look like they could matter with this.

Done; I put there "-Wno-return-type" too; this is more readable (instead
of jumping between the Makefile and the patch); what do you think?

I would be tempted to use -D variable assignments also for
include_directories and link_directories, if possible; but I do not know
if such variables exists... I tried with CMAKE_INCLUDE_PATH, without
success. Let me know.

>> - removing a symbolic link required after the install (I moved it in
>> the post-install target, since doing it the way it was doesn't work,
>> even if I didn't fully understand why... please comment!)

What about this point? Is it ok?

[...]
> Other stuff:
> You don't need HOMEPAGE or DISTNAME, the GH_* variables do that for you
> in this case.

Removed

> Please put a newline between WANTLIB and MODULES.
>
> Pull the SHARED_LIBS line up to where it belongs.
>
> License is GPLv2+.

Fixed

Attached please find the updated tarball.
--
Alessandro DE LAURENZIS
[mailto:just22@atlantide.t28.net]
Web: http://www.atlantide.t28.net
LinkedIn: http://it.linkedin.com/in/delaurenzis