Subject: Re: bug#78011: [PATCH electronics-team] gnu: Add opensta.

From: Maxim Cournoyer <[email-masked]>
Date: Sat, 26 Apr 2025 21:44:14 +0900

Hi,

Cayetano Santos <[email-masked]> writes:

[...]

> +(define-public opensta
> +  ;; There are no releases, we use last commit.
> +  (let ((commit "eb8d39a7dd81b5ca2582ad9bbce0fb6e094b3e0f")
> +        (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 "0bpc7fj4pd5713yny2vrh542jbag1kj20g0ji01c9scqb9av5qw5"))))
> +      (build-system cmake-build-system)
> +      (arguments
> +       (list

Please use the -DBUILD_SHARED_LIBRARY=ON #:configure-flags I suggested
to reduce the output size, as suggested in my original review.

> +        #:phases
> +        #~(modify-phases %standard-phases
> +            (replace 'check
> +              (lambda* (#:key (tests? #t) #:allow-other-keys)

Please do not set default values for arguments to this anonymous
procedure; it just clutters the view (#:tests? is always passed by the
build system -- it is it which holds the default value).

> +                (when tests?
> +                  (invoke "../source/test/regression")))))
> +        #:configure-flags
> +        #~(list
> +           (string-append "-DCUDD_DIR=" #$(this-package-input "cudd")))))
> +      (native-inputs (list swig bison flex))
> +      (inputs (list tcl tcllib zlib cudd eigen))

It looks like you overlooked my suggestion to sort the
native-inputs/inputs.

> +      (synopsis "Parallax Static Timing Analyzer")
> +      (description
> +       "OpenSTA is a gate level static timing verifier.  As a stand-alone executable
> +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))))

Why the license change from gpl3+ to gpl3?  The former was correct.

Otherwise, LGTM.

Could you send a v3?

-- 
Thanks,
Maxim