GNU bug report logs - #78011
[PATCH electronics-team] gnu: Add opensta.

Previous Next

Full log

View this message in rfc822 format

From: Maxim Cournoyer <maxim.cournoyer <at> gmail.com>
To: Cayetano Santos <csantosb <at> inventati.org>
Cc: 78011 <at> debbugs.gnu.org, Ekaitz Zarraga <ekaitz <at> elenq.tech>
Subject: [bug#78011] [PATCH electronics-team] gnu: Add opensta.
Date: Fri, 25 Apr 2025 11:58:51 +0900

Hi!

Cayetano Santos <csantosb <at> inventati.org> writes:

> * gnu/packages/electronics.scm (opensta): New variable.
>
> Change-Id: If753078958535971c8ab3fd7c934d19d24f0a2f7
> ---

[...]

> +(define-public opensta

It's good to include a comment about why a precise commit is used,
e.g. "Use the latest commit as they included unreleased fixes."
> +  (let ((commit "400c473fe384773a4788ee8378238462b4291fe3")
> +        (revision "0"))
> +    (package
> +      (name "opensta")
> +      (version (git-version "2.6.2" revision commit))
> +      (source
> +       (origin
> +         (method git-fetch)
> +         (uri (git-reference
> +               (url "https://github.com/parallaxsw/OpenSTA/")
> +               (commit commit)))
> +         (file-name (git-file-name name version))
> +         (sha256
> +          (base32 "091np9jnhfzggc7f5q9kx7kfa1fxjrj7d1mnw6q4gniqbs5hwrg5"))))
> +      (build-system cmake-build-system)
> +      (arguments
> +       (list
> +        #:tests? #f ; no tests

There are tests; their Jenkinsfile runs them via the 'test/regression'
script.  You'll need to override the check phase as it's not registered
as a CMake test target.

> +        #:out-of-source? #t

No need to explicit this; it's the default for cmake-build-system.

> +        #:configure-flags
> +        #~(list
> +           (string-append "-DCUDD_DIR=" #$cudd)

It'd be better if this referred to the same cudd package that is
provided as an input (otherwise it cannot be rewritten via package
transformations).  You can refer to it within this gexp with

#$(this-package-input "cudd")

> +           (string-append "-DCMAKE_INSTALL_PREFIX=" #$output))

No need for this, as it's also the default.

> +        #:make-flags
> +        #~(list (string-append "CC=" #$(cc-for-target))
> +                (string-append "CXX=" #$(cxx-for-target))
> +                (string-append "PREFIX=" #$output))))

I doubt these are needed; our cmake build system should know how to
cross-compile without this being manually set.

> +      (native-inputs (list swig bison flex))
> +      (inputs (list tcl tcllib zlib cudd eigen))
> +      (synopsis "Parallax Static Timing Analyzer")
> +      (description
> +       "OpenSTA is a gate level static timing verifier.  As a stand-alone executable

Please fold your paragraphs so they stay within < 80 columns, our coding
style guideline (M-q in Emacs).

> +it can be used to verify the timing of a design using standard file formats.")
> +      (home-page "https://github.com/parallaxsw/OpenSTA/")
> +      (license license:gpl3+))))

Otherwise it LGTM.  Could you please send a v2 with the above comments
taken into account?

-- 
Thanks,
Maxim

Previous Next

GNU bug tracking system
Copyright (C) 1999 Darren O. Benham, 1997,2003 nCipher Corporation Ltd, 1994-97 Ian Jackson.