On 08/16/18 06:41, Alessandro DE LAURENZIS wrote:
> Hi Brian,
>
> On 08/15/18 23:07, Brian Callahan wrote:
> [...]
>>> 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'm ok with this. You maybe want to work with upstream to fix it so
>> you don't need to silence the warnings, though :)
>>
>>> 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.
>>>
>> You can add the include directories right into -DCMAKE_C_FLAGS. If
>> you're going to put it into the port Makefile, it'll look like
>> -I${X11BASE}/include. For the linker flags
>> -DCMAKE_EXE_LINKER_FLAGS="${LDFLAGS} -L${X11BASE}/lib ..." (and maybe
>> -DCMAKE_SHARED_LINKER_FLAGS="${LDFLAGS} -L${X11BASE}/lib ..." for the
>> shared library if necessary).
>
> After double checking, I think that preserving the include_directories
> and link_directories lines in CMakeLists.txt makes the "Really really
> not C99" comment more in line with the CONFIGURE_ARGS contents.
>
> So I would ask for OKs for the port as it is.
>
> All the best
>
I think that's fine. There would still be a patch for CMakeLists.txt
even if you were to pull that bit out, so there is nothing gained or
lost here.
At this point, the only thing I would change is the pkg/DESCR. As it is,
you can combine the first two sentences into one, something like:
graywolf is a fork of TimberWolf 6.3.5, a placement tool used in VLSI
design.
But the middle paragraph of the DESCR doesn't really tell me what this
is or what it does, just that it used to be a project developed at Yale.
Maybe instead that's a spot to explain what a placement tool is or does.
The last bit is fine, it's likely useful especially for those who have
used the original software in the past.
~Brian
No comments:
Post a Comment