Saturday, August 11, 2018

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

Hi Alessandro --

On 8/11/18 8:56 AM, Alessandro DE LAURENZIS wrote:
> Dear ports@ readers,
>
> enclosed please find a port for graywolf [1], the tool required by
> Qflow [2] for placement:
>
> [... snip ...]
> graywolf - a fork of TimberWolf 6.3.5
>
> graywolf is a placement tool used in VLSI design.
>
> TimberWolf was developed at Yale University, and was distributed
> as open source for a time until it was taken commercial. The last
> open-source version of TimberWolf does not perform detail routing,
> but is a professional-grade placement tool.
>
> The main improvement in graywolf is that the build process is more
> streamlined and that it behaves as a normal tool - you can call it
> from anywhere and no environment variables must be set first.
>
> It's mainly used together with Qflow
> (http://opencircuitdesign.com/qflow/).[... snip ...]
>
> First things first, upstream released a new version a month ago (after
> 2 years); I didn't have the chance to try it yet, so this tarball is
> for the previous release (0.1.4).
>

I would try with the latest version sooner rather than later.

> Patches:
> ========
> Apart from usual stuff needed to correctly set include files when the
> __OpenBSD__ identifier is found (patch-include_yalecad_base_h,
> patch-src_Ylib_timer_c), the main problem here is that the code is
> very old (with very limited possibility to see it updated, since the
> current maintainer doesn't know it very well; he's doing a great work
> in tweaking it as needed to build in a "modern" environemnt, but
> that's all):
>
> - in patch-src_Ylib_okmalloc_c, I removed the cfree function, using
> free instead (this should be fixed in the latest release);
>
> - in patch-src_date_getdate_c, I fixed the definition in main function
> from:
>
> main( argc , argv )
> int argc ;
> char *argv ;
>
> to:
>
> main( argc , argv )
> int argc ;
> char **argv ;
>
> Finally, I patched CMakeList.txt:
>
> - including /usr/X11R6/include and adding /usr/X11R6/lib to the link
> directories (please suggest if there is a wiser way to do that...);
>

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

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

> - 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!)
>
> Compiles and runs correctly on amd64, lightly tested in Qflow.
>
> Of course, I step-up for maintainership.
>
> [1] https://github.com/rubund/graywolf
> [2] https://marc.info/?l=openbsd-ports&m=153270090320035&w=2
>

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

Please put a newline between WANTLIB and MODULES.

Pull the SHARED_LIBS line up to where it belongs.

License is GPLv2+.

~Brian

No comments:

Post a Comment